diff --git a/README.md b/README.md index 54697be..e967f31 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # GitHub Copilot modernization - Copilot CLI Plugin +[![Listed in Awesome Copilot](https://img.shields.io/badge/Listed_in-Awesome_Copilot-blue?logo=github)](https://awesome-copilot.github.com/plugins/#file=plugins%2Fgithub-copilot-modernization) + Autonomous application modernization using multi-agent orchestration for [GitHub Copilot CLI](https://github.com/github/copilot-cli). ## Overview diff --git a/plugins/github-copilot-modernization/.mcp.json b/plugins/github-copilot-modernization/.mcp.json index 4394106..018e5d6 100644 --- a/plugins/github-copilot-modernization/.mcp.json +++ b/plugins/github-copilot-modernization/.mcp.json @@ -5,7 +5,7 @@ "command": "npx", "args": [ "-y", - "@microsoft/github-copilot-app-modernization-mcp-server@1.21.0", + "@microsoft/github-copilot-app-modernization-mcp-server@1.22.0", "--callerType", "github-copilot-modernization-plugin" ], diff --git a/plugins/github-copilot-modernization/README.md b/plugins/github-copilot-modernization/README.md index 54697be..e967f31 100644 --- a/plugins/github-copilot-modernization/README.md +++ b/plugins/github-copilot-modernization/README.md @@ -1,5 +1,7 @@ # GitHub Copilot modernization - Copilot CLI Plugin +[![Listed in Awesome Copilot](https://img.shields.io/badge/Listed_in-Awesome_Copilot-blue?logo=github)](https://awesome-copilot.github.com/plugins/#file=plugins%2Fgithub-copilot-modernization) + Autonomous application modernization using multi-agent orchestration for [GitHub Copilot CLI](https://github.com/github/copilot-cli). ## Overview diff --git a/plugins/github-copilot-modernization/agents/assessment-coordinator.agent.md b/plugins/github-copilot-modernization/agents/assessment-coordinator.agent.md index 13ba9fa..01c23d7 100644 --- a/plugins/github-copilot-modernization/agents/assessment-coordinator.agent.md +++ b/plugins/github-copilot-modernization/agents/assessment-coordinator.agent.md @@ -37,6 +37,7 @@ You coordinate the assessment phase by detecting the project language, invoking - `enableContainerization`: boolean - `targetOS`: Array of `windows` | `linux` - `minimumCveSeverity`: `low` | `medium` | `high` | `critical` + - `cveScanScope`: `direct` | `all` ## Language Detection diff --git a/plugins/github-copilot-modernization/agents/execution-coordinator.agent.md b/plugins/github-copilot-modernization/agents/execution-coordinator.agent.md index f02142e..7c273c0 100644 --- a/plugins/github-copilot-modernization/agents/execution-coordinator.agent.md +++ b/plugins/github-copilot-modernization/agents/execution-coordinator.agent.md @@ -60,6 +60,7 @@ When a worker agent returns (success OR failure): - `modernize-azure-java` - For Azure Service Bus, SQL, Redis, Key Vault, and other Azure migrations - `modernize-java-security` - For CVE fixes and vulnerability scanning in Java/Maven (in-place fixes only, NOT Azure service integrations) - `modernize-azure-dotnet` - For .NET Azure migrations and CVE fixes in NuGet +- `modernize-deployment` - For infrastructure and deployment tasks: Dockerfiles, Kubernetes/AKS/ACA, Bicep/IaC, CI/CD pipelines - `modernize-rearchitecture` - For structural rewrites and rearchitecture (only when task does not match any known scenario) ## Delegation Workflow @@ -82,16 +83,16 @@ When a worker agent returns (success OR failure): │ • Deprecated API migration │ │ • Application Insights │ │ • Maven security plugin │ │ • Maven / Gradle config │ │ • Managed Identity │ │ • Jackson / Log4j CVE fix │ └──────────────────────────────┘ └──────────────────────────────┘ └──────────────────────────────┘ -┌──────────────────────────────┐ ┌──────────────────────────────┐ -│ modernize-azure-dotnet │ │ modernize-rearchitecture │ -│ │ │ │ -│ • .NET Azure migration │ │ • Structural rewrites when │ -│ • NuGet CVE vulnerability │ │ no known scenario matches │ -│ • ASP.NET to Azure │ │ • WinForms → React/Angular │ -│ • dotnet build / test │ │ • Monolith → Microservices │ -│ • .NET CVE advisory check │ │ • JSP → Modern SPA │ -│ • NuGet security audit │ │ • Module extraction (new dir)│ -└──────────────────────────────┘ └──────────────────────────────┘ +┌──────────────────────────────┐ ┌──────────────────────────────┐ ┌──────────────────────────────┐ +│ modernize-azure-dotnet │ │ modernize-deployment │ │ modernize-rearchitecture │ +│ │ │ │ │ │ +│ • .NET Azure migration │ │ • Dockerfile generation │ │ • Structural rewrites when │ +│ • NuGet CVE vulnerability │ │ • AKS/ACA deployment │ │ no known scenario matches │ +│ • ASP.NET to Azure │ │ • Bicep/ARM IaC generation │ │ • WinForms → React/Angular │ +│ • dotnet build / test │ │ • CI/CD pipeline generation │ │ • Monolith → Microservices │ +│ • .NET CVE advisory check │ │ • Docker Image Scanning │ │ • JSP → Modern SPA │ +│ • NuGet security audit │ │ • Region/SKU/Pricing checks │ │ • Module extraction (new dir)│ +└──────────────────────────────┘ └──────────────────────────────┘ └──────────────────────────────┘ ``` **How to delegate:** @@ -139,6 +140,7 @@ You have access to specialized migration agents for application modernization: - **modernize-azure-java**: Azure Service Bus, Azure SQL, Azure Redis, Azure Key Vault, and other Azure service migrations - **modernize-java-security**: CVE vulnerability scanning and fixes in Java/Maven dependencies (in-place fixes only) - **modernize-azure-dotnet**: .NET Azure migrations and CVE fixes in NuGet dependencies +- **modernize-deployment**: Infrastructure and deployment tasks (Dockerfiles, Kubernetes/AKS/ACA, Bicep/IaC, CI/CD pipelines) - **modernize-rearchitecture**: Structural rewrites only when the task does not match any known scenario These agents query the MCP knowledge base directly for migration patterns and best practices. @@ -295,25 +297,12 @@ Before delegating tasks, check if a rulebook exists and pass its context to all **You create ONE branch before delegating, and pass it to ALL workers. You do NOT generate or pass session IDs — workers handle their own session IDs.** -1. Generate timestamp with **second-level precision**: `YYYYMMDDHHMMSS` (e.g., `20260424153045`). This MUST include hours, minutes, AND seconds — never truncate to just the date or hour. - - **DO NOT guess the time.** Run a terminal command to get the real current time: - - PowerShell: `Get-Date -Format "yyyyMMddHHmmss"` - - Bash/Linux: `date +"%Y%m%d%H%M%S"` - - Use the exact output as the timestamp. Never fabricate a round number like `120000`. -2. Branch name depends on detected language: - - Java projects: `modernize/java-` - - .NET projects: `modernize/dotnet-` -3. **Handle uncommitted changes BEFORE creating the branch:** - - First, retrieve the policy: Try calling `appmod-get-vscode-config(configName: "uncommittedChangesAction")` to get the user's configured policy. If the tool is not available (e.g., in CLI mode), default to **"Always Stash"**. - - Use `appmod-version-control(action: "checkForUncommittedChanges", workspacePath: )` to check - - If uncommitted changes exist, handle them according to the retrieved policy: - - **Always Stash** (default): Use `appmod-version-control(action: "stashChanges", stashMessage: "Auto-stash: Save uncommitted changes before migration", workspacePath: )` - - **Always Commit**: Use `appmod-version-control(action: "commitChanges", commitMessage: "Auto-commit: Save uncommitted changes before migration", workspacePath: )` - - **Always Discard**: Use `appmod-version-control(action: "discardChanges", workspacePath: )` - - **Always Ask**: Inform the user about the uncommitted changes and ask how they would like to proceed (stash, commit, or discard). Wait for the user's response before taking action. - - Verify clean: Use `appmod-version-control(action: "checkForUncommittedChanges", workspacePath: )` to confirm working directory is clean -4. Create the branch via `appmod-version-control(action: "createBranch", branchName: "modernize/-", workspacePath: )` -5. Pass `BRANCH: modernize/-` in every delegation prompt +1. Detect project language (from `tasks.json` metadata or project indicators): `java` or `dotnet`. +2. **Handle uncommitted changes and create branch with a single call**: + `appmod-version-control(action: "prepareBranch", language: "java"|"dotnet", workspacePath: )` + The tool handles any uncommitted changes, auto-generates the branch name (`modernize/-`), and returns it in `details.branchName`. + - **Handle the response**: the call handles any uncommitted changes automatically per the host-configured policy and creates the branch. Use `details.branchName` as `BRANCH` for all delegation prompts. +3. Pass the resulting `BRANCH` value in every delegation prompt. **Workers MUST NOT handle uncommitted changes** — this is already done here before branch creation. @@ -324,12 +313,8 @@ Workers use the provided branch (skipping their own branch creation) but generat ### Mode 1: Planned Execution (planning-path provided) 1. **Create Branch** - - Generate timestamp by running a terminal command (do NOT guess): - - PowerShell: `Get-Date -Format "yyyyMMddHHmmss"` - - Bash/Linux: `date +"%Y%m%d%H%M%S"` - Detect language from `tasks.json` metadata or project indicators - - **Handle uncommitted changes** (per Branching Strategy step 3): try `appmod-get-vscode-config` for policy (default: Always Stash) → check → handle per policy → verify clean - - Create branch: `modernize/java-` (Java) or `modernize/dotnet-` (.NET) + - **Handle uncommitted changes and create branch** (per Branching Strategy step 2): call `appmod-version-control(action: "prepareBranch", language: ..., workspacePath: ...)` once. The tool resolves the host's uncommitted-changes policy automatically, handles any uncommitted changes, and returns the auto-generated branch name in `details.branchName`. - **Do NOT generate or pass a session ID.** Each worker generates its own. 2. **Load Plan** @@ -376,7 +361,7 @@ Workers use the provided branch (skipping their own branch creation) but generat BRANCH: modernize/java- Workspace: /path/to/app - The coordinator has already created and checked out this branch — you are already on it. Do NOT run `git checkout`, `git switch`, or `#appmod-version-control` with action `createBranch`. Commit directly on the current HEAD. + The coordinator has already created and checked out this branch — you are already on it. Do not create or switch branches yourself; commit directly on the current HEAD. ``` **Example - CWE (one delegation per CWE id):** @@ -387,7 +372,7 @@ Workers use the provided branch (skipping their own branch creation) but generat BRANCH: modernize/java- Workspace: /path/to/app - The coordinator has already created and checked out this branch — you are already on it. Do NOT run `git checkout`, `git switch`, or `#appmod-version-control` with action `createBranch`. Commit directly on the current HEAD. + The coordinator has already created and checked out this branch — you are already on it. Do not create or switch branches yourself; commit directly on the current HEAD. ``` > Do NOT include `kbId:` / `taskId:` / `by kbId:` for CWE tasks. The worker will pass the goal sentence as `scenario` to `#appmod-run-task`. Never pass a `taskId` derived from `tasks.json`. @@ -400,7 +385,7 @@ Workers use the provided branch (skipping their own branch creation) but generat BRANCH: modernize/java- Workspace: /path/to/app - The coordinator has already created and checked out this branch — you are already on it. Do NOT run `git checkout`, `git switch`, or `#appmod-version-control` with action `createBranch`. Commit directly on the current HEAD. + The coordinator has already created and checked out this branch — you are already on it. Do not create or switch branches yourself; commit directly on the current HEAD. Rulebook: .github/modernize/rulebook/ (if exists) ``` @@ -431,7 +416,7 @@ Workers use the provided branch (skipping their own branch creation) but generat BRANCH: modernize/java- Workspace: /path/to/app - The coordinator has already created and checked out this branch — you are already on it. Do NOT run `git checkout`, `git switch`, or `#appmod-version-control` with action `createBranch`. Commit directly on the current HEAD. + The coordinator has already created and checked out this branch — you are already on it. Do not create or switch branches yourself; commit directly on the current HEAD. Rulebook: .github/modernize/rulebook/ (if exists) ``` @@ -444,7 +429,7 @@ Workers use the provided branch (skipping their own branch creation) but generat BRANCH: modernize/dotnet- Workspace: /path/to/dotnet-app - The coordinator has already created and checked out this branch — you are already on it. Do NOT run `git checkout`, `git switch`, or `#appmod-version-control` with action `createBranch`. Commit directly on the current HEAD. + The coordinator has already created and checked out this branch — you are already on it. Do not create or switch branches yourself; commit directly on the current HEAD. ``` 5. **Task Dependency Management** @@ -463,12 +448,8 @@ Workers use the provided branch (skipping their own branch creation) but generat ### Mode 2: Specific Task Intent (task-details provided) 1. **Create Branch** - - Generate timestamp by running a terminal command (do NOT guess): - - PowerShell: `Get-Date -Format "yyyyMMddHHmmss"` - - Bash/Linux: `date +"%Y%m%d%H%M%S"` - Detect language from task-details or project indicators - - **Handle uncommitted changes** (per Branching Strategy step 3): try `appmod-get-vscode-config` for policy (default: Always Stash) → check → handle per policy → verify clean - - Create branch: `modernize/java-` (Java) or `modernize/dotnet-` (.NET) + - **Handle uncommitted changes and create branch** (per Branching Strategy step 2): call `appmod-version-control(action: "prepareBranch", language: ..., workspacePath: ...)` once. The tool resolves the host's uncommitted-changes policy automatically and returns the auto-generated branch name in `details.branchName`. - **Do NOT generate or pass a session ID.** The worker generates its own. 2. **Check for Rulebook** (see [Rulebook-Aware Execution](#rulebook-aware-execution)) @@ -492,7 +473,7 @@ Workers use the provided branch (skipping their own branch creation) but generat BRANCH: modernize/java- Workspace: /testbed/java-migration-examples/containerproxy - The coordinator has already created and checked out this branch — you are already on it. Do NOT run `git checkout`, `git switch`, or `#appmod-version-control` with action `createBranch`. Commit directly on the current HEAD. + The coordinator has already created and checked out this branch — you are already on it. Do not create or switch branches yourself; commit directly on the current HEAD. Rulebook: .github/modernize/rulebook/ (if exists) ``` @@ -514,8 +495,9 @@ Route by **task type**, using this priority order: 4. **Credential migration to Azure Key Vault** (adds Azure SDK) → `modernize-azure-java` 5. **.NET tasks** → `modernize-azure-dotnet` 6. **Technology migration matching a known scenario** (see list below) → `modernize-azure-java` -7. **No matching scenario + requires structural rewrite** → `modernize-rearchitecture` -8. **No matching scenario + NOT structural rewrite** → `modernize-azure-java` (fallback, let worker search KB at runtime) +7. **Infrastructure/deployment task** (Dockerfile, K8s, AKS/ACA, Bicep, CI/CD) → `modernize-deployment` +8. **No matching scenario + requires structural rewrite** → `modernize-rearchitecture` +9. **No matching scenario + NOT structural rewrite** → `modernize-azure-java` (fallback, let worker search KB at runtime) ### Known Scenarios — KB-backed (→ `modernize-azure-java`) @@ -533,6 +515,28 @@ These scenarios have knowledge bases. Any task matching one of these goes to `mo - **Build Tools**: Ant → Maven, Eclipse → Maven - **Kafka (Confluent Cloud)**: Confluent Cloud Kafka authentication +### Known Scenarios — Deployment-backed (→ `modernize-deployment`) + +These scenarios involve infrastructure and deployment artifacts. Any task matching one of these goes to `modernize-deployment`: + +- **End to End Containerization**: Analyze application, generate optimized Dockerfile, build and verify image, scan for vulnerabilities +- **End to End Deployment**: Analyze application, containerize if needed, generate Bicep/Terraform, deploy to Azure, validate deployment +- **Dockerfile Generation**: Generate optimized Dockerfile for the application based on its structure and dependencies +- **Docker Image Build**: Build Docker images from Dockerfile and verify the build +- **Docker Image Scan**: Scan Docker images for vulnerabilities +- **Kubernetes/AKS Manifests**: Kubernetes deployment manifests, Helm charts, Azure Kubernetes Service configuration +- **Azure Container Apps**: ACA configuration, Dapr integration, scaling rules +- **Infrastructure as Code**: Bicep/ARM/Terraform templates for Azure resources (ACR, AKS, ACA, Log Analytics, Key Vault references, etc.) +- **IaC Rules**: Get best practices and rules for writing Bicep/Terraform for Azure deployments +- **CI/CD Pipelines**: GitHub Actions, Azure DevOps, GitLab CI/CD pipelines for build and deployment +- **CI/CD Pipeline Guidance**: Get best practices and guidance for setting up CI/CD pipelines +- **Architecture Diagram**: Generate application architecture diagrams +- **Repository Analysis**: Analyze repository structure for containerization +- **Pricing Estimation**: Estimate Azure costs for the deployment +- **SKU Availability**: Check availability of Azure SKUs in different regions +- **Quota Checks**: Check Azure subscription quotas for relevant resources +- **App Logs**: Get Azure app deployment logs + ### Known Scenarios — RAG-backed (→ `modernize-java-upgrade`) These scenarios have RAG prompts. Any task matching one of these goes to `modernize-java-upgrade`: @@ -543,7 +547,6 @@ These scenarios have RAG prompts. Any task matching one of these goes to `modern - Jakarta EE upgrade (javax→jakarta) - Deprecated API upgrade - Azure legacy Java SDK upgrade -- Containerization (→ handled by `modernize-azure-java` as infra task) ### Migration vs Rearchitecture @@ -573,6 +576,8 @@ If a task does NOT match any known scenario but is a simple technology swap → | cwe-fix (per CWE id) | `modernize-azure-java` | CWE rule-based code remediation | | credential-to-azure-keyvault | `modernize-azure-java` | Azure Key Vault integration (adds Azure SDK + Managed Identity) | | dotnet-azure-migration / dotnet-cve-fix | `modernize-azure-dotnet` | .NET Azure migration or CVE fixes | +| deployment | `modernize-deployment` | Deployment to Azure to Container Apps, AKS, App Service | +| containerization | `modernize-deployment` | Containerization (Dockerfile generation, Docker image validation, Kubernetes preparation) | | rearchitecture / structural-rewrite | `modernize-rearchitecture` | ONLY for fundamental architecture changes (not technology swaps) | | database-migration (H2, PostgreSQL, MySQL, etc.) | `modernize-azure-java` | Any database migration uses the same workflow | | build-verification / compile-check | Same worker as preceding migration tasks | Verification is part of the migration, not a separate routing | @@ -611,7 +616,7 @@ Orchestrator → You: } You: -1. Create branch → modernize/java-20260413120000 +1. Call prepareBranch(language: "java") → branch name returned: modernize/java-20260413120000 2. Load tasks.json → 8 tasks (3 Java upgrade, 5 Azure migration) 3. Check for rulebook → Found .github/modernize/rulebook/ 4. Read rulebook → all .md files in rulebook folder @@ -642,7 +647,7 @@ Orchestrator → You: } You: -1. Create branch → modernize/java-20260413150000 +1. Call prepareBranch(language: "java") → branch name returned: modernize/java-20260413150000 2. Check for rulebook → No rulebook found, skip 3. Determine agent → modernize-azure-java (Azure migration) 4. Delegate to `modernize-azure-java` subagent with prompt: @@ -666,7 +671,7 @@ Orchestrator → You: You: 1. Load plan → tasks.json has metadata.language = "dotnet", 3 tasks found -2. Create branch → modernize/dotnet-20260413120000 +2. Call prepareBranch(language: "dotnet") → branch name returned: modernize/dotnet-20260413120000 3. Check for rulebook → No rulebook found, skip 4. Route ALL tasks to modernize-azure-dotnet (all with BRANCH only — no session ID): - Task 1: modernize-azure-dotnet (SQL Server → Azure SQL) diff --git a/plugins/github-copilot-modernization/agents/modernize-azure-dotnet.agent.md b/plugins/github-copilot-modernization/agents/modernize-azure-dotnet.agent.md index 33aa459..5e98418 100644 --- a/plugins/github-copilot-modernization/agents/modernize-azure-dotnet.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize-azure-dotnet.agent.md @@ -29,7 +29,6 @@ tools: - appmod-consistency-validation - appmod-create-migration-summary - appmod-fetch-knowledgebase - - appmod-get-vscode-config - appmod-preview-markdown - appmod-run-task - appmod-search-file @@ -142,24 +141,12 @@ Use #appmod-version-control with action 'commitChanges' and commitMessage "Code ⚠️ **CRITICAL INSTRUCTIONS FOR VERSION CONTROL SETUP**: * You MUST execute these steps BEFORE starting any code migration tasks * **Branch handling (delegation-aware)**: - - **IF a `BRANCH` value was provided in the delegation prompt** (e.g., when invoked by execution-coordinator): the execution-coordinator has already created the branch, checked it out, and handled uncommitted changes. You are already on ``. Do NOT run `git checkout`, `git switch`, or any direct git command. Do NOT call `#appmod-version-control` with action `stashChanges`, `createBranch`, or `checkForUncommittedChanges`. You MAY call `#appmod-version-control` with action `checkStatus` only to record the current branch into the progress file — do not switch branches based on the result. + - **IF a `BRANCH` value was provided in the delegation prompt** (e.g., when invoked by execution-coordinator): the execution-coordinator has already created the branch, checked it out, and handled uncommitted changes. You are already on `` — use `` directly when recording the current branch in the progress file. Do not create, switch, or query branches yourself, and do not run direct `git` commands. Only call `#appmod-version-control` later for the final-commit step (`checkForUncommittedChanges` + `commitChanges`). Skip the rest of this section. - **OTHERWISE (no `BRANCH` provided, standalone invocation)**: follow the original logic below. -* Use #appmod-version-control to check if version control system is available: - - Check status with action 'checkStatus' in workspace directory: {{workspacePath}} - - ⚠️ **MANDATORY**: Check for existing uncommitted changes before creating any new branch: - * Use #appmod-version-control with action 'checkForUncommittedChanges' in workspace directory: {{workspacePath}} - * ⚠️ **CRITICAL**: IF uncommitted changes exist, you MUST handle them according to the 'uncommittedChangesAction' retrieved during plan generation BEFORE proceeding to branch creation: - - If the policy is 'Always Stash': You MUST use #appmod-version-control with action 'stashChanges' and stashMessage "Auto-stash: Save uncommitted changes before migration" in workspace directory: {{workspacePath}} - - If the policy is 'Always Commit': You MUST use #appmod-version-control with action 'commitChanges' and commitMessage "Auto-commit: Save uncommitted changes before migration" in workspace directory: {{workspacePath}} - - If the policy is 'Always Discard': You MUST use #appmod-version-control with action 'discardChanges' in workspace directory: {{workspacePath}} - - If the policy is 'Always Ask': You MUST inform the user about the uncommitted changes and ask how they would like to proceed, providing these options: stash, commit, or discard. Wait for the user's response before taking any action. - * ⚠️ **VERIFICATION REQUIRED**: After handling uncommitted changes, you MUST use #appmod-version-control with action 'checkForUncommittedChanges' to verify that the working directory is clean in workspace directory: {{workspacePath}} before proceeding to branch creation - * IF no uncommitted changes exist: proceed directly to branch creation - - ⚠️ **ONLY AFTER handling uncommitted changes**: Use #appmod-version-control with action 'createBranch' and branchName "{{targetBranch}}" in workspace directory: {{workspacePath}} - - Verify branch creation was successful before proceeding - - You MUST check the previous branch and the new branch in the general section of progress file. -* If NO version control system detected (as indicated by the response from #appmod-version-control): - - Note "No version control detected" and proceed with direct migration on workspace directory: {{workspacePath}} +* Call #appmod-version-control with action 'prepareBranch', branchName '{{targetBranch}}' in workspace directory: {{workspacePath}}. This single call handles any uncommitted changes and creates the branch. +* Handle the tool response: + * If `success=false` and `details.versionControlAvailable=false`: note "No version control detected" in the progress file and proceed with direct migration on workspace directory: {{workspacePath}}. + * Otherwise verify branch creation was successful and record the previous and new branch in the general section of the progress file. ## Core Principles diff --git a/plugins/github-copilot-modernization/agents/modernize-azure-java.agent.md b/plugins/github-copilot-modernization/agents/modernize-azure-java.agent.md index 605f17f..0238265 100644 --- a/plugins/github-copilot-modernization/agents/modernize-azure-java.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize-azure-java.agent.md @@ -24,7 +24,6 @@ tools: - appmod-consistency-validation - appmod-create-migration-summary - appmod-fetch-knowledgebase - - appmod-get-vscode-config - appmod-preview-markdown - appmod-run-task - appmod-search-file @@ -47,7 +46,6 @@ tools: - appmod-mcp-server/appmod-consistency-validation - appmod-mcp-server/appmod-create-migration-summary - appmod-mcp-server/appmod-fetch-knowledgebase - - appmod-mcp-server/appmod-get-vscode-config - appmod-mcp-server/appmod-preview-markdown - appmod-mcp-server/appmod-run-task - appmod-mcp-server/appmod-search-file @@ -161,7 +159,6 @@ Use the response to fill in the placeholders below throughout this workflow: * USE - #appmod-consistency-validation to validate code consistency after migration and ensure behavior equivalence * USE - #appmod-completeness-validation to validate migration completeness by systematically discovering ALL unchanged items across ALL KB patterns before fixing them - NO EXCEPTIONS for perceived "unused" or "intentional" files * You MUST use tool #appmod-validate-cves-for-java to validate and fix introduced CVEs -* You MUST use tool #appmod-get-vscode-config to retrieve extension configuration settings ## Subagent Usage Instructions * You MUST use #agent tool to delegate complex, multi-step tasks that require deep analysis and systematic execution @@ -229,24 +226,12 @@ Use the response to fill in the placeholders below throughout this workflow: ⚠️ **CRITICAL INSTRUCTIONS FOR VERSION CONTROL SETUP**: * You MUST execute these steps BEFORE starting any code migration tasks * **Branch handling (delegation-aware)**: - - **IF a `BRANCH` value was provided in the delegation prompt** (e.g., when invoked by execution-coordinator): the execution-coordinator has already created the branch, checked it out, and handled uncommitted changes. You are already on ``. Do NOT run `git checkout`, `git switch`, or any direct git command. Do NOT call `#appmod-version-control` with action `stashChanges`, `createBranch`, or `checkForUncommittedChanges`. You MAY call `#appmod-version-control` with action `checkStatus` only to record the current branch into the progress file — do not switch branches based on the result. + - **IF a `BRANCH` value was provided in the delegation prompt** (e.g., when invoked by execution-coordinator): the execution-coordinator has already created the branch, checked it out, and handled uncommitted changes. You are already on `` — use `` directly when recording the current branch in the progress file. Do not create, switch, or query branches yourself, and do not run direct `git` commands. Only call `#appmod-version-control` later for the final-commit step (`checkForUncommittedChanges` + `commitChanges`). Skip the rest of this section. - **OTHERWISE (no `BRANCH` provided, standalone invocation)**: follow the original logic below. -* Use #appmod-version-control to check if version control system is available: - - Check status with action 'checkStatus' in workspace directory: {{workspacePath}} - - ⚠️ **MANDATORY**: Check for existing uncommitted changes before creating any new branch: - * Use #appmod-version-control with action 'checkForUncommittedChanges' in workspace directory: {{workspacePath}} - * ⚠️ **CRITICAL**: IF uncommitted changes exist, you MUST handle them according to the 'uncommittedChangesAction' retrieved during plan generation BEFORE proceeding to branch creation: - - If the policy is 'Always Stash': You MUST use #appmod-version-control with action 'stashChanges' and stashMessage "Auto-stash: Save uncommitted changes before migration" in workspace directory: {{workspacePath}} - - If the policy is 'Always Commit': You MUST use #appmod-version-control with action 'commitChanges' and commitMessage "Auto-commit: Save uncommitted changes before migration" in workspace directory: {{workspacePath}} - - If the policy is 'Always Discard': You MUST use #appmod-version-control with action 'discardChanges' in workspace directory: {{workspacePath}} - - If the policy is 'Always Ask': You MUST inform the user about the uncommitted changes and ask how they would like to proceed, providing these options: stash, commit, or discard. Wait for the user's response before taking any action. - * ⚠️ **VERIFICATION REQUIRED**: After handling uncommitted changes, you MUST use #appmod-version-control with action 'checkForUncommittedChanges' to verify that the working directory is clean in workspace directory: {{workspacePath}} before proceeding to branch creation - * IF no uncommitted changes exist: proceed directly to branch creation - - ⚠️ **ONLY AFTER handling uncommitted changes**: Use #appmod-version-control with action 'createBranch' and branchName "{{targetBranch}}" in workspace directory: {{workspacePath}} - - Verify branch creation was successful before proceeding - - You MUST check the previous branch and the new branch in the general section of progress file. -* If NO version control system detected (as indicated by the response from #appmod-version-control): - - Note "No version control detected" and proceed with direct migration on workspace directory: {{workspacePath}} +* Call #appmod-version-control with action 'prepareBranch', branchName '{{targetBranch}}' in workspace directory: {{workspacePath}}. This single call handles any uncommitted changes and creates the branch. +* Handle the tool response: + * If `success=false` and `details.versionControlAvailable=false`: note "No version control detected" in the progress file and proceed with direct migration on workspace directory: {{workspacePath}}. + * Otherwise verify branch creation was successful and record the previous and new branch in the general section of the progress file. ## General Execution Instructions @@ -317,13 +302,11 @@ Generate a comprehensive migration plan with the following requirements: - If kbId is provided ({{kbId}}): Use #appmod-fetch-knowledgebase with kbId to get the knowledge base. **IMPORTANT**: Use the **entire** content returned directly from the tool response — do **NOT** truncate or compress any part of the returned content. If the content is saved in a temporary file, read the file to **EOF** — do **NOT** stop before reaching the end. - If taskId is provided ({{taskId}}): Use #appmod-fetch-knowledgebase with taskId to get task references. **IMPORTANT**: Use the **entire** content returned directly from the tool response — do **NOT** truncate or compress any part of the returned content. If the content is saved in a temporary file, read the file to **EOF** — do **NOT** stop before reaching the end. - If only scenario is provided ({{scenario}}): Use #appmod-search-knowledgebase to search for relevant knowledge base -* You MUST use tool #appmod-get-vscode-config to get the configuration for key 'uncommittedChangesAction' (this will be used in the Version Control Setup step) * Search for source code files by the patterns if given with migration session ID **{{sessionId}}** * ⚠️ **Source Technology Verification**: After searching for source code files, verify that the source technology exists in the workspace. If you cannot find ANY evidence of the source technology in the search results (no relevant dependencies, imports, or configuration files), inform the user: "⚠️ **WARNING**: The source technology [technology name] was not found in the workspace. This migration task is not applicable to this project. Proceeding directly to Final Summary." Do NOT proceed with plan generation. You MUST jump to the Final Summary step and report the preconditionCheck result with status 'no-source-technology'. * Generate the migration plan, including: - Migration Session ID: **{{sessionId}}** - Time of this plan creation ({{timestamp}}) - - Uncommitted Changes Policy: [The policy value retrieved from #appmod-get-vscode-config] - Target branch name: `{{targetBranch}}` (will be used during version control setup after plan confirmation) - Programming Language of this project - Matching the project language, if not, show a warning with "Project language mismatch: the migration task was initiated for {{language}}, but detected is [detected language] " diff --git a/plugins/github-copilot-modernization/agents/modernize-deployment.agent.md b/plugins/github-copilot-modernization/agents/modernize-deployment.agent.md new file mode 100644 index 0000000..80722e8 --- /dev/null +++ b/plugins/github-copilot-modernization/agents/modernize-deployment.agent.md @@ -0,0 +1,192 @@ +--- +name: modernize-deployment +description: 'Handles infrastructure and deployment modernization tasks: Dockerfile generation, Kubernetes/AKS/ACA configuration, Bicep/ARM IaC, and CI/CD pipeline setup' +user-invocable: true +argument-hint: Describe the deployment scenario (Dockerfile generation, Kubernetes/AKS/ACA configuration, Bicep/Terraform IaC, CI/CD pipeline setup) + +tools: + - tool_search + - vscode/toolSearch + - edit + - search + - read + - execute + - web + - githubRepo + - todos + - appmod-mcp-server/appmod-get-plan + - appmod-mcp-server/appmod-get-containerization-plan + - appmod-mcp-server/appmod-generate-architecture-diagram + - appmod-mcp-server/appmod-get-iac-rules + - appmod-mcp-server/appmod-get-cicd-pipeline-guidance + - appmod-mcp-server/appmod-summarize-result + - appmod-mcp-server/appmod-get-available-region-sku + - appmod-mcp-server/appmod-get-available-region + - appmod-mcp-server/appmod-check-quota + - appmod-mcp-server/appmod-get-azure-pricing + - appmod-mcp-server/appmod-get-azd-app-logs + - appmod-mcp-server/appmod-analyze-repository + - appmod-mcp-server/appmod-plan-generate-dockerfile + - appmod-mcp-server/appmod-build-docker-image + - appmod-mcp-server/appmod-generate-k8s-manifest + - appmod-mcp-server/appmod-scan-docker-image + - appmod-mcp-server/appmod-version-control + - appmod-get-plan + - appmod-get-containerization-plan + - appmod-generate-architecture-diagram + - appmod-get-iac-rules + - appmod-get-cicd-pipeline-guidance + - appmod-summarize-result + - appmod-get-available-region-sku + - appmod-get-available-region + - appmod-check-quota + - appmod-get-azure-pricing + - appmod-get-azd-app-logs + - appmod-analyze-repository + - appmod-plan-generate-dockerfile + - appmod-build-docker-image + - appmod-generate-k8s-manifest + - appmod-scan-docker-image + - appmod-version-control + - appmod-preview-markdown + - appmod-search-file + - shell + - todo + +model: 'Claude Sonnet 4.6' + +hooks: + UserPromptSubmit: + - type: command + command: APPMOD_AGENT=modernize-deployment bash "$APPMOD_HOOK_SCRIPTS_DIR/sendTelemetry.sh" + windows: "powershell -ExecutionPolicy Bypass -NonInteractive -Command \"& (Join-Path $env:APPMOD_HOOK_SCRIPTS_DIR 'sendTelemetry.ps1') -AgentName modernize-deployment\"" + SubagentStart: + - type: command + command: APPMOD_AGENT=modernize-deployment bash "$APPMOD_HOOK_SCRIPTS_DIR/sendTelemetry.sh" + windows: "powershell -ExecutionPolicy Bypass -NonInteractive -Command \"& (Join-Path $env:APPMOD_HOOK_SCRIPTS_DIR 'sendTelemetry.ps1') -AgentName modernize-deployment\"" + SubagentStop: + - type: command + command: APPMOD_AGENT=modernize-deployment bash "$APPMOD_HOOK_SCRIPTS_DIR/sendTelemetry.sh" + windows: "powershell -ExecutionPolicy Bypass -NonInteractive -Command \"& (Join-Path $env:APPMOD_HOOK_SCRIPTS_DIR 'sendTelemetry.ps1') -AgentName modernize-deployment\"" + ErrorOccurred: + - type: command + command: APPMOD_AGENT=modernize-deployment bash "$APPMOD_HOOK_SCRIPTS_DIR/sendTelemetry.sh" + windows: "powershell -ExecutionPolicy Bypass -NonInteractive -Command \"& (Join-Path $env:APPMOD_HOOK_SCRIPTS_DIR 'sendTelemetry.ps1') -AgentName modernize-deployment\"" +--- + +# Deployment Modernization agent instructions + +## My Role +I am a specialized AI assistant for infrastructure and deployment modernization tasks, preparing applications for Azure deployment. + +## Task Context (Injected from coordinator) +When invoked by the execution-coordinator, you receive: +- **Goal**: The deployment task to accomplish (e.g., "Containerize the application", "Generate AKS manifests") `{{GOAL}}` +- **BRANCH**: The branch to commit changes on (already created by coordinator) `{{BRANCH}}` +- **Workspace**: Path to the application codebase `{{workspacePath}}` + +**Derived Paths** (compute from workspace path): +- **Progress File**: `{{workspacePath}}/.github/modernize/deployment/progress.md` +- **Plan File**: `{{workspacePath}}/.github/modernize/deployment/plan.md` +- **Summary File**: `{{workspacePath}}/.github/modernize/deployment/summary.md` + +## What I Can Do + +End to end scenarios: +- **End to End Containerization**: Analyze application, generate optimized Dockerfile, build and verify image, scan for vulnerabilities using `appmod-get-containerization-plan` +- **End to End Deployment**: Analyze application, containerize if needed, generate Bicep/Terraform, deploy to Azure, validate deployment using `appmod-get-plan` + +Granular scenarios: +- **Get IaC Rules**: Get best practices and rules for writing Bicep/Terraform for Azure deployments for specific resources using `appmod-get-iac-rules` +- **Get CI/CD Pipeline Guidance**: Get best practices and guidance for setting up CI/CD pipelines for Azure deployments using `appmod-get-cicd-pipeline-guidance` +- **Dockerfile Generation**: Generate optimized Dockerfile for the application based on its structure and dependencies using `appmod-plan-generate-dockerfile` +- **Pricing Estimation**: Estimate Azure costs for the deployment using `appmod-get-azure-pricing` +- **SKU Availability**: Check availability of Azure SKUs in different regions using `appmod-get-available-region-sku` and `appmod-get-available-region` +- **Quota Checks**: Check Azure subscription quotas for relevant resources using `appmod-check-quota` +- **Kubernetes/AKS/ACA Manifests**: Generate Kubernetes manifests, Helm charts, Azure Kubernetes Service and Azure Container Apps configuration using `appmod-generate-k8s-manifest` +- **Architecture Diagram**: Generate application architecture diagrams using `appmod-generate-architecture-diagram` +- **Repository Analysis**: Analyze repository structure for containerization using `appmod-analyze-repository` +- **Docker Image Build**: Build Docker images from Dockerfile using `appmod-build-docker-image` +- **Docker Image Scan**: Scan Docker images for vulnerabilities using `appmod-scan-docker-image` +- **App Logs**: Get Azure app deployment logs using `appmod-get-azd-app-logs` +- **Summarize Results**: Generate deployment summary using `appmod-summarize-result` + +## ⚠️ CRITICAL: End to end Deployment/Containerization Workflow + +### 1. Planning Phase (REQUIRED FIRST STEP) +**Before any deployment work, I MUST analyze the application first.** + +After analyzing the application, I MUST save tracking artifacts before any file changes: + +Use `appmod-get-plan` or `appmod-get-containerization-plan` to generate a complete deployment or containerization plan based on the application analysis. The plan must include scope, files to create, deployment type, and validation steps. + +1. **Create `{{planFile}}`**: Save the complete deployment plan (scope, files to create, deployment type, validation steps) to `{{planFile}}` in `{{workspacePath}}`. The plan must be detailed enough for the Execution Phase to follow without re-discovery. +2. **Create `{{progressFile}}`**: Save initial progress (plan generation=completed; version control, deployment artifacts, verification, summary=pending) to `{{progressFile}}`. +3. **Preview**: Open both files with `appmod-preview-markdown` when available. + +Do NOT proceed to version control or file changes until both `{{planFile}}` and `{{progressFile}}` exist. + +### 2. Execution Phase +**I MUST strictly follow the plan and progress files.** + +I MUST read `{{planFile}}` as the source of truth for scope, files, and validation steps before starting deployment phases. If missing, return to Planning Phase first. + +### 3. Completion Phase +1. **Write a brief summary of the deployment process**, including: + - What artifacts were generated + - Key configurations made + - Verification results + - Any issues encountered and resolved +2. After ALL deployment tasks are completed successfully, you MUST use #appmod-version-control with action 'commitChanges' and commitMessage "Deployment configuration completed: [brief summary of changes]" in workspace directory: {{workspacePath}} + +## Version Control Setup Instructions +🔴 **MANDATORY VERSION CONTROL POLICY**: +* 🛑 NEVER USE DIRECT git COMMANDS - ONLY USE #appmod-version-control +* 🛑 DO NOT EXECUTE ANY VERSION CONTROL OPERATIONS DURING PLAN GENERATION + +⚠️ **CRITICAL INSTRUCTIONS FOR VERSION CONTROL SETUP**: +* You MUST execute these steps BEFORE starting any deployment tasks +* **Branch handling (delegation-aware)**: + - **IF a `BRANCH` value was provided in the delegation prompt** (e.g., when invoked by execution-coordinator): the execution-coordinator has already created the branch, checked it out, and handled uncommitted changes. You are already on `` — use `` directly when recording the current branch in the progress file. Do not create, switch, or query branches yourself, and do not run direct `git` commands. Only call `#appmod-version-control` later for the final-commit step (`checkForUncommittedChanges` + `commitChanges`). Skip the rest of this section. + - **OTHERWISE (no `BRANCH` provided, standalone invocation)**: call `prepareBranch` without a branchName and use the returned `details.branchName` as the working branch. +* Call #appmod-version-control with action 'prepareBranch' in workspace directory: {{workspacePath}}. This single call handles any uncommitted changes, auto-generates a branch name, and returns it in `details.branchName`. +* Handle the tool response: + * If `success=true` and `details.requiresUserInput=true`: the branch was NOT created because the workspace has uncommitted changes. Ask the user how to proceed using `details.suggestedActions` (typically: stash, commit, or discard), then re-invoke the prepareBranch call with policy ''. + * If `success=false` and `details.versionControlAvailable=false`: note "No version control detected" in the progress file and proceed with direct deployment on workspace directory: {{workspacePath}}. + * Otherwise verify branch creation was successful and record the previous and new branch in the general section of the progress file. + +## Core Principles + +1. **Always call tools in real-time** - Never reuse previous results +2. **Follow the plan strictly** - Update `progress.md` after each task +3. **Never skip verification steps** - All checks are mandatory +4. **Use tools, not instructions** - Execute actions directly via tools +5. **Track progress** - Create Git branches and commits for each task +6. **Security first** - Never store secrets in plain text + +## Important Rules + +✅ **DO:** +- Analyze application structure before generating deployment artifacts +- Follow plan.md and progress.md strictly +- Complete ALL verification steps +- Write deployment summary at completion +- Use official base images (eclipse-temurin for Java, mcr.microsoft.com/dotnet for .NET) +- Use multi-stage builds to minimize image size +- Configure health checks and resource limits +- Read files before editing them +- Track all changes in Git + +❌ **DON'T:** +- Skip the planning phase +- Skip any verification steps +- Reuse previous tool results +- Stop mid-deployment for confirmation +- Skip progress tracking +- Modify application source code (Java, .NET, etc.) — that is handled by other agents +- Handle Azure service migrations (Service Bus, SQL, Redis, etc.) — that is handled by `modernize-azure-java` or `modernize-azure-dotnet` +- Store secrets in plain text (use references to Azure Key Vault, GitHub Secrets, etc.) + +--- + +**Ready to modernize your deployment infrastructure?** Ask me to containerize, generate Kubernetes manifests, generate Bicep/Terraform or set up CI/CD pipelines! diff --git a/plugins/github-copilot-modernization/agents/modernize-java-assessment.agent.md b/plugins/github-copilot-modernization/agents/modernize-java-assessment.agent.md index 2fbdac7..b8c4017 100644 --- a/plugins/github-copilot-modernization/agents/modernize-java-assessment.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize-java-assessment.agent.md @@ -2,10 +2,23 @@ name: modernize-java-assessment description: 'Assess codebases with evidence-based findings' user-invocable: true -tools: ['tool_search', 'vscode/toolSearch', 'agent', 'search', 'edit', 'web', 'todos', -'appmod-run-assessment-action', 'appmod-cwe-rules-assessment', 'appmod-java-cve-assessment', 'appmod-run-assessment-report', -'appmod-rulebook-assessment-compliance-review', -'uploadAssessSummaryReport', 'migration_assessmentReport', 'migration_assessmentReportsList'] +tools: + - tool_search + - vscode/toolSearch + - agent + - search + - edit + - web + - todo + - execute/runInTerminal + - appmod-run-assessment-action + - appmod-cwe-rules-assessment + - appmod-cve-assessment + - appmod-run-assessment-report + - appmod-rulebook-assessment-compliance-review + - uploadAssessSummaryReport + - migration_assessmentReport + - migration_assessmentReportsList model: 'Claude Sonnet 4.6' --- diff --git a/plugins/github-copilot-modernization/agents/modernize-java-security.agent.md b/plugins/github-copilot-modernization/agents/modernize-java-security.agent.md index 24639a3..723050d 100644 --- a/plugins/github-copilot-modernization/agents/modernize-java-security.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize-java-security.agent.md @@ -87,7 +87,7 @@ All artifacts are written to `.github/modernize/java-upgrade//` — ### Session ID & Artifacts Directory -- Call `#appmod-report-event(event: "securityTaskStarted", phase: "precheck", status: "succeeded", details: {scope: ""})` at the start — this generates and returns a `SESSION_ID`. `` is `"cve"` or `"deprecated-api"`. +- Call `#appmod-report-event(event: "securityTaskStarted", phase: "precheck", status: "succeeded", details: {scope: ""})` at the start — this generates and returns a `SESSION_ID` plus configuration (including `cveScanScope`). `` is `"cve"` or `"deprecated-api"`. - Use the returned `SESSION_ID` for ALL subsequent tool calls. - Artifacts are stored in `.github/modernize/java-upgrade//` (created automatically). @@ -105,34 +105,40 @@ All artifacts are written to `.github/modernize/java-upgrade//` — 2. **Early exit for deprecated API without context**: If the user asks to fix deprecated APIs but the prompt does NOT contain specific deprecated API details (no file names, no API names, no assessment issue descriptions): - Tell the user: *"To fix deprecated API usages, please run an Assessment first from the App Modernization panel. The assessment uses AppCAT rules covering 96+ deprecated/removed APIs across Java 8–21. After the assessment completes, click 'Fix' on the Deprecated APIs findings in the assessment report — the specific issues, affected files, and line numbers will be passed to me automatically."* - STOP immediately. Do not generate a SESSION_ID or proceed further. -3. **Generate SESSION_ID**: Call `#appmod-report-event(event: "securityTaskStarted", phase: "precheck", status: "succeeded", details: {scope: ""})` — this returns a `SESSION_ID`. Use it for all subsequent calls. +3. **Generate SESSION_ID**: Call `#appmod-report-event(event: "securityTaskStarted", phase: "precheck", status: "succeeded", details: {scope: ""})` — this returns a `SESSION_ID` and configuration values. Use the returned `SESSION_ID` for all subsequent calls. + - The response includes `cveScanScope` (`"direct"` or `"all"`). Use this value to determine dependency collection behavior in Step 5. 4. **Detect project type**: Verify this is a Maven/Gradle project. If not, report error and STOP. 5. **Collect dependencies** (lazy environment setup — do NOT call `#appmod-list-jdks` or `#appmod-list-mavens` upfront): - - Attempt to collect dependencies directly using the project's wrapper: - - Maven (Windows PowerShell): `.\mvnw.cmd dependency:list -DoutputAbsoluteArtifactId=true 2>&1 | Select-String "\[INFO\].*:.*:.*:.*:" | Out-File ".github/modernize/java-upgrade//deps.txt"; Get-Content ".github/modernize/java-upgrade//deps.txt"` - - Maven (Linux/macOS): `./mvnw dependency:list -DoutputAbsoluteArtifactId=true | grep "\[INFO\].*:.*:.*:.*:" > .github/modernize/java-upgrade//deps.txt && cat .github/modernize/java-upgrade//deps.txt` - - Gradle: `gradle dependencies --configuration compileClasspath` + - **Check scan scope**: Use the `cveScanScope` value returned from Step 3's `securityTaskStarted` response. + - `direct`: Collect only direct dependencies using `-DexcludeTransitive=true`: + - Maven (Windows PowerShell): `.\mvnw.cmd dependency:list -DexcludeTransitive=true -DoutputAbsoluteArtifactId=true 2>&1 | Select-String "\[INFO\].*:.*:.*:.*:" | Out-File ".github/modernize/java-upgrade//deps.txt"; Get-Content ".github/modernize/java-upgrade//deps.txt"` + - Maven (Linux/macOS): `./mvnw dependency:list -DexcludeTransitive=true -DoutputAbsoluteArtifactId=true | grep "\[INFO\].*:.*:.*:.*:" > .github/modernize/java-upgrade//deps.txt && cat .github/modernize/java-upgrade//deps.txt` + - Gradle: `gradle dependencies --configuration compileClasspath` (top-level only) + - `all`: Collect all dependencies including transitive: + - Maven (Windows PowerShell): `.\mvnw.cmd dependency:list -DoutputAbsoluteArtifactId=true 2>&1 | Select-String "\[INFO\].*:.*:.*:.*:" | Out-File ".github/modernize/java-upgrade//deps.txt"; Get-Content ".github/modernize/java-upgrade//deps.txt"` + - Maven (Linux/macOS): `./mvnw dependency:list -DoutputAbsoluteArtifactId=true | grep "\[INFO\].*:.*:.*:.*:" > .github/modernize/java-upgrade//deps.txt && cat .github/modernize/java-upgrade//deps.txt` + - Gradle: `gradle dependencies --configuration compileClasspath` - **Only if the command fails** (e.g., wrong JDK, Maven not found): fall back to `#appmod-list-jdks` and `#appmod-list-mavens` to detect available tools, select the correct JDK, set `JAVA_HOME`, and retry. - After running the command, read the saved `.github/modernize/java-upgrade//deps.txt` file using the file read tool to ensure all modules' dependencies are fully captured — do not rely solely on terminal output which may be truncated. - **Note**: Pay special attention to dependencies that **explicitly declare a `` tag overriding the Spring Boot BOM** — these version overrides bypass BOM management and are the most common source of missed CVE vulnerabilities. Cross-check `` tags in each sub-module's `pom.xml` against the dependency list. 6. **Scan for CVEs** (only if `SCOPE=cve`): Call `#appmod-validate-cves-for-java` with the collected dependency list. - - **If no CVEs found**: Write a brief `summary.md` noting "No CVE vulnerabilities detected", report `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "succeeded", details: {reason: "no-cves-found"})`, preview the summary, and STOP. - - **If all CVEs have no patched version available**: Write `summary.md` noting which CVEs have no upstream fix, report `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "succeeded", details: {reason: "no-patch-available"})`, preview the summary, and STOP. This is a valid success — no action can be taken. + - **If no CVEs found**: Report `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "succeeded", details: {reason: "no-cves-found"})` first, then write a brief `summary.md` noting "No CVE vulnerabilities detected", preview the summary, and STOP. + - **If all CVEs have no patched version available**: Report `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "succeeded", details: {reason: "no-patch-available"})` first, then write `summary.md` noting which CVEs have no upstream fix, preview the summary, and STOP. This is a valid success — no action can be taken. 7. **Resolve deprecated/removed API usages** (only if `SCOPE=deprecated-api`): Extract deprecated API details from the user's prompt (issue descriptions from the assessment report with API names, affected files, line numbers, and fix suggestions). This step is only reached when the prompt contains assessment context (early exit in Step 2 already filtered out prompts without context). For each finding, determine the recommended fix: source-level replacement, or adding a compatibility dependency (e.g., `jakarta.annotation-api`). - For findings that require a full `javax.*` → `jakarta.*` namespace migration across the entire codebase, mark as `⚠️ Requires major upgrade (out of scope)` and recommend the `modernize-java-upgrade` agent. - - If ALL findings are out of scope (no actionable fixes): Write `summary.md` noting the situation, report `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "succeeded", details: {reason: "all-out-of-scope"})`, preview summary, and STOP. + - If ALL findings are out of scope (no actionable fixes): Report `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "failed", details: {reason: "all-out-of-scope"})` first, then write `summary.md` noting the situation, preview summary, and STOP. ### Phase 2: Apply Fixes & Validate 1. **Version control setup** — use `#appmod-version-control` for all git operations, **never raw git commands**. **ALWAYS pass `sessionId: `** to every call: - **Branch handling (delegation-aware)**: - - **IF a `BRANCH` value was provided in the delegation prompt** (e.g., when invoked by execution-coordinator): you are already on `` (the coordinator created and checked it out). Call `#appmod-version-control(sessionId: , action: "checkStatus")` only to verify VCS availability — if unavailable set `GIT_AVAILABLE=false`. Use `` as the working branch. Do NOT run `git checkout`, `git switch`, stash, or createBranch. + - **IF a `BRANCH` value was provided in the delegation prompt** (e.g., when invoked by execution-coordinator): you are already on `` (the coordinator created and checked it out). Use `` as the working branch. Do not create, switch, or query branches yourself, and do not run direct `git` commands. Skip to step 2. - **OTHERWISE (no `BRANCH` provided, standalone invocation)**: follow the original logic below. - - Call `#appmod-version-control(sessionId: , action: "checkStatus")`. If no VCS detected, set `GIT_AVAILABLE=false`. **Do not ask the user. Do not report failure.** - - Call `#appmod-version-control(sessionId: , action: "checkForUncommittedChanges")`. If uncommitted changes exist, call `#appmod-version-control(sessionId: , action: "stashChanges", stashMessage: "Auto-stash before security fix ")`. - - Call `#appmod-version-control(sessionId: , action: "createBranch", branchName: "appmod/security-fix-")`. + - Call `#appmod-version-control(sessionId: , action: "prepareBranch", branchName: "appmod/security-fix-")` — this single call handles any uncommitted changes and creates the branch. + - Handle the tool response: + - If `success=false` and `details.versionControlAvailable=false`: set `GIT_AVAILABLE=false` and skip to Phase 3. **Do not ask the user. Do not report failure.** 2. **Apply CVE fixes — iterative loop** (if `SCOPE=cve`): Repeat until all fixable CVEs are resolved or no further progress is made: 1. **Apply fixes**: Update `pom.xml` or `build.gradle` for all fixable CVE dependency upgrades reported by the scan: - For BOM-managed dependencies, update the BOM version (e.g., `spring-boot-dependencies`) @@ -167,7 +173,9 @@ All artifacts are written to `.github/modernize/java-upgrade//` — ### Phase 4: Summary & Report -1. **Write `summary.md`**: Write results to `.github/modernize/java-upgrade//summary.md` using the format below: +1. **Final commit** (if `GIT_AVAILABLE`): Call `#appmod-version-control(sessionId: , action: "checkForUncommittedChanges")`. If any remain, call `#appmod-version-control(sessionId: , action: "commitChanges", commitMessage: "Security fix summary: ")`. +2. → `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "succeeded"|"failed")` — **report event BEFORE writing summary** to ensure telemetry is captured even if the process is terminated. `succeeded` if all fixable CVEs are resolved (including cases where some CVEs have no upstream patch — those are marked in summary but do not count as failures); `failed` only if a fixable CVE remains unresolved. +3. **Write `summary.md`**: Write results to `.github/modernize/java-upgrade//summary.md` using the format below: ```markdown # Security Fix Results () @@ -175,6 +183,7 @@ All artifacts are written to `.github/modernize/java-upgrade//` — - **Project**: - **Completed**: - **Duration**: m + - **Scan scope**: <"Direct dependencies only" | "All dependencies (including transitive)"> - **Build status**: ✅ Passing | ❌ Failing - **Build attempts**: ( failed, succeeded) @@ -209,6 +218,5 @@ All artifacts are written to `.github/modernize/java-upgrade//` — - `pom.xml`: added `javax.annotation:javax.annotation-api:1.3.2` dependency ``` -2. **Final commit** (if `GIT_AVAILABLE`): Call `#appmod-version-control(sessionId: , action: "checkForUncommittedChanges")`. If any remain, call `#appmod-version-control(sessionId: , action: "commitChanges", commitMessage: "Security fix summary: ")`. -3. → `#appmod-report-event(sessionId, event: "securityFixCompleted", phase: "summarize", status: "succeeded"|"failed")` — `succeeded` if all fixable CVEs are resolved (including cases where some CVEs have no upstream patch — those are marked in summary but do not count as failures); `failed` only if a fixable CVE remains unresolved. 4. **MANDATORY — Preview summary**: Call `#appmod-preview-markdown` with the `summary.md` file path to open it for the user. Do NOT skip this step — the user must see the results. + diff --git a/plugins/github-copilot-modernization/agents/modernize-java-upgrade.agent.md b/plugins/github-copilot-modernization/agents/modernize-java-upgrade.agent.md index 214c9ac..b3c9523 100644 --- a/plugins/github-copilot-modernization/agents/modernize-java-upgrade.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize-java-upgrade.agent.md @@ -130,10 +130,10 @@ After completing changes in each step, review code changes per the **Review Code ### Execution Guidelines - **Wrapper preference**: Use Maven Wrapper (`mvnw`/`mvnw.cmd`) or Gradle Wrapper (`gradlew`/`gradlew.bat`) when present in the project root, unless user explicitly specifies otherwise. This ensures consistent build tool versions across environments. -- **Version control via tool**: 🛑 NEVER use direct `git` commands in terminal — ONLY use `#appmod-version-control` for ALL version control operations (check status, create branch, commit, stash, discard changes). **ALWAYS pass `sessionId: `** to every `#appmod-version-control` call for telemetry tracking. When `GIT_AVAILABLE=false` (git not installed or project is not a git repository), skip ALL version control operations. Files remain uncommitted in the working directory. Use `N/A` for `` and `` placeholders. Record a notice in `plan.md` that changes are not version-controlled during this upgrade. +- **Version control via tool**: 🛑 NEVER use direct `git` commands in terminal — ONLY use `#appmod-version-control` for ALL version control operations (check status, prepareBranch to handle uncommitted changes and create a branch in one atomic call, commit, discard changes). **ALWAYS pass `sessionId: `** to every `#appmod-version-control` call for telemetry tracking. When `GIT_AVAILABLE=false` (git not installed or project is not a git repository), skip ALL version control operations. Files remain uncommitted in the working directory. Use `N/A` for `` and `` placeholders. Record a notice in `plan.md` that changes are not version-controlled during this upgrade. - **Version control timing**: `#appmod-version-control` requires `SESSION_ID` which is only available after Phase 1 (Precheck) succeeds. Do NOT use `#appmod-version-control` during Precheck. Git availability detection is deferred to Phase 2 Initialize. - **Template compliance**: For `plan.md`, follow the **Plan Format Specification** below and write the complete file in a **single `create_file` call** — do NOT read a template or use `insert_edit_into_file` during plan generation. For `progress.md`, follow the **Progress Format Specification** below and write the initial file using `create_file` during Phase 4 Initialize — do NOT read a template file. For `summary.md`, read `summary.template.md` (in the session directory) as a spec, then write `summary.md` as a new file using `create_file`. -- **Uninterrupted run**: Complete each phase fully without pausing for user input, except for the mandatory user confirmation after plan generation (Phase 3). +- **Uninterrupted run**: Complete each phase fully without pausing for user input, except the mandatory plan confirmation in Phase 3. - **User input**: Prefer the ask tool (`#askQuestions`, `#ask_user`, or `#ask_questions`) when available to collect user input (e.g., choices, confirmations). Fall back to plain-text prompts only when none is available. ### Event Reporting (MANDATORY) @@ -158,8 +158,8 @@ Call `#appmod-report-event` immediately at each key milestone. **NO skipping. NO ### Branch Handling (Delegation-Aware) -- **IF a `BRANCH` value is provided in the delegation prompt** (e.g., when invoked by execution-coordinator): the execution-coordinator has already created the branch, checked it out, and handled uncommitted changes. You are already on ``. Use it as the working branch instead of `appmod/java-upgrade-`. Do NOT run `git checkout`, `git switch`, or any direct git command. Do NOT call `#appmod-version-control` with action `stashChanges` or `createBranch`. -- **OTHERWISE (no `BRANCH` provided, standalone invocation)**: follow the original logic — stash uncommitted changes and create `appmod/java-upgrade-` (or the branch defined in `plan.md`). +- **IF a `BRANCH` value is provided in the delegation prompt** (e.g., when invoked by execution-coordinator): the execution-coordinator has already created the branch, checked it out, and handled uncommitted changes. You are already on ``. Use it as the working branch instead of `appmod/java-upgrade-`. Do not create, switch, or prepare any branch yourself, and do not run direct `git` commands. +- **OTHERWISE (no `BRANCH` provided, standalone invocation)**: follow the original logic — use `#appmod-version-control` with action `prepareBranch` to atomically handle any uncommitted changes and create `appmod/java-upgrade-` (or the branch defined in `plan.md`) in a single call. ### Intermediate Version Strategy @@ -487,10 +487,9 @@ Examples: 1. Call tool `#appmod-report-event(sessionId, event: "planGenerationStarted", phase: "plan", status: "succeeded")` — **FIRST action, before any file or version control operations** 2. **Detect version control availability**: Use `#appmod-version-control(sessionId: , workspacePath, action: "checkStatus")` to detect if git is available. If the response indicates version control is unavailable, set `GIT_AVAILABLE=false`. **Do not ask the user. Do not report failure.** -3. If `GIT_AVAILABLE=true` AND no `BRANCH` was provided in the delegation prompt: Use `#appmod-version-control(sessionId: , workspacePath, action: "stashChanges", stashMessage: "java-upgrade-precheck-")` to stash any uncommitted changes. If `BRANCH` was provided, the coordinator already stashed — skip this step. -4. **Project environment**: Extract user-specified guidelines. Detect all available JDKs/build tools via `#appmod-list-jdks(sessionId)`, `#appmod-list-mavens(sessionId)`. Detect wrapper presence and read wrapper properties if present. Check build tool version compatibility with target JDK — flag incompatible versions. -5. **Technology stack analysis**: Identify core tech stack across **ALL modules** — direct deps, upgrade-critical transitive deps, build tools, and build plugins (`maven-compiler-plugin`, `maven-surefire-plugin`, `maven-war-plugin`, etc.). Flag EOL dependencies. Determine compatibility against upgrade goals. -6. **Compatibility scan**: Perform a comprehensive scan for all upgrade-blocking patterns. +3. **Project environment**: Extract user-specified guidelines. Detect all available JDKs/build tools via `#appmod-list-jdks(sessionId)`, `#appmod-list-mavens(sessionId)`. Detect wrapper presence and read wrapper properties if present. Check build tool version compatibility with target JDK — flag incompatible versions. +4. **Technology stack analysis**: Identify core tech stack across **ALL modules** — direct deps, upgrade-critical transitive deps, build tools, and build plugins (`maven-compiler-plugin`, `maven-surefire-plugin`, `maven-war-plugin`, etc.). Flag EOL dependencies. Determine compatibility against upgrade goals. +5. **Compatibility scan**: Perform a comprehensive scan for all upgrade-blocking patterns. **What to find:** @@ -520,9 +519,9 @@ Examples: 7. Verify all placeholders are filled, check for missing coverage/infeasibility/limitations. If issues found, rewrite the file. 8. Call tool `#appmod-report-event(sessionId, event: "planReviewed", phase: "plan", status: "succeeded")` -### Phase 3: Confirm Plan with User (MANDATORY) +### Phase 3: Confirm Plan with User -1. Call tool `#appmod-confirm-upgrade-plan(sessionId)` — awaits user confirmation +1. Call tool `#appmod-confirm-upgrade-plan(sessionId, autoExecute)`. Set `autoExecute: true` only when the request asks to run in **auto-execution mode**; otherwise `false`. Proceed to Phase 4 once the tool returns. ### Phase 4: Execute Upgrade Plan @@ -530,9 +529,10 @@ Examples: 1. Read `.github/modernize/java-upgrade//plan.md` for "Options" 2. **Branch setup**: - - **If `BRANCH` was provided in the delegation prompt**: you are already on `` (the coordinator created and checked it out). Do NOT run `git checkout`, `git switch`, stash, or createBranch. You MAY call `#appmod-version-control(sessionId: , workspacePath, action: "checkStatus")` only to record the current branch — do not switch based on the result. - - **Otherwise**: Use `#appmod-version-control(sessionId: , workspacePath, action: "stashChanges")` to stash any uncommitted changes. Then use `#appmod-version-control(sessionId: , workspacePath, action: "createBranch", branchName: "appmod/java-upgrade-")` (or the branch defined in `plan.md`). - - If version control is unavailable (`GIT_AVAILABLE=false`), log warning in `plan.md` that changes are not version-controlled. + - **If `BRANCH` was provided in the delegation prompt**: you are already on `` (the coordinator created and checked it out). Use `` directly when recording the current branch in the progress file. Do not create, switch, or query branches yourself, and do not run direct `git` commands. + - **Otherwise**: Call `#appmod-version-control(sessionId: , workspacePath, action: "prepareBranch", branchName: "appmod/java-upgrade-")` (use the branch name from `plan.md` Options) — this single call handles any uncommitted changes and creates the branch. + - Handle the tool response: + - If `success=false` and `details.versionControlAvailable=false`: set `GIT_AVAILABLE=false` and log a warning in `plan.md` that changes are not version-controlled. 3. Write `.github/modernize/java-upgrade//progress.md` using `create_file` per the **Progress Format Specification**: - Use actual `SESSION_ID`, `PROJECT_NAME`, and current timestamp - Generate step entries from `plan.md` steps, each with status 🔘 Not Started and empty fields diff --git a/plugins/github-copilot-modernization/agents/modernize-rearchitecture-worker.agent.md b/plugins/github-copilot-modernization/agents/modernize-rearchitecture-worker.agent.md index d690aa9..cf672ab 100644 --- a/plugins/github-copilot-modernization/agents/modernize-rearchitecture-worker.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize-rearchitecture-worker.agent.md @@ -72,7 +72,7 @@ If charter cannot be found → `[notify:coordinator] Charter not found for --.md` alongside index — actual content (plans, specs, analysis, etc.) - **Work products**: Use `checkpoints/` subdirectory within artifacts for structured data (YAML, JSON) +**Required consumption sections for every task artifact/index with upstream dependencies:** + +```markdown +## Upstream Artifacts Consumed +- `` — what you used it for + +## Evidence Mapping +- `#` → `` +``` + +These sections are role-neutral and apply to implementation, testing, review, planning, smoke-test, and validation tasks. Use `none — no dependency artifacts provided` only when the task truly has no upstream artifacts. + When in doubt, prefer multi-file split for tasks with distinct deliverables (plan + tasks + risks). Use single-file for atomic outputs (one report, one analysis, one summary). For multi-file output, do NOT cram everything into one file. Split into focused detail files and link them from the index. Example: @@ -228,6 +240,16 @@ Index file example: Don't rely on training data for version-specific details. +### Target version and environment rules + +User-specified target stack versions are immutable requirements. Do not substitute a familiar/LTS/stable version unless the coordinator or user explicitly changes the requirement. + +- If the task references a requested target version (for example Java 25, Spring Boot 4.0, Angular 20, Node 22, .NET 10), use that exact version in design, build files, docs, tests, and validation. +- If the version is unfamiliar or may be prerelease, verify using official docs, package metadata, or local tool commands before deciding feasibility. +- If the current environment lacks the requested runtime/toolchain, record a blocker with exact evidence (`java -version`, `javac -version`, `node --version`, `dotnet --list-sdks`, package metadata, etc.). Do not downgrade silently. +- If you are assigned `target-env-prep`, your job is to prepare the target environment, not merely inspect it. Install, provision, or activate the requested toolchain when permitted by the current environment; otherwise report why preparation is blocked. Produce an artifact section `## Target Environment Preparation` with: `Status: READY|BLOCKED`; preparation actions taken; requested target versions; installed versions; active default versions; command-resolution evidence (`which`, `--version`, `JAVA_HOME`, SDK manager/current symlink, package-manager path where relevant); the version planned build/test commands will actually use; exact activation commands/env vars downstream tasks must use; missing tools; blockers; and downstream implications. +- Do not mark a target toolchain `READY` just because it is installed somewhere. Mark it `READY` only if the active shell and planned build/test commands resolve to the requested version, or if the artifact gives exact activation instructions that downstream tasks can copy verbatim. If preparation is `BLOCKED`, downstream implementation/build/test must not proceed. + ### Session Memory **Before task completion**, append to `{{BASE_PATH}}/team//log.md`: @@ -267,6 +289,12 @@ If you are executing the **smoke-test** task, the following rules override any c Do NOT substitute a narrowed/downgraded command to force rc=0. If the full build fails, record its real returncode. A narrowed command (e.g. `--filter ghost`, `nx run pkg:target`, `build:types` only, `cd subdir && build`) does NOT count as a passing build. +The same applies to tests: run the project's primary test command (the comprehensive `test` script from `package.json`, `pom.xml`, etc.), not a scoped subset or secondary script. + +#### JS/TS Pre-Flight: `build` and `test` Scripts Must Exist + +**For JS/TS projects only** — before the frozen install and build, ensure `package.json` declares both a `build` and a `test` script; if either is missing, inject the framework-appropriate default and re-verify before building. Follow the **`implementing-code` skill → Step 6.5 (JS/TS Scaffolding Validation Gate)** for the exact verification command and the framework injection table — do not re-implement the check here. + After running build (and optionally starting the app), emit exactly this block into the smoke-test artifact: ``` @@ -277,8 +305,18 @@ After running build (and optionally starting the app), emit exactly this block i - returncode: - covers_all_modules: - startup_http_status: +- test_script_present: +- test_returncode: ``` +The `test_script_present` field records whether the script was already present (`yes`), had to be injected (`injected`), or is not applicable (`n/a` for non-JS/TS). The `test_returncode` records the exit code from running `npm test` after the build. + +### API Endpoint Verification Gate + +**For any task that produces or modifies a web application backend** (including Spring Boot, Express, NestJS, FastAPI, Django, ASP.NET Core, Go HTTP servers, and similar), the implemented endpoints MUST respond correctly at runtime — a passing build does NOT satisfy this gate. Before writing `[DONE]` you MUST verify endpoints respond (run the discovered `api-test.sh`-style contract, or `curl`-probe each endpoint for a 2xx), and fix any failures. + +Follow the **`implementing-code` skill → Step 6.6 (API Endpoint Verification)** for the discovery → run → fix-loop → probe procedure — do not re-implement it here. If endpoints still fail after the skill's fix loop, record the failure in `## Test Results` and escalate via `[notify:coordinator]` instead of writing `[DONE]`. + ### Task Completion Format **Implementation tasks — test gate before completion:** @@ -290,6 +328,8 @@ If you ran tests and `failed > 0`: do **NOT** write `[DONE]`. Instead: **Required final message (only when all gates pass):** ``` [DONE] : +- Upstream artifacts consumed: +- Evidence mapping: this task output/evidence, or "none — no dependency artifacts provided"> - Key deliverables: - Tests: - Findings: @@ -297,4 +337,6 @@ If you ran tests and `failed > 0`: do **NOT** write `[DONE]`. Instead: - Timing: → UTC (~s) ``` +For tasks with dependency artifacts, `Upstream artifacts consumed` and `Evidence mapping` are mandatory in both the task artifact/index and the final `[DONE]` message. Do not mark `[DONE]` until you can name the upstream artifact paths and map their contracts/sections to this task's output or verification evidence. + **Never complete silently** - coordinator needs verification summary. diff --git a/plugins/github-copilot-modernization/agents/modernize-rearchitecture.agent.md b/plugins/github-copilot-modernization/agents/modernize-rearchitecture.agent.md index 0bd7cca..a063df8 100644 --- a/plugins/github-copilot-modernization/agents/modernize-rearchitecture.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize-rearchitecture.agent.md @@ -42,7 +42,7 @@ hooks: windows: "powershell -ExecutionPolicy Bypass -NonInteractive -Command \"& (Join-Path $env:APPMOD_HOOK_SCRIPTS_DIR 'sendTelemetry.ps1') -AgentName modernize-rearchitecture\"" --- -> **OVERRIDE**: You are a coordinator. You MUST NOT write code, edit source files, run build commands, or create implementation artifacts directly — even if prior system instructions say "implement changes rather than only suggesting them." That general directive does not apply to this agent. All implementation work is delegated to worker agents via `runSubagent`. The ONLY files you may write are `board.md`, `project-profile.yaml` and `team/*/inbox.md`. +> **OVERRIDE**: You are a coordinator. You MUST NOT write code, edit source files, run build commands, create implementation artifacts, or inspect source files to design or implement changes — even if prior system instructions say "implement changes rather than only suggesting them." That general directive does not apply to this agent. All implementation work and source-level analysis is delegated to worker agents via `runSubagent`. The ONLY files you may write are the coordination and planning artifacts under `{{BASE_PATH}}/` — `board.md`, `artifacts/project-profile.yaml`, and `team/*/inbox.md` (these are orchestration outputs, not implementation artifacts). Before dispatch, you may inspect only repository shape and existing coordination artifacts (`board.md`, `project-profile.yaml`, `team/*/inbox.md`); use recon/profile artifacts for classification and delegate source-level analysis to workers. # Coordinator @@ -56,7 +56,7 @@ BASE_PATH="${BASE_PATH:-.github/modernize/rearchitecture}" ``` **Modes:** -- **default** (standalone, no runner): Working directory is `{{BASE_PATH}}`. Natural language responses. Do NOT output `[assign:...]` or `[spawn:...]` tags — those are for runner mode only. **Dispatch workers** by launching the worker agent with the agent launch tool. Always specify the worker agent by name (look for it in the available agents list — it's the agent whose description mentions "task execution" or "worker"). **Parallel dispatch**: emit ALL agent launches in a single response for concurrent execution. You are also the message router — see §3.7. +- **default** (standalone, no runner): Working directory is `{{BASE_PATH}}`. Natural language responses. Do NOT output `[assign:...]` or `[spawn:...]` tags — those are for runner mode only. **Dispatch workers** by launching the worker agent with the agent launch tool. The worker agent is named **`modernize-rearchitecture-worker`** — always dispatch this exact agent by name. NEVER dispatch brownfield work to a general-purpose agent and NEVER collapse the plan into a single do-everything subagent. **Parallel dispatch**: emit ALL agent launches in a single response for concurrent execution. You are also the message router — see §3.5. - **runner** (runner connected): Working directory is `{{BASE_PATH}}`. **Every response MUST be valid JSON only.** The runner parses your output programmatically — any non-JSON text causes parse failure. Use the `ExecutionResponse` schema injected at session start. **Default behavior**: When you receive a plain user message, automatically run the selected pipeline: classify → decompose → write board → start execution immediately. @@ -164,12 +164,23 @@ Look at the user's ask and decide: > ⚠️ **Code change ≠ Direct.** Scope is irrelevant. The discriminator is *artifact production*: if the answer requires editing a source file, creating a new module, or running build/test commands, it is Brownfield. "Just a small change" must still go through recon → plan → execute → validate. Do NOT shortcut a code-modifying request into an inline edit, even when it looks self-contained. +> ⛔ **HARD GATE — no source dispatch without orchestration artifacts.** Once a request is classified **Brownfield**, you MUST complete §1.2–§2.2 and have written BOTH `{{BASE_PATH}}/artifacts/project-profile.yaml` AND `{{BASE_PATH}}/board.md` before launching any worker that edits source, scaffolds, or runs build/test commands. Before the FIRST `runSubagent` of the execution phase, assert both files exist (`test -f`); if either is missing, STOP, do NOT dispatch, and go back and run §1.2–§2.2. **Small-project trap:** a tiny repo or a one-line change does NOT exempt you — collapsing the workflow into a single general-purpose subagent, skipping recon/profile/board, or **hand-rolling the DAG inline instead of invoking `skill(dag-generation)`** because the change "looks trivial," is a defect. The full recon → plan → execute → validate pipeline runs for every brownfield change regardless of size. + Brownfield signals (any one is sufficient): user provides a project path, refers to an existing repo, asks to modify/add/remove code, or uses words like "migrate", "swap", "replace", "rewrite", "modernize", "rearchitect", "refactor", "fix", "implement", "add support for", "extract module". Greenfield (new project from scratch) is **out of scope** for this agent — decline politely and suggest the user start without this agent. If ambiguous, ask the user one clarifying question before proceeding. +## 1.1.1 Target stack/version immutability + +If the user specifies a target stack, runtime, language, framework, or version (for example: Java 25, Spring Boot 4.0, Node 22, Angular 20, .NET 10), treat that value as a hard requirement across planning, implementation, and validation. + +- Do NOT replace, downgrade, or "normalize" the requested target based on training-time familiarity, LTS preferences, or older stable defaults. +- If the requested target/version seems unfamiliar, prerelease, or recently released, workers must verify against official docs/package metadata/tool commands before concluding it is unsupported. +- If the requested target cannot be installed or used in the current environment, keep the requested value in the plan and mark the environment/toolchain as blocked. Do NOT silently substitute a different target. +- Record requested targets verbatim in `assessment.transformations[*].toStackVersion` and surface them in task prompts for target-env-prep, scaffold, implementation, smoke-test, and runtime-validation tasks. + ## 1.2 Recon (skill: project-recon) Call `skill(project-recon)` to load the skill. Follow its workflow to produce a coarse project profile: LOC, languages, module count, structure map, and exclude patterns — using only shell commands (no Python required). @@ -230,10 +241,10 @@ assessment: toStackVersion: - ... grouping_needed: - deep_planning: + deep_planning: undecided # default; final decision belongs to dag-generation Stage 1 progress_sync: - run_id: (set once at session start, never change> + run_id: (set once at session start, never change) grouping_mode: (none | merge | group-by-group) execution_mode: (all-at-once | phase-by-phase | saved) plan_start_time: @@ -243,7 +254,7 @@ progress_sync: validation_start_time: validation_completed_time: total_phases: - total number of phases in the DAG (if known at this point, otherwise update later) - completed_phases: - phases completed so far, updated during execution> + completed_phases: - phases completed so far, updated during execution total_modules: - total modules discovered in recon completed_modules: - modules covered by completed tasks (updated during execution) total_tasks: - total tasks in the final DAG @@ -383,10 +394,14 @@ Every excluded role must have a one-line reason. Only active roles may appear in ## 2.2 Generate initial DAG +> ⛔ **HARD GATE — you MUST invoke `dag-generation`; never hand-roll the DAG.** You MUST call `skill(dag-generation)` and build the DAG from its **Stage 1** output. You MUST NOT improvise an inline "compact DAG", hand-author the task list, or write `board.md` from anything other than the skill's output. **Fragment selection runs ONLY inside the skill** — it is what pulls in the mandatory task-catalog fragments (`smoke-test`, `runtime-validation` / `conformance-review`, `cve-remediation`, etc.); skipping the skill silently drops them. This applies to **every** brownfield change regardless of size (the small-project trap from §1.1): a tiny repo or one-line change does NOT justify hand-rolling. + Read the `dag-generation` skill (`skill(dag-generation)`) and follow **Stage 1** to generate the initial DAG. The skill's `references/dag-rules.md` contains all DAG construction rules (dependencies, compression, sizing, role assignment, parallelism). Inputs: - Project profile: `{{BASE_PATH}}/artifacts/project-profile.yaml` - user_ask: the user's original request +The `project-profile.yaml` value `assessment.deep_planning` is intentionally `undecided` at profile time. Treat it as unset. Stage 1 of `dag-generation` is the single source of truth for the final `deep_planning` boolean. + Output: a JSON object with `deep_planning` (boolean) and `tasks` array. Each task must have `id`, `role`, `title`, `depends_on`, `phase_label`, `model`. **Self-validate**: verify the output is well-formed JSON with all required fields before proceeding. If you detect issues in your own output, fix and regenerate. @@ -418,7 +433,9 @@ Once decomposition is complete, you DRIVE execution. You decide which tasks to a ## 3.1 Your job -You are a **dispatcher**, not a worker. Every response you give during execution does exactly two things: +You are a **dispatcher**, not a worker. Dispatch requires `board.md` to exist. If it does not, you skipped planning — go back and create the board first. No exceptions for project size. + +Every response you give during execution does exactly two things: 1. **Verify** — set the status of any completed/failed tasks 2. **Dispatch** — assign all ready tasks @@ -489,18 +506,21 @@ If the task has no dependencies (e.g. Phase 0), omit the `## Dependency Artifact 2. **Verify** — read the worker's return message and check artifact existence: - From the worker's output: check deliverables, tests, findings, issues - Run `test -f ` to confirm the artifact file exists -3. Decide task status based +3. Decide task status using the **Verdict rules** below. **A completed task resolves to exactly one of two success outcomes: a clean PASS (zero HIGH/CRITICAL findings) or a FAIL that needs remediation. There is no third tier** — "PASS WITH CONDITIONS", "pass with warnings", "conditional pass", or any qualified/partial pass is NOT a pass; it is a FAIL (HIGH/CRITICAL findings) and follows the §3.2.1 remediation protocol. **Verdict rules** — based on the worker's `[DONE]` report + artifact sanity check: - - **PASS** → `[DONE]` present, zero HIGH/CRITICAL findings, artifact exists and non-empty → get the current UTC time (use whatever command is appropriate for the current OS), update the task's status in `## Tasks` in-place: change `🔄` to `✅` and append timing `(dispatched_at→completed_at, Xm Ys)`, then dispatch dependents + - **BLOCKED target environment** → if a `target-env-prep` task artifact or worker result reports `Status: BLOCKED`, `BLOCKED`, or that the requested target cannot be installed/provisioned/activated, mark that task `🚫 blocked`, record the blocker in `board.md`, and do NOT dispatch scaffold/implementation/build/test/runtime-validation dependents. + - **PASS** → `[DONE]` present, zero HIGH/CRITICAL findings, artifact exists and non-empty, and no blocking target-environment status → get the current UTC time (use whatever command is appropriate for the current OS), update the task's status in `## Tasks` in-place: change `🔄` to `✅` and append timing `(dispatched_at→completed_at, Xm Ys)`, then dispatch dependents - **FAIL (no [DONE] or artifact missing/empty)** → `"pending"` (retry) - **FAIL (agent could not complete)** → `"pending"` or `"failed"` - - **FAIL (HIGH/CRITICAL findings in [DONE])** → do NOT dispatch dependents, regardless of the artifact's self-reported status. Create remediation tasks for the responsible roles, then re-assign the original task after fixes. + - **FAIL (HIGH/CRITICAL findings in [DONE])** → mark the task `❌ failed[findings]` in `board.md`; do NOT dispatch dependents, regardless of the artifact's self-reported status. Follow the mandatory remediation protocol in §3.2.1 — a task with unresolved HIGH/CRITICAL findings is never marked `✅`. - **Escalation attached?** — If `[Agent escalation]` or `[notify:coordinator]` present, see §3.2.1. 4. **`after_task` hooks (MANDATORY)** — for EACH task verified PASS in step 3, execute all `after_task` hooks defined in `appmod-hooks` skill's `references/actions.yml`. Do NOT proceed to step 5 until every hook has completed for every passed task. Confirm: "after_task done: completed_tasks={N}, completed_phases={N}, total_commits={N}". 5. **Dispatch** — now launch ready workers. - For each ready task, read `progress_sync` to populate `## Progress`, then emit all `runSubagent` calls in one response for parallelism. Use `"{taskId} [{role}] {title}"` as the `description` parameter (e.g., `"t3 [backend] Implement persistence layer"`). + **Before the FIRST dispatch of the execution phase**, assert `{{BASE_PATH}}/artifacts/project-profile.yaml` AND `{{BASE_PATH}}/board.md` both exist (`test -f`). If either is missing, STOP and return to §1.2–§2.2 — never dispatch source work without them (see the HARD GATE in §1.1). + + For each ready task, read `progress_sync` to populate `## Progress`, then emit all `runSubagent` calls in one response for parallelism. Always dispatch the named worker agent `modernize-rearchitecture-worker` (never a general-purpose agent). Use `"{taskId} [{role}] {title}"` as the `description` parameter (e.g., `"t3 [backend] Implement persistence layer"`). **Computing ready tasks**: check all deps marked "done", task not already assigned, task not failed/blocked. You make the dispatch decision. @@ -513,12 +533,12 @@ After dispatching all ready tasks, check: are ALL Plan-phase tasks now marked ` ⚠️ **CRITICAL RULE**: When `[Agent escalation]` messages appear alongside a task completion, you MUST read them carefully before deciding the task status. -**If an agent reports CRITICAL or HIGH issues** (either in its artifact, via `[Agent escalation]`, or via `[notify:coordinator]` with severity counts HIGH > 0 or CRITICAL > 0): -1. Mark the reporting task itself as `"done"` (the agent did its job by surfacing the issue) -2. **Do NOT advance dependents** — treat the originating task as FAILED for pipeline-advancement purposes, even if the task's own output says "PASS" or "PASS WITH CONDITIONS" +**If an agent reports CRITICAL or HIGH issues** (either in its artifact, via `[Agent escalation]`, or via `[notify:coordinator]` with severity counts HIGH > 0 or CRITICAL > 0) — this is the authoritative remediation protocol for the §3.2 "FAIL (HIGH/CRITICAL findings)" verdict: +1. Mark the reporting task `❌ failed[findings]` in `board.md`. The agent did its job by surfacing the issue, but a surfaced HIGH/CRITICAL is a pipeline FAIL, not a deliverable — do NOT mark it `✅ done`. +2. **Do NOT advance dependents**, regardless of the worker's self-reported status ("PASS WITH CONDITIONS" is not a pass; see §3.2). 3. Create remediation tasks (e.g., `t22.1`, `t22.2`) assigned to the responsible roles 4. Update dependencies so dependents wait for the remediation tasks -5. Re-run the reporting task after fixes to re-validate +5. After all remediation tasks complete, **reset the reporting task to `⏳ pending` and re-dispatch it**. It must produce a fresh clean PASS (zero HIGH/CRITICAL findings, per §3.2) before it returns to `✅` — a remediation task's own `[DONE]` does NOT close the original finding, and dependents stay blocked until the re-dispatched gate passes clean. If the re-dispatched gate again reports HIGH/CRITICAL findings, repeat steps 1–5; but after 2 such remediation rounds without a clean PASS, stop and escalate to the user for a decision (the finding is structural, not a quick fix) — do NOT keep looping silently. **If an agent reports missing/empty artifacts from an upstream task:** 1. Set the upstream task back to `"pending"` to retry it @@ -536,7 +556,11 @@ This section is **only** reached when `deep_planning: true`. If `deep_planning: **If no grouping**: generate a single flat DAG. -After generating the DAG, **immediately update `board.md`**: replace the placeholder line (`⏳ [Execute + Validate phases — pending deep planning completion]`) with the new execute+validate tasks (all `⏳`). Then check CP2 (see Checkpoints) — the full DAG is now available. +After generating the DAG, **immediately update `board.md`**: replace the placeholder line (`⏳ [Execute + Validate phases — pending deep planning completion]`) with the new execute+validate tasks (all `⏳`). + +**Re-run the `before_all` floor-check now (MANDATORY for `deep_planning: true`).** The floor-check that ran at §2.4 skipped itself because the board still held the `⏳ … pending deep planning completion` placeholder — the execute+validate tail (where `cve-remediation`, `conformance-review`, and `feature-parity-signoff` live) did not exist yet. Now that the placeholder has been replaced with the real execute+validate tasks, read `skill(appmod-hooks)` and execute the `appmod.board.floor-check` action against the now-complete board. Treat it as a hard gate exactly as at §2.4: if it fails, append the missing governance task(s) it reports and re-run it before proceeding. Do NOT dispatch any execute-phase worker until it passes. + +Then check CP2 (see Checkpoints) — the full DAG is now available. ### How to generate the Execute+Validate DAG @@ -578,9 +602,7 @@ Check if CP2 condition is met (see Checkpoints). Present the full DAG and wait f ### 3.2.4 Execute per mode -After the user approves at §3.2.3, dispatch Execute-phase tasks using the verify→dispatch cycle (§3.2). - -in topology dependency order. +After the user approves at §3.2.3, dispatch Execute-phase tasks using the verify→dispatch cycle (§3.2), in topology dependency order. **Mode (b) with topology — group by group**: execute only the selected groups sequentially in topology dependency order. 1. Dispatch execute tasks for Gn. @@ -606,7 +628,9 @@ When a task fails: - **Replan**: add/remove/split tasks as needed — add new tasks, skip unnecessary ones, update dependencies 3. **Update `{{BASE_PATH}}/board.md`** -⚠️ Do not leave failed tasks hanging. If tasks depend on a `failed` task, they will never become ready — you must either retry the failed task or explicitly fail the dependents too. Once all non-failed tasks are `"done"` and failed tasks are intentionally skipped, you are done. +⚠️ Do not leave failed tasks hanging. If tasks depend on a `failed` task, they will never become ready — you must either retry the failed task or explicitly fail the dependents too. Once all non-failed tasks are `"done"` and failed tasks are intentionally skipped, **go to §3.7 and verify every completion criterion is met before closing** — task status reaching a terminal state is necessary but not sufficient; §3.7 is the sole authority on whether the project is actually done. + +**`❌ failed[findings]` is NOT eligible for "intentionally skipped" treatment.** A task carrying unresolved HIGH/CRITICAL findings must be remediated and re-dispatched to a clean PASS per §3.2.1 step 5 — it can only be left unresolved with explicit user approval (§3.7), never on your own judgement. Plain `❌ failed` (agent could not complete, no findings) is the only kind you may intentionally skip. **Preserve completed/in-progress tasks** — never modify or re-assign tasks that are already done or currently running. @@ -616,10 +640,10 @@ Quality has two layers: **Peer review (continuous):** Each downstream agent reviews its upstream dependencies inline. If B depends on A's output, B reads A's artifacts, validates them, and uses `[notify:A-role]` to request fixes — no coordinator round-trip needed. -**Quality gates (phase boundaries):** At key pipeline checkpoints (e.g. after Design & Plan), assign a quality gate task to the role whose charter owns quality validation. That agent reads upstream artifacts, runs quality checklists, and produces a pass/fail verdict. If the gate fails, assign remediation tasks to the responsible roles before advancing. +**Quality gates (phase boundaries):** At key pipeline checkpoints (e.g. after Design & Plan), assign a quality gate task to the role whose charter owns quality validation. That agent reads upstream artifacts, runs quality checklists, and produces a pass/fail verdict. If the gate reports HIGH/CRITICAL findings, apply the §3.2.1 remediation protocol before advancing. **Your role as coordinator**: You verify every task (§3.2 checklist) and intervene when: -- A task's artifact reports problems or blockers — create remediation tasks before dispatching dependents +- A task's artifact reports HIGH/CRITICAL findings or blockers — apply the §3.2.1 remediation protocol before dispatching dependents - An agent reports via `[notify:coordinator]` that it's blocked and can't resolve the issue peer-to-peer - A task fails after 2 retries - You need to replan (add/remove/split tasks) @@ -658,12 +682,12 @@ Acknowledge the user's input and act on it in the same response. ## 3.7 Completion criteria -Stop and report done when: +Stop and report done when **all** of the following hold — this section is the single authority for project completion (§3.3 directs here once all tasks reach terminal status): - All groups have completed their loops (plan → execute → validate), OR the selected groups have completed (in selective mode) -- All validation gates defined in the DAG's validation-phase entries have passed **unconditionally** — any gate with HIGH or CRITICAL findings must be remediated and re-run. "PASS WITH CONDITIONS" is NOT a pass. +- All validation gates defined in the DAG's validation-phase entries have passed **unconditionally** — any gate that ever reported HIGH or CRITICAL findings must have been reset to `⏳ pending`, re-dispatched (per §3.2.1 step 5), and produced a fresh clean PASS (zero HIGH/CRITICAL). "PASS WITH CONDITIONS" is NOT a pass. - The user's original request has been fully satisfied -- Any failures have been addressed or explicitly decided to skip +- No task remains marked `❌ failed[findings]` — each was either remediated and re-passed (per §3.2.1) or explicitly skipped with user approval. A `❌ failed[findings]` task that was never re-dispatched and re-passed does NOT satisfy completion, regardless of any remediation task's `[DONE]`. Before finishing, run `after_all` hooks — per `references/actions.yml`, this includes the final git commit and profile finalization. Then get the current UTC time and update `board.md`: append `**Project completed**: ` and `**Total duration**: ` below the `**Project started**` line. @@ -695,4 +719,4 @@ Before finishing, run `after_all` hooks — per `references/actions.yml`, this i - ⏳ t7 [] Final signoff [deps: t5, t6] ``` -Status markers: `⏳` pending, `🔄` in-progress, `✅` completed, `❌` failed. Update in-place — never move tasks between sections. \ No newline at end of file +Status markers: `⏳` pending, `🔄` in-progress, `✅` completed, `❌` failed. A `❌` carrying the `[findings]` suffix (`❌ failed[findings]`) means the task completed but surfaced unresolved HIGH/CRITICAL findings — it must be remediated and re-dispatched to a clean PASS per §3.2.1 before completion (§3.7), and is never silently skipped. Update in-place — never move tasks between sections. diff --git a/plugins/github-copilot-modernization/agents/modernize.agent.md b/plugins/github-copilot-modernization/agents/modernize.agent.md index 5fd3b86..4446aef 100644 --- a/plugins/github-copilot-modernization/agents/modernize.agent.md +++ b/plugins/github-copilot-modernization/agents/modernize.agent.md @@ -165,6 +165,7 @@ When user specifies EXACTLY what to do: | Java Azure service migration | `execution-coordinator` directly → hint: `modernize-azure-java` | `planning-coordinator` → `execution-coordinator` → hint: `modernize-azure-java` | | CVE / vulnerability fix (Java) | `execution-coordinator` directly → hint: `modernize-java-security` | `planning-coordinator` → `execution-coordinator` → hint: `modernize-java-security` | | .NET Azure migration or CVE fix | `execution-coordinator` directly → hint: `modernize-azure-dotnet` | `planning-coordinator` → `execution-coordinator` → hint: `modernize-azure-dotnet` | +| Infrastructure / deployment (Dockerfile, K8s, IaC) | `execution-coordinator` directly → hint: `modernize-deployment` | `planning-coordinator` → `execution-coordinator` → hint: `modernize-deployment` | | Structural rewrite / rearchitecture | `execution-coordinator` directly → hint: `modernize-rearchitecture` | `planning-coordinator` → `execution-coordinator` → hint: `modernize-rearchitecture` | **Example delegation — single task, version specified (e.g., "upgrade Java to 21"):** @@ -402,6 +403,7 @@ EXECUTE: Delegate to execution-coordinator subagent with task details directly - Azure migrations → modernize-azure-java - CVE/security fixes → modernize-java-security - .NET migrations → modernize-azure-dotnet + - Infrastructure/deployment → modernize-deployment - Structural rewrites → modernize-rearchitecture ↓ Present final results to user → STOP (wait for user input) @@ -498,6 +500,7 @@ The execution-coordinator will automatically route tasks to specialized migratio - Azure migration tasks → `modernize-azure-java` (Service Bus, Azure SQL, Redis, etc.) - CVE/security fix tasks → `modernize-java-security` (Java/Maven vulnerability scanning and fixes) - .NET tasks → `modernize-azure-dotnet` (.NET Azure migrations and NuGet CVE fixes) +- Infrastructure/deployment tasks → `modernize-deployment` (Dockerfiles, K8s/AKS/ACA, Bicep, CI/CD) - Structural rewrite tasks → `modernize-rearchitecture` (new stack, new directory, rearchitecture) You do NOT invoke these migration agents directly - always delegate to execution-coordinator. @@ -609,7 +612,7 @@ After each phase, results are saved to `.github/modernize//` director **Why this matters:** - The execution-coordinator knows how to route tasks to specialized agents -- Custom agents (modernize-java-upgrade, modernize-azure-java, modernize-java-security, modernize-azure-dotnet, modernize-rearchitecture) have built-in retry logic +- Custom agents (modernize-java-upgrade, modernize-azure-java, modernize-java-security, modernize-azure-dotnet, modernize-deployment, modernize-rearchitecture) have built-in retry logic - Custom agents self-verify and save results properly - Delegation enables sequential/parallel execution for multiple tasks @@ -640,7 +643,7 @@ Before starting execution phase, CHECK: - Run assessment when user provides specific task intent ❌ - Run assessment tools directly (delegate to assessment-coordinator) - **Call ANY MCP migration tools directly (appmod-* / AppModJavaUpgrade-* / AppModAzureJavaCLI-*)** ❌ -- **Invoke modernize-java-upgrade, modernize-azure-java, modernize-java-security, modernize-azure-dotnet, or modernize-rearchitecture directly** ❌ +- **Invoke modernize-java-upgrade, modernize-azure-java, modernize-java-security, modernize-azure-dotnet, modernize-deployment, or modernize-rearchitecture directly** ❌ - Execute task skills directly (delegate to execution-coordinator) - Proceed without user approval between phases (except in headless mode or specific task mode) @@ -661,7 +664,7 @@ Before starting execution phase, CHECK: **WHY YOU CANNOT USE THESE TOOLS:** - You are the ORCHESTRATOR, not an EXECUTOR -- MCP tools are for custom agents (modernize-java-upgrade, modernize-azure-java, modernize-java-security, modernize-azure-dotnet, modernize-rearchitecture) only +- MCP tools are for custom agents (modernize-java-upgrade, modernize-azure-java, modernize-java-security, modernize-azure-dotnet, modernize-deployment, modernize-rearchitecture) only - Your job is to ROUTE work to coordinators, not to DO the work yourself **WHAT YOU SHOULD DO INSTEAD:** diff --git a/plugins/github-copilot-modernization/plugin.json b/plugins/github-copilot-modernization/plugin.json index 005e6bb..0f456d2 100644 --- a/plugins/github-copilot-modernization/plugin.json +++ b/plugins/github-copilot-modernization/plugin.json @@ -1,7 +1,7 @@ { "name": "github-copilot-modernization", "description": "Autonomous application modernization with assess → plan → execute workflow", - "version": "1.21.0", + "version": "1.22.0", "author": { "name": "Microsoft", "email": "copilot-support@microsoft.com" diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/SKILL.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/SKILL.md index 56a1882..ef3b3f8 100644 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/SKILL.md +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/SKILL.md @@ -1,90 +1,239 @@ --- name: analyzing-architecture description: | - Runs deep codebase analysis: architecture patterns, tech stack, data models, integration points, and migration risks. Produces research artifacts consumed by creating-implementation-plan and feature-inventory. - Triggers: "analyze architecture", "analyze existing application", "analyze the codebase", "codebase architecture analysis", "discover dependencies", "assess migration risks", "run codebase discovery", "document module boundaries", "map action mappings". - NOT for: knowledge graph generation (use building-java-knowledge-graph), spec writing (use feature-inventory). + Architect analysis for rewrite/migration: produces structured architecture artifacts plus global prose research views (project-structure, tech-stack, data-model) for planning, implementation, feature-inventory, and gates. This is the single architect task. + Triggers: "analyze architecture", "analyze existing application", "analyze the codebase", "codebase architecture analysis", "analyze for migration", "prepare migration analysis", "produce migration artifacts", "analyze before rewrite". + NOT for: greenfield projects, pure syntax/version migrations (Python2→3, Java 8→17 — no paradigm shift), runtime validation (use runtime-validation), infrastructure/deployment analysis (use analyzing-operations), feature inventory/spec writing (use feature-inventory). --- +# Analyzing Architecture + +## Purpose + +Produce **only** artifacts that reduce a named failure mode. Anything that doesn't is excluded — it wastes agent context. + +There are two artifact purposes: + +- **Implementation fidelity**: behavior/contract fidelity to translate one unit correctly, plus seam contracts to integrate against the un-migrated remainder. +- **Design evidence**: evidence to decide unit count and splits — never pre-baked decisions. + +## Required References + +Each reference contains the YAML schema, extraction rules, and self-check for its artifact. Read all before starting the workflow. -## Output +| Reference | Artifact | What it provides | +|---|---|---| +| `references/unit-graph.md` | `unit_graph.yaml` | Schema for units, exported_signature, dynamic_entrypoints, shared_refs | +| `references/behavior.md` | `units/*/behavior.yaml` | Schema for side_effects, branches, error_paths, concurrency | +| `references/bindings.md` | `units/*/bindings.yaml` | Schema for framework wiring, runtime_config | +| `references/wire-contracts.md` | `wire_contracts.yaml` | Schema for external contracts, target_contract, semantic_divergence | +| `references/shared-modules.md` | `shared_modules.yaml` | Schema for god-class registry, fields with types, shared_refs relationship | +| `references/cross-unit-state.md` | `cross_unit_state.yaml` | Schema for implicit state flows, pairing values, verification_hint | +| `references/migration-boundary.md` | `migration_boundary.yaml` | Intent interpretation, must_rewrite with reasons, strategy rules | +| `references/seams.md` | `seams.yaml` | Schema for frozen_contract, bridge_points, declared vs inferred rules | +| `references/unit-decomposition.md` | `units/*/unit_decomposition.yaml` | Schema for candidate_splits, split-driver vocabulary | +| `references/project-structure.md` | `project-structure.md` | Functional domains, layers, project type — global prose view for planning/feature-inventory | +| `references/tech-stack.md` | `tech-stack.md` | Frameworks, deps, runtime versions, migration blockers — global prose view | +| `references/data-model.md` | `data-model.md` | Entity inventory, relationships, key-entities summary — global prose view | +| `references/extraction-signals.md` | (all artifacts) | Signal→artifact mapping, what to look for per signal area | +| `references/architecture-index.md` | `architecture_index.md` | Implementation Guide contract, per-unit navigation template | +| `references/consumption-contract.md` | (downstream) | How implementation agents read the artifacts | -All outputs are written under your task's `Artifact path:` (from task metadata). File names: +## Design Principles + +1. **Failure-mode-driven**: every field traces to a row in the Failure Mode Map (end of file). Can't name the failure it prevents → don't produce it. +2. **Source-loc as identifier**: `source_loc: path:line` is the natural ID. Never invent stable IDs. +3. **Self-contracting fields**: each field carries its consumption contract (`must_preserve`, `must_appear_in_target`) so the next agent needs no extra skill loaded. +4. **Per-unit sharding + global tables**: agents load one unit's small files plus global indexes, not a monolith. +5. **Extraction heuristics inline**: tell the executing LLM HOW to find data, not just the schema. +6. **Evidence, never fabricated numbers**: emit only values a tool actually computed, each with provenance. No invented composite scores, no made-up `confidence: 0.9`. Where confidence matters, report the *evidence basis* (`static` vs `static+runtime`), not a number nobody measured. +7. **Analyze observes; design decides**: candidate splits, candidate seams — never a committed unit count, never aggregates/BCs, never a priority ranking that pre-empts design's choice. +8. **Migration boundary is a first-class contract**: for rewrite/migration work, always identify the smallest runtime boundary that can satisfy the user's acceptance criteria. Expand to a full rewrite only when the user asks for clean removal/no legacy residue, or when technical constraints make a partial boundary unsafe. `source_anchors` are discovery evidence, not rewrite targets. +9. **Heuristic flag vs control gate**: + - **Heuristic flags allowed**: a magic number that only *labels* something for design to re-check. Design sees the data and can overrule. + - **Control gates forbidden**: a number that *changes what reaches the artifact set* (truncating candidates at a cap, skipping a flow below a floor). Replace with raw counts + per-row semantic contracts. + - Classification vocabularies are *examples to recognize by judgment*, not closed enums to CI-validate. + +## What is a "Unit" + +A unit = one externally triggerable entry point (HTTP route, scheduled job, message handler, UI page, public API surface, CLI command). **Uniqueness invariant**: each source file appears in at most one unit's `source_anchors`. Files used by ≥2 units → `shared_modules.yaml`. + +## The Artifacts ``` -project-structure.md -tech-stack.md -data-model.md -architecture-summary.md -migration-risks.md -infrastructure.md -test-coverage.md -deployment.md +artifacts/ +├── architecture_index.md top-level implementation guide +├── project-structure.md global prose, functional domains + layers + project type +├── tech-stack.md global prose, frameworks + deps + runtime versions +├── data-model.md global prose, entity inventory + key-entities summary +├── unit_graph.yaml global index, lightweight +├── migration_boundary.yaml global, minimal runnable boundary + rewrite scope contract +├── wire_contracts.yaml global, outward contracts +├── shared_modules.yaml global, files used by ≥2 units; god-class registry +├── cross_unit_state.yaml global, implicit shared-state flows +├── seams.yaml global, partial-migration cut points + bridge design +└── units// + ├── behavior.yaml per-unit, heavyweight + ├── bindings.yaml per-unit; may be [] + reason + └── unit_decomposition.yaml per-unit, CANDIDATES only, no commit ``` -(Only relevant files are generated based on selected tasks.) +## Outputs -# Analyzing Architecture +Base path: `{artifact_root}/` (typically `.github/modernize/rearchitecture/artifacts/`) -Provides a pool of research tasks for deep codebase analysis during the analysis phase. -Each task's full prompt is in `references/.md`. +**Global artifacts** (1 each): +- `unit_graph.yaml` — always +- `migration_boundary.yaml` — rewrite/migration work +- `wire_contracts.yaml` — always +- `shared_modules.yaml` — always +- `cross_unit_state.yaml` — always +- `seams.yaml` — when seams exist (declared or inferred); omit file entirely if no seams found +- `architecture_index.md` (top-level implementation guide) — always +- `project-structure.md` (global prose: functional domains, layers, project type) — always +- `tech-stack.md` (global prose: frameworks, deps, runtime versions, migration blockers) — always +- `data-model.md` (global prose: entity inventory + key-entities summary) — when project has entities/ORM/DB access -## ⚡ Dispatch: All Tasks in ONE Parallel Batch +**Per-unit artifacts** (one set per unit in `unit_graph.yaml`): +- `units//behavior.yaml` +- `units//bindings.yaml` +- `units//unit_decomposition.yaml` -**Do NOT run tasks in phases.** Select all applicable tasks upfront and dispatch them in a single parallel batch using the `task` tool (agent_type: "explore"). +**Completeness invariant**: `count(units/*/behavior.yaml) == count(units in unit_graph.yaml)`. -### Quick-Select by Project Type +For how downstream agents consume these artifacts, load `references/consumption-contract.md`. -Use the task description and a quick scan of the project root (README, build file, package.json) to classify the project, then select ALL tasks for that type: +--- -| Project Type | Tasks | -|---|---| -| **Java backend migration/upgrade** | `project-structure`, `tech-stack`, `data-model`, `migration-risks`, `architecture-summary`, `api-surface`, `infrastructure`, `test-coverage`, `deployment` (if Dockerfile exists) | -| **Java backend (new feature)** | `project-structure`, `tech-stack`, `data-model`, `architecture-summary`, `api-surface` | -| **Frontend SPA** | `project-structure`, `tech-stack`, `ui-components`, `state-routing`, `build-bundle`, `test-coverage` | -| **Fullstack** | ALL applicable from both backend and frontend | -| **Library/SDK** | `project-structure`, `tech-stack`, `api-surface`, `test-coverage` | +`target_idiom` is NOT produced here — lives in `guidelines/-to-/`. -**When in doubt, include the task** — extra analysis is cheap; missing analysis causes bad specs. +### Architecture index artifact — implementation guide -### Dispatch Method +The top-level architect artifact is an **implementation index**, not a prose summary. It must tell implementation agents which artifact paths to read, why each matters, how to filter global rows, and what completion evidence to report. -For each selected task, load `references/.md` to get the full prompt, then dispatch: +Load `references/architecture-index.md` for the required `Implementation Guide` contract and example shape. -``` -task(agent_type: "explore", name: "", prompt: "") -``` +--- + +## Implementation-Fidelity Artifacts + +Core artifacts: +- `unit_graph.yaml` (entry-point enumeration + `exported_signature` + `dynamic_entrypoints` + per-unit `shared_refs`). Load `references/unit-graph.md` for schema and self-check. +- `behavior.yaml` (side_effects, branches, error_paths, concurrency). Load `references/behavior.md` for schema. +- `bindings.yaml` (framework wiring + runtime_config). Load `references/bindings.md` for schema. +- `wire_contracts.yaml` (rest/grpc/kafka/sql/semantic_divergence). Load `references/wire-contracts.md` for schema and scope boundary. +- `shared_modules.yaml` (god-class registry: kind/used_by_units/fields/split_candidate). Load `references/shared-modules.md` for schema and `shared_refs` relationship. +- `cross_unit_state.yaml` (implicit session/ThreadLocal/SSO flows; per-row `must_confirm:runtime` for any unpaired flow). Load `references/cross-unit-state.md` for schema and `pairing` values. + +### `migration_boundary.yaml` — minimal runnable boundary + rewrite scope contract -**Issue ALL task() calls in a single assistant turn** so they run in parallel. +Records the smallest runtime-reachable implementation boundary that satisfies the user's acceptance criteria. Implementation scope comes from `must_rewrite`, not from all `source_anchors` or every legacy-framework file. -After all explore agents complete, synthesize their findings into the output files. +Load `references/migration-boundary.md` for intent interpretation rules, schema, and self-check. + +### `seams.yaml` — partial-migration cut points + bridge design + +Records deliberate cuts for partial migration: which side is frozen, which side migrates, and how the bridge converts protocols/idioms. `declared` seams are authoritative; `inferred` seams are advisory. + +Load `references/seams.md` for schema, conditional `frozen_contract` rules, discovery signals, and self-check. --- -## Task Reference +## Design-Evidence Artifact -### Core Tasks (Always Run) +### `units//unit_decomposition.yaml` (per-unit) -| Task | Reference | Output | -|------|-----------|--------| -| `project-structure` | `references/project-structure.md` | Functional domains, layers, project type | -| `tech-stack` | `references/tech-stack.md` | Frameworks, deps, migration blockers | +Records split candidates for design. It produces `candidate_splits`, not target units; the design phase owns the decision. -### Optional Task Pool +Load `references/unit-decomposition.md` for schema and split-driver vocabulary. -| Task | When Relevant | Reference | Output | -|------|---------------|-----------|--------| -| `data-model` | Project has ORM / entity classes / DB access | `references/data-model.md` | Entity inventory, schema | -| `migration-risks` | Any migration/upgrade task | `references/migration-risks.md` | Risk by module, patterns | -| `architecture-summary` | Knowledge graph exists, or complex inter-module deps | `references/architecture-summary.md` | Arch patterns, coupling | -| `api-surface` | Project exposes REST/GraphQL/gRPC endpoints | `references/api-surface.md` | Endpoint inventory, DTOs | -| `integration-points` | Project calls external services, MQ, cache, 3rd-party APIs | `references/integration-points.md` | External deps, service boundaries | -| `infrastructure` | **Always for migration/upgrade** | `references/infrastructure.md` | DB/MQ/cache deps, test infra | -| `test-coverage` | **Always for migration/upgrade** | `references/test-coverage.md` | Test inventory, portability, gaps | -| `deployment` | Project has Dockerfile/K8s/CI configs | `references/deployment.md` | Container, CI/CD, IaC | -| `ui-components` | Frontend with component-based framework | `references/ui-components.md` | Component tree, design system | -| `state-routing` | Frontend SPA with state management | `references/state-routing.md` | State, routes, data fetching | -| `build-bundle` | Frontend or Node.js with custom build config | `references/build-bundle.md` | Build tool, bundling, optimization | +--- + +## Workflow + +1. **Load context** — source/target framework, existing KG, `guidelines/-to-/`, and any **user-declared seams** (cut points the user specified). +1b. **Load extraction signals** — read `references/extraction-signals.md` and map discovered signals into the structured artifacts. +1c. **Produce global prose views** — alongside the structured per-unit artifacts, emit `project-structure.md` (functional domains, layers, project type), `tech-stack.md` (frameworks, deps, runtime versions, migration blockers), and `data-model.md` (entity inventory + key-entities summary) per their reference schemas. These global views are consumed by creating-implementation-plan, feature-inventory, and the spec-quality gate; the structured YAML artifacts do not replace them. +2. **Build `unit_graph.yaml`** (spine). Resolve `exported_signature` from public signatures only. Seed `shared_modules.yaml` same pass; flag god-class + reference-cliff candidates. +2b. **Build `migration_boundary.yaml`** — infer user intent, acceptance criteria, cleanup requirement, and the smallest runtime-reachable rewrite boundary. Populate `must_rewrite`, `copy_as_is`, `legacy_allowed_to_remain`, and `defer_cleanup`. Only choose `full_rewrite` when user intent or technical evidence requires it; do not equate `source_anchors` with rewrite targets. +3. **Per-unit files — IMMEDIATELY after unit_graph, before global tables.** For EVERY unit listed in `unit_graph.yaml`, create `units//behavior.yaml`, `units//bindings.yaml`, and `units//unit_decomposition.yaml`. Do not skip units. Do not create "representative samples". Do not defer to a later step. >~200 lines per file → re-examine the unit boundary. Populate `shared_refs` from subset whitelist. `unit_decomposition` sets `commit: false`. +3b. **Verify per-unit completeness before proceeding.** Run: count the units in `unit_graph.yaml` and count the `units/*/behavior.yaml` files. If they do not match, create the missing per-unit files NOW. Do not proceed to step 4 until every unit has all three files. +4. **Build `wire_contracts.yaml`** — outward edges; cross-ref unit_graph for external interfaces. +4b. **Build `cross_unit_state.yaml`** — scan medium patterns; pair across units only; per-row `must_confirm:runtime` on any unpaired flow. +4c. **Build `seams.yaml`** (skip if no declared or inferred seams exist) — emit every user-declared seam first (`source: declared`). Then add `inferred` seams from discovery signals. For each seam: record `frozen_side` + `frozen_side_rule` (always), and — where protocols differ — the `bridge_points` conversion design (mapping_rule + edge_cases + idempotency_retry + fallback). Add `frozen_contract` **only** when the migrating agent cannot read the frozen side's behavior from source (binary/private dependency, config/data-gated semantics, name-contradicts-behavior); when the frozen source is visible and self-explanatory, omit it — don't restate what the agent reads directly. Resolve declared/inferred conflicts toward declared. +4d. **Build the architecture index** — top-level architect artifact with an `Implementation Guide`. For each unit, list exact artifact paths, purpose of each file, how to filter global rows, and required completion evidence. Do not make it a prose-only summary. +5. **Self-check before completion** (hard — execute, do not skip): + - **Per-unit completeness gate (MUST execute):** Count units in `unit_graph.yaml` (`grep -c '^\s*- name:' artifacts/unit_graph.yaml`). Count per-unit behavior files (`ls artifacts/units/*/behavior.yaml | wc -l`). If counts do not match, list missing units and create their behavior.yaml, bindings.yaml, and unit_decomposition.yaml NOW. Do not report done until counts match. "Representative samples" or "most controllers follow identical patterns" is NOT acceptable — every unit gets all three files. + - Every unit in `unit_graph` has behavior/bindings/decomposition files. + - `migration_boundary.yaml` exists for rewrite/migration work; `must_rewrite` is the implementation scope; `source_anchors` are not treated as rewrite targets. + - The architecture index contains an `Implementation Guide` for every unit, with exact artifact paths, purpose, row-filter instructions, and completion evidence requirements. + - The architecture index explicitly states that it is not the full contract and implementation agents must follow the listed artifact paths before implementation. + - No `TBD` in `wire_contracts.yaml::target_contract`, `seams.yaml::frozen_contract` target form, or `seams.yaml::bridge_points[].mapping_rule`. + - No source file in `source_anchors` of multiple units. + - Every produced field maps to a Failure Mode Map row. + - `unit_decomposition.commit == false`. + - Every `shared_refs.used_fields ⊆ {f.name for f in shared_modules.fields}`. + - Every `cross_unit_state` flow with `pairing != matched` carries `must_confirm: runtime`. + - **Every `declared` seam present; every seam has a `frozen_side_rule`; `frozen_contract` present only where the frozen source is invisible/unrecoverable; every `protocol_shift != null` seam has ≥1 `bridge_point` with concrete `mapping_rule` (no `TBD`).** + +## Rules + +- No prose narrative artifacts. Reasoning lives in `notes:` / `rationale:`. +- No stable IDs. `source_loc: path:line` is the identifier. +- No tests / build / deploy / infra coverage. No standalone risk register. No `target_idiom.yaml`. +- Per-unit files capped at ~200 lines. Larger → split. +- `TBD` forbidden in wire contract `target_contract`, seam `frozen_contract` target form, and seam `mapping_rule`. +- `unit_decomposition.yaml` MUST set `commit: false`. +- Factual numbers (line numbers, counts, lists) recorded as-is. Quality/cohesion values MUST be tool-computed with provenance; inventing scores is a hard failure. +- A **`declared` seam is authoritative** — design may not overrule it; an `inferred` seam is a candidate. +- The **frozen side of a seam MUST NOT be refactored** in the migrating phase; the bridge adapts to it, not the reverse. +- **Implementation scope comes from `migration_boundary.yaml::must_rewrite`**, not from `unit_graph.source_anchors`, all files of the old framework, or inventory lists. `source_anchors` prove behavior exists; they do not mandate rewriting that file. +- Classification lists are vocabularies for judgment, not CI-enforced enums. +--- +## Failure Mode Map + +| # | Failure mode | Prevented by | +|---|---|---| +| 1 | Dropped side-effect | `behavior.yaml::side_effects[must_preserve]` | +| 2 | Dropped framework binding | `bindings.yaml::bindings[must_appear_in_target]` | +| 3 | Hallucinated target API | `guidelines/-to-/` (out of scope) | +| 4 | Broken caller (signature unsync) | `unit_graph.yaml::depends_on` + `exported_signature` | +| 5 | Dead-code removal of reflection/DI class | `unit_graph.yaml::dynamic_entrypoints` | +| 6 | Wire contract break | `wire_contracts.yaml::stability:frozen + target_contract` | +| 7 | Tx boundary lost | `behavior.yaml::concurrency.tx_boundary` | +| 8 | Cross-language semantic gotcha | `wire_contracts.yaml::semantic_divergence` | +| 9 | Missing runtime config | `bindings.yaml::runtime_config[must_appear_in_target]` | +| 10 | Significant branch dropped | `behavior.yaml::branches[must_preserve]` | +| 11 | Error contract drift | `behavior.yaml::error_paths[contract + must_preserve]` | +| 12 | Shared module duplicated/lost | `shared_modules.yaml::migration_strategy` | +| 13 | Concurrency model mismatch | `behavior.yaml::concurrency.model` | +| 14 | Implicit cross-unit state lost | `cross_unit_state.yaml::flows[must_preserve]` | +| 15 | Static pairing missed dynamic key / external writer | `cross_unit_state.yaml::pairing + must_confirm:runtime` | +| 16 | God-class field drift / hallucinated fields | `shared_modules.yaml::god_class + shared_refs.used_fields ⊆` | +| 17 | Premature commit to target unit count | `unit_decomposition.yaml::commit:false + candidate_splits` | +| 18 | Split candidate without driver/rationale | `unit_decomposition.yaml::candidate_splits[].drivers + rationale` | +| 19 | Fabricated score/confidence misleads design | Principle 6 + self-check: no composite_score | +| 20 | **Partial-migration cut breaks at the seam (an *unreadable* frozen-side semantic — binary dep, config-gated, or name-contradicts-behavior — never recorded)** | **`seams.yaml::frozen_contract[must_preserve]`, conditional: only when source is invisible/unrecoverable** | +| 21 | **Frozen side refactored, breaking un-migrated peers** | **`seams.yaml::frozen_side_rule`** | +| 22 | **Protocol-shift conversion left to the migrating agent's guess (wrong mapping/edge cases)** | **`seams.yaml::bridge_points[mapping_rule + edge_cases + idempotency_retry + fallback]`** | +| 23 | **User-specified cut point silently overruled by analyze** | **`seams.yaml::source:declared` authoritative rule** | +| 24 | **Inventory-driven over-rewrite: every discovered framework file or `source_anchor` becomes an implementation task, even though a smaller runtime boundary satisfies acceptance** | **`migration_boundary.yaml::{strategy,must_rewrite,legacy_allowed_to_remain}` + rule: source_anchors are not rewrite targets** | +| 25 | **User asked for clean/full rewrite but analyze silently leaves legacy runtime residue** | **`migration_boundary.yaml::{user_intent.cleanup_required,full_rewrite_reason}`** | + +## NOT Included + +- Test coverage map — tester / runtime-validation +- Build / packaging / deploy topology — `analyzing-operations` +- Performance baseline — cutover phase +- Standalone risk register — inline `notes` / `stability` +- Architecture summary prose — implementation agents need source-anchored contracts, not prose-only summaries +- Idiom mapping — `guidelines/-to-/` +- Function-level call graph beyond unit boundaries — `exported_signature` suffices +- **Cohesion metrics / co-access clusters (LCOM4/TCC)** — structural numbers did not change design decisions. God-class smells live on `shared_modules.yaml::split_candidate`. +- Pure syntax migration (Py2→3, Java 8→17) +- **Target unit count commitment** — design phase +- **Aggregate / BC / VO decisions, domain renames, migration sequencing** — design phase + human EventStorming +- **Composite/priority scores, decision gate ratios** — design weighs evidence with full context diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/api-surface.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/api-surface.md deleted file mode 100644 index 4f8045f..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/api-surface.md +++ /dev/null @@ -1,10 +0,0 @@ -Analyze all API endpoints in the codebase. Focus on: -- REST endpoints (paths, HTTP methods, parameters, response types) -- GraphQL schemas/resolvers (if present) -- gRPC service definitions (if present) -- API versioning strategy -- Authentication/authorization on endpoints -- Request/response DTOs and serialization format -- OpenAPI/Swagger specs (if present) - -Output: `./api-surface.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/architecture-index.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/architecture-index.md new file mode 100644 index 0000000..d88ca9a --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/architecture-index.md @@ -0,0 +1,62 @@ +# Architecture Index Artifact + +This reference is loaded from `SKILL.md` when producing the top-level architecture/index artifact for implementation agents. + +### Architecture index artifact — implementation guide + +The top-level architect artifact is an **implementation index**, not a prose summary. Its filename should be `architecture_index.md` in the artifact root. Implementation agents discover it by this name. + +It MUST include an `Implementation Guide` section. For every implementation unit, list: +- assigned unit name plus external trigger (route, API, message, job, CLI command, or public surface) +- exact artifact paths to read for that unit, expressed relative to the artifact root +- what each artifact is used for: behavior, bindings, wire contracts, shared modules, cross-unit state, seams, migration boundary, or split candidates +- how to filter global rows for the unit (for example by `used_by_units`, `flows`, `cut_between`, or equivalent fields) +- completion evidence required from the implementation agent: artifact paths read, implemented `must_preserve` items, unresolved/deferred contracts, and tests/build/runtime evidence + +The index MUST say explicitly: "This index is not the full contract. Do not implement from this file alone; follow the artifact paths below." + +Example shape (file names are illustrative; use the actual produced paths): + +```markdown +## Implementation Guide + +### Global artifacts +- `` + - who reads: all implementation workers + - use for: unit boundary, entrypoints, dependencies, exported signatures, shared refs + - how to consume: find your assigned unit; follow its source anchors, shared refs, and dynamic entrypoints +- `` + - who reads: all implementation workers + - use for: implementation scope (must_rewrite vs copy_as_is vs legacy_allowed_to_remain) + - how to consume: check must_rewrite for your unit's files; source_anchors are NOT rewrite targets +- `` + - who reads: workers touching external/API calls + - use for: request/response/error contracts, semantic divergence warnings + - how to read: filter rows where unit matches your assigned unit +- `` + - who reads: workers whose unit references shared code + - use for: migration strategy, field subset whitelist, god-class awareness + - how to read: filter rows where used_by_units includes your unit +- `` + - who reads: workers whose unit reads or writes implicit shared state + - use for: session/ThreadLocal/static state flows, must_confirm:runtime flags + - how to read: filter flows where writer.unit or reader.unit matches your unit +- `` (when present) + - who reads: workers whose unit touches a migration cut point + - use for: frozen side rules, bridge point mapping, declared vs inferred seam authority + - how to read: filter cuts where cut_between includes your unit + +### Unit: +- external trigger: +- must read: + - `` — use for side effects, branches, loading/error states, user-visible behavior + - `` — use for selectors, route/query/runtime bindings, framework wiring + - `` — use for optional split candidates only; do not treat as required target structure +- relevant global rows: + - `` rows tied to `` + - `` rows where used_by_units includes `` + - `` flows touching `` + - `` cuts touching `` +- before DONE report: artifacts_read, implemented_must_preserve, unresolved_or_deferred, verification evidence +``` + diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/architecture-summary.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/architecture-summary.md deleted file mode 100644 index b7aec20..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/architecture-summary.md +++ /dev/null @@ -1,13 +0,0 @@ -If `setup_artifacts.knowledge_graph_dir` is null or the path does not exist, skip this step silently — do not report an error, continue with direct source analysis instead. - -Otherwise, read `knowledge-graph.json` from `setup_artifacts.knowledge_graph_dir`. Focus on: -- Key classes and their roles -- Inter-module dependencies -- Core architectural patterns (layering, DI, transaction scope) -- Tightly coupled areas (high fan-in/fan-out, classes with many dependencies) - -> ⚠️ **Document the existing architecture only.** Do not suggest target architecture or refactoring approaches. - -Distill into a concise markdown summary (do NOT reproduce the full JSON). - -Output: `./architecture-summary.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/behavior.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/behavior.md new file mode 100644 index 0000000..43e21a6 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/behavior.md @@ -0,0 +1,52 @@ +# behavior.yaml — Per-Unit Behavior Contract + +The heavyweight per-unit artifact. Records all externally observable behavior: side effects, conditional branches, error paths, and concurrency semantics. Implementation agents use this as the primary fidelity contract — every `must_preserve: true` item must appear in the target. + +Per-unit file: `units//behavior.yaml`. + +## Schema + +```yaml +unit: +side_effects: + - source_loc: "" + effect: "" + must_preserve: # true = must appear in target (Failure Mode 1) +branches: + - source_loc: "" + condition: "" + behavior: "" + must_preserve: # true = branch path must be preserved (Failure Mode 10) +error_paths: + - source_loc: "" + trigger: "" + outcome: "" + contract: "" + must_preserve: # true = error contract must be preserved (Failure Mode 11) +concurrency: + model: + # the concurrency model used by this unit (Failure Mode 13) + tx_boundary: # transaction scope (Failure Mode 7) + source_loc: "" + scope: "" + isolation: "" + must_preserve: +``` + +## Key Fields + +- **`side_effects[].must_preserve`**: when true, the effect must exist in target code. Prevents Failure Mode 1 (dropped side-effect). Common examples: audit log writes, notification sends, cache invalidations. +- **`branches[].must_preserve`**: when true, the conditional path must be replicated. Prevents Failure Mode 10 (significant branch dropped). Focus on user-visible branching — not every `if` statement. +- **`error_paths[].contract`**: the external-facing error contract (HTTP status code, error response shape, exception type thrown to callers). Prevents Failure Mode 11 (error contract drift). May be empty for internal-only error handling. +- **`concurrency.model`**: the threading/async model. Prevents Failure Mode 13 (concurrency model mismatch). Critical when migrating between sync and async frameworks. +- **`concurrency.tx_boundary`**: transaction scope — which operations are atomic. Prevents Failure Mode 7 (tx boundary lost). Record the actual scope, not just "uses transactions". + +## Size Cap + +Per-unit behavior files are capped at ~200 lines. If a unit's behavior exceeds this, re-examine the unit boundary — it may need splitting (see `unit_decomposition.yaml`). + +## Self-Check + +- Every unit in `unit_graph.yaml` has a `behavior.yaml` file. +- Every `must_preserve: true` item has a `source_loc`. +- `concurrency` block is present when the unit uses transactions, async patterns, or thread pools. diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/bindings.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/bindings.md new file mode 100644 index 0000000..be7d78e --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/bindings.md @@ -0,0 +1,50 @@ +# bindings.yaml — Per-Unit Framework Wiring & Runtime Config + +Records framework-specific wiring (annotations, XML config, DI registrations, route declarations) and externalized runtime configuration that must appear in the target. Per-unit file: `units//bindings.yaml`. + +## Schema + +```yaml +unit: +bindings: + - type: + name: "" + source_loc: "" + target: "" + must_appear_in_target: # true = implementation must wire this in target framework + notes: "" +runtime_config: + - key: "" + source_loc: "" # where it's read + source: + default_value: "" + must_appear_in_target: + notes: "" +``` + +## Key Fields + +- **`bindings[].must_appear_in_target`**: when true, the implementation agent must create equivalent wiring in the target framework. Prevents Failure Mode 2 (dropped framework binding). The *form* will differ (e.g., Struts XML → Spring Boot annotation), but the *effect* must be preserved. +- **`bindings[].type`**: what kind of framework wiring. Helps the implementation agent find the right target-framework equivalent. +- **`runtime_config[].must_appear_in_target`**: when true, this config must be externalized in the target. Prevents Failure Mode 9 (missing runtime config). + +## Scope Boundary + +- **Included**: framework annotations/decorators, XML/YAML config bindings, DI container registrations, route/filter/interceptor/listener declarations, property/env-var references. +- **Excluded**: business logic (→ `behavior.yaml`), external API contracts (→ `wire_contracts.yaml`), ORM entity mappings that are purely internal data access (unless they define a cross-service data contract). + +## Empty Bindings + +When a unit has no framework wiring (rare — usually means it's a pure domain unit), produce `bindings: []` with a `reason` field explaining why: + +```yaml +unit: pure_domain_calculator +bindings: [] +reason: "No framework annotations or DI wiring — pure computation with no framework coupling." +runtime_config: [] +``` + +## Self-Check + +- Every unit in `unit_graph.yaml` has a `bindings.yaml` file (even if `bindings: []`). +- Every `must_appear_in_target: true` binding has a non-empty `name` and `source_loc`. diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/build-bundle.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/build-bundle.md deleted file mode 100644 index 080d3be..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/build-bundle.md +++ /dev/null @@ -1,9 +0,0 @@ -Analyze build tooling and bundling. Focus on: -- Build tool configuration (Webpack, Vite, Rollup, esbuild, Turbopack, etc.) -- Code splitting strategy -- Asset handling (images, fonts, SVGs) -- Environment-specific builds (dev/staging/prod) -- Build plugins and custom transformations -- Bundle size and optimization - -Output: `./build-bundle.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/business-logic-extraction.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/business-logic-extraction.md deleted file mode 100644 index 4edaf20..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/business-logic-extraction.md +++ /dev/null @@ -1,215 +0,0 @@ ---- -name: Business Logic Extraction -description: Extract and catalog business logic from source codebase for rewrite migration mode. -mode: rewrite ---- - -## Overview - -This skill systematically identifies, extracts, and documents all business logic from a source codebase to enable accurate rewrite in a new technology stack. The goal is to ensure **functional equivalence** - the rewritten application produces the same business outcomes. - -## User Input - -```text -``` - -You **MUST** consider the user input before proceeding (if not empty). - -## When to Use - -- **Mode**: REWRITE only -- **Phase**: Planning Phase 0 (Research) — invoked by planning skill -- **Prerequisites**: Constitution defined, Knowledge graph available - -## Extraction Process - -### Step 1: Extract Business Logic Units - -For each business logic unit, document: - -```yaml -business_logic_unit: - id: "BL-001" - name: "Order Total Calculation" - category: "calculation" - source_location: "src/main/java/com/example/service/OrderService.java:45-78" - source_methods: ["calculateTotal", "applyDiscount", "calculateTax"] - - description: | - Calculates the total order amount including: - - Item subtotals - - Tax calculation based on region - - Discount application - - Shipping cost - - inputs: - - name: "orderItems" - type: "List" - description: "Items in the order with quantity and price" - - name: "customerRegion" - type: "String" - description: "Customer's geographic region for tax calculation" - - name: "discountCode" - type: "String" - optional: true - description: "Optional promotional discount code" - - outputs: - - name: "orderTotal" - type: "BigDecimal" - description: "Final order total after all calculations" - - name: "taxAmount" - type: "BigDecimal" - description: "Calculated tax amount" - - dependencies: - - "TaxService - for tax rate lookup" - - "DiscountService - for discount validation" - - business_rules: - - "Tax is calculated on subtotal before discount" - - "Maximum discount is 50% of subtotal" - - "Free shipping for orders over $100" - - # Behavioral specification: precise branch-level logic extracted from source code - # Each entry documents one conditional branch or behavioral path - behavioral_spec: - - condition: "orderItems.isEmpty()" - action: "return BigDecimal.ZERO" - branch_type: "early_return" - - condition: "discountCode != null && discountService.isValid(discountCode)" - action: "subtotal * (1 - discount.percentage / 100)" - constraint: "discountedAmount <= subtotal * 0.5" - - condition: "discountCode != null && !discountService.isValid(discountCode)" - action: "ignore discount, use full subtotal" - error_response: "none (silent ignore)" - - condition: "subtotal > 100.00" - action: "shipping = 0" - - condition: "subtotal <= 100.00" - action: "shipping = flatShippingRate" - - # Data validation rules from source code - validation_rules: - - field: "orderItems" - rules: ["NOT_NULL", "NOT_EMPTY"] - - field: "orderItems[].quantity" - rules: ["MIN: 1", "MAX: 999"] - - field: "orderItems[].price" - rules: ["NOT_NULL", "MIN: 0.01"] - - field: "discountCode" - rules: ["OPTIONAL", "PATTERN: ^[A-Z0-9]{6,12}$"] - - # Side effects produced by this logic - side_effects: - - "Audit log entry created for discount application" - - "Order total cached in session" - - edge_cases: - - "Empty order returns zero total" - - "Invalid discount code is ignored, not error" - - "International orders have different tax rules" - - test_scenarios: - - description: "Basic order with tax" - input: "3 items, US region, no discount" - expected_output: "subtotal + 8.5% tax" - - description: "Order with discount" - input: "3 items, discount code 'SAVE20'" - expected_output: "subtotal - 20% + tax" - - description: "Empty order" - input: "0 items" - expected_output: "BigDecimal.ZERO" - - description: "Excessive discount capped" - input: "1 item $10, discount 80%" - expected_output: "subtotal - 50% (capped) + tax" -``` - -### Step 4: Generate Business Logic Inventory - -Create `FEATURE_DIR/business-logic-inventory.md`: - -```markdown -# Business Logic Inventory - -**Source Application**: [APP_NAME] -**Extraction Date**: [DATE] -**Total Business Logic Units**: [COUNT] - -## Summary by Category - -| Category | Count | Complexity | -|----------|-------|------------| -| Validation | 12 | Low-Medium | -| Calculation | 8 | Medium-High | -| Workflow | 5 | High | -| Transformation | 15 | Low | -| Integration | 6 | Medium | -| Rules | 10 | Medium | - -## Business Logic Units - -### Validation Logic - -#### BL-001: Order Validation -- **Source**: `OrderService.java:23-45` -- **Purpose**: Validates order before processing -- **Inputs**: Order object -- **Outputs**: ValidationResult -- **Rules**: [list rules] -- **Rewrite Notes**: Use Jakarta Bean Validation - -[Continue for each unit...] - -## Cross-Cutting Concerns - -### Authentication/Authorization -- Location: [files] -- Pattern: [describe pattern] -- Rewrite approach: Use Spring Security - -### Transaction Management -- Location: [files] -- Pattern: [describe pattern] -- Rewrite approach: Use @Transactional - -### Error Handling -- Location: [files] -- Pattern: [describe pattern] -- Rewrite approach: Use @ControllerAdvice - -## Dependencies Map - -```mermaid -graph TD - A[OrderController] --> B[OrderService] - B --> C[TaxService] - B --> D[DiscountService] - B --> E[InventoryService] -``` - -## Rewrite Priority - -| Priority | Business Logic | Reason | -|----------|---------------|--------| -| P1 | Core workflows | Essential for MVP | -| P2 | Calculations | Business critical | -| P3 | Validations | Can use framework defaults initially | -| P4 | Integrations | Can be stubbed initially | -``` - -## Output Artifacts - -| Artifact | Path | Purpose | -|----------|------|---------| -| Business Logic Inventory | `FEATURE_DIR/business-logic-inventory.md` | Master list of all business logic | - -## Key Rules - -- **Completeness**: Every piece of business logic must be documented -- **No Implementation Details**: Focus on WHAT, not HOW (that's for target design) -- **Testability**: Each unit must have clear inputs, outputs, and test scenarios -- **Traceability**: Link to source code locations for reference during implementation -- **Source Methods**: Every BL unit MUST include `source_methods` listing the exact method names — these are used by the tasks skill to generate `[Source:]` annotations and by the implementation skill for source-anchored implementation -- **Behavioral Specification**: For each BL unit, extract `behavioral_spec` documenting every conditional branch, validation check, and error path from the source code. The implementation agent uses this to achieve branch-level parity. -- **Validation Rules**: Document `validation_rules` with exact field-level constraints extracted from source validation logic (annotations, XML validators, programmatic checks) -- **Side Effects**: Document all `side_effects` (database writes, notifications, cache updates, audit logs) so they are not lost during rewrite diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/consumption-contract.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/consumption-contract.md new file mode 100644 index 0000000..ff25f7a --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/consumption-contract.md @@ -0,0 +1,104 @@ +# Consumption Contract — How Downstream Agents Read Architecture Artifacts + +This reference defines how implementation/design/review agents consume the artifacts produced by `analyzing-architecture`. The architect produces; downstream reads by following this contract. + +## Artifact Root + +All paths below are relative to `{artifact_root}/` (typically `.github/modernize/rearchitecture/artifacts/`). + +## Reading Strategy by Role + +### Implementation agent (assigned unit X) + +**Must read before writing any code:** + +1. `unit_graph.yaml` — filter to unit X: + - `source_anchors` → source files to understand (NOT rewrite targets — see `migration_boundary.yaml::must_rewrite` for scope) + - `depends_on` + `exported_signature` → call sites to preserve; `stability: frozen` signatures must keep exact shape in target + - `dynamic_entrypoints` → do not dead-code-eliminate these + - `shared_refs` → per-unit subset of shared modules this unit uses; `used_fields` names must ⊆ `shared_modules[module].fields[].name` + +2. `migration_boundary.yaml` + - `must_rewrite` → the files/modules in implementation scope + - `copy_as_is` → leave untouched + - `legacy_allowed_to_remain` → acceptable residue + - Rule: implementation scope = `must_rewrite`, NOT `source_anchors` + +3. `shared_modules.yaml` — rows where `used_by_units` includes X: + - `migration_strategy` → how to handle (extract, wrap, split) + - `shared_refs.used_fields` → only these fields are your concern (subset whitelist) + - God-class entries → respect `split_candidate` guidance + +4. `units/X/behavior.yaml` + - `side_effects` where `must_preserve: true` → each must appear in target + - `branches` where `must_preserve: true` → each branch path preserved + - `error_paths` → preserve `contract` and `must_preserve` items + - `concurrency.tx_boundary` → replicate transaction scope + +5. `units/X/bindings.yaml` + - `bindings` where `must_appear_in_target: true` → wire in target framework + - `runtime_config` where `must_appear_in_target: true` → externalize config + +**Filter remaining global artifacts for unit X:** + +6. `wire_contracts.yaml` — rows where unit field matches X: + - `stability: frozen` → preserve exact contract shape + - `target_contract` → required target-side signature + - `semantic_divergence` → cross-language gotchas to handle + +7. `cross_unit_state.yaml` — flows where `writer.unit == X` or `reader.unit == X`: + - `must_preserve: true` → replicate the state-passing mechanism + - `pairing != matched` + `must_confirm: runtime` → flag for runtime verification + +8. `seams.yaml` — cuts touching X: + - `source: declared` → authoritative, do not change frozen side + - `frozen_side_rule` → which side you must not modify + - `bridge_points` → use `mapping_rule` for conversion; handle `edge_cases` + - `frozen_contract` (when present) → the frozen side's behavior you cannot read from source + +**Optional (do not treat as binding):** + +9. `units/X/unit_decomposition.yaml` + - `candidate_splits` — advisory only (`commit: false`) + - Design may have already resolved these; check design artifacts first + +### Design agent + +1. Read `unit_graph.yaml` (all units) — unit count, dependencies, boundaries +2. Read `units/*/unit_decomposition.yaml` — candidate splits + drivers +3. Read `shared_modules.yaml` — god-class entries inform split decisions +4. Read `seams.yaml` — declared seams constrain design choices +5. Design owns: final unit count, split decisions, sequencing, domain naming + +### Review / gate agent + +Verify completeness by checking: +- `ls units/*/behavior.yaml | wc -l` == unit count in `unit_graph.yaml` +- Every unit has `bindings.yaml` and `unit_decomposition.yaml` +- `migration_boundary.yaml` exists (for rewrite/migration work) +- Architecture index contains `Implementation Guide` with per-unit entries +- Architecture index states it is not the full contract +- No `TBD` in `wire_contracts.yaml::target_contract`, `seams.yaml::frozen_contract` target form, or `seams.yaml::bridge_points[].mapping_rule` +- No source file appears in `source_anchors` of multiple units +- Every `unit_decomposition.yaml` has `commit: false` +- Every `shared_refs[].used_fields ⊆ {f.name for f in shared_modules[module].fields}` +- Every `cross_unit_state` flow with `pairing != matched` carries `must_confirm: runtime` +- Every `declared` seam present; every seam has `frozen_side_rule` +- Every `protocol_shift != null` seam has ≥1 `bridge_point` with concrete `mapping_rule` (no TBD) + +## Relationship to Architecture Index + +The **architecture index** (produced artifact) is a run-specific navigation file: it lists exact paths and per-unit reading instructions for a specific codebase's artifacts. + +This **consumption contract** (reference file) defines the general field-level semantics: what each field means, which are hard contracts, how to filter global rows. Implementation agents use both: +1. Architecture index → find artifact paths for their assigned unit +2. Consumption contract → understand field semantics and consumption rules + +## Key Rules for Consumers + +1. **`source_anchors` ≠ rewrite targets.** Implementation scope comes from `migration_boundary.yaml::must_rewrite`. +2. **`must_preserve` / `must_appear_in_target` are hard contracts.** Missing one = failure mode triggered (see Failure Mode Map in SKILL.md). +3. **`commit: false` in unit_decomposition is real.** Do not treat candidate splits as decided structure. +4. **`declared` seams are authoritative.** Do not refactor the frozen side. +5. **Filter global artifacts to your unit.** Do not read all rows; use unit-name matching fields (`used_by_units`, `unit`, `writer.unit`/`reader.unit`, `cut_between`). +6. **Architecture index is a navigation aid, not the full contract.** Always follow the artifact paths it lists and read the actual YAML files. diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/cross-unit-state.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/cross-unit-state.md new file mode 100644 index 0000000..46561b5 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/cross-unit-state.md @@ -0,0 +1,49 @@ +# cross_unit_state.yaml — Implicit Shared-State Flows + +Records implicit state passing between units: session attributes, ThreadLocal, SSO tokens, request-scoped data, static/singleton state, framework-managed context. These are invisible contracts that break silently when one side is migrated. + +## Schema + +```yaml +flows: + - name: "" + medium: + key: "" + writer: + unit: + source_loc: "" + reader: + unit: # different unit than writer + source_loc: "" + pairing: + # matched: both writer and reader found in analyzed units + # unmatched-reader: reader found but writer not in any analyzed unit + # unmatched-writer: writer found but reader not in any analyzed unit + # dynamic-key: key is constructed dynamically; static analysis cannot pair + # external-writer: state written by external system (SSO, reverse proxy, etc.) + must_preserve: + must_confirm: # required when pairing != matched + verification_hint: "" + notes: "" +``` + +## Key Fields + +- **`medium`**: how state is passed. Critical for migration — different target frameworks handle these differently (e.g., `threadlocal` may not exist in async/reactive targets). +- **`pairing`**: whether static analysis found both ends of the flow. + - `matched` — both sides identified, flow is fully understood. + - `unmatched-reader` / `unmatched-writer` — one end missing from analysis. Requires `must_confirm: runtime`. + - `dynamic-key` — key is computed at runtime, cannot statically pair. Requires `must_confirm: runtime`. + - `external-writer` — state injected by something outside the codebase (SSO provider, reverse proxy headers, etc.). Requires `must_confirm: runtime`. +- **`must_confirm: runtime`**: hard flag — this flow must be verified with runtime testing because static analysis cannot guarantee correctness. Prevents Failure Mode 15. +- **`must_preserve`**: when true, the state-passing mechanism must be replicated in the target. Prevents Failure Mode 14. + +## Filter Key + +Implementation agent for unit X reads flows where `writer.unit == X` or `reader.unit == X`. + +## Self-Check + +- Every flow with `pairing != matched` carries `must_confirm: runtime`. +- Every flow has valid `writer.unit` and `reader.unit` that exist in `unit_graph.yaml` (or are marked external). +- No duplicate flows (same writer + reader + key). diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/deployment.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/deployment.md deleted file mode 100644 index 4f1bcd6..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/deployment.md +++ /dev/null @@ -1,9 +0,0 @@ -Analyze deployment configuration. Focus on: -- Dockerfiles and container configuration -- Kubernetes manifests / Helm charts -- CI/CD pipeline definitions (GitHub Actions, Jenkins, GitLab CI, etc.) -- Environment variables and secrets management -- Infrastructure as Code (Terraform, CloudFormation, etc.) -- Monitoring/logging configuration - -Output: `./deployment.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/extraction-signals.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/extraction-signals.md new file mode 100644 index 0000000..86cd75a --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/extraction-signals.md @@ -0,0 +1,129 @@ +# Extraction Signals + +Use this reference as a signal checklist while producing the structured architecture artifacts. Do not produce prose inventory reports. + +## Signal → artifact map + +```text +signal area write findings into +---------------------- ------------------------------------------------------------ +project structure unit_graph.yaml, shared_modules.yaml, migration_boundary.yaml +state and routing unit_graph.yaml, bindings.yaml, cross_unit_state.yaml, behavior.yaml +UI components unit_graph.yaml, bindings.yaml, behavior.yaml, shared_modules.yaml +business logic behavior.yaml, wire_contracts.yaml, cross_unit_state.yaml +API surface wire_contracts.yaml +integration points wire_contracts.yaml, seams.yaml, cross_unit_state.yaml +data model wire_contracts.yaml, bindings.yaml, shared_modules.yaml, cross_unit_state.yaml +tech stack bindings.yaml, wire_contracts.yaml, migration_boundary.yaml +``` + +## Project structure signals + +Look for: +- top-level modules, packages, apps, and deployable boundaries +- framework entrypoints: `main`, routes, web descriptors, Vite/Next/GWT config, Struts/Spring config, CLI commands, scheduled jobs +- functional areas and domains +- layer structure when it affects unit boundaries +- generated, build, or vendor directories to exclude from source anchors + +Record entrypoints, routes/pages/jobs/CLI/API surfaces, module boundaries, dependencies, exported signatures, shared files/packages, and runtime-reachable files. + +## State and routing signals + +Look for: +- router config, URL patterns, route guards, redirects +- UI state stores, session/local storage, cookies, request/session attributes, thread/static/framework implicit state +- event handlers that trigger route or state changes +- dynamic route construction and framework conventions + +Record route declarations, route params, query params, UI event bindings, framework state bindings, cross-unit state flows, user-visible navigation behavior, redirects, loading/error states, and branch-specific outcomes. + +## UI component signals + +Look for: +- page/component tree only where it affects unit boundaries or shared module decisions +- event handlers and data flow +- forms, validation, error states, loading states +- visible text/selectors relied on by tests or users + +Record UI pages/components that are externally triggerable or unit boundaries, props/events/selectors/template bindings, framework directives, data-testid/test-visible selectors, runtime config, user actions, visible states, conditional rendering, validation/error/loading behavior, side effects, and shared UI utilities/components/hooks/stores. + +## Business logic signals + +Look for: +- externally triggered behavior in the unit's source anchors and directly called domain/service methods +- conditional paths, validation branches, early returns, redirect/navigation decisions +- writes, notifications, cache/session mutations, audit logs, external calls triggered by behavior +- exceptions, validation failures, fallback paths, user-visible errors, HTTP/status outcomes +- async jobs, polling, transactions, locks, retries, timeouts +- business state written by one unit and read by another + +Record each behavior with `source_loc`, `must_preserve`, and target evidence expectations where applicable. Prefer branch-level contracts over prose descriptions. Do not add rewrite priority, target framework suggestions, or implementation approach. + +Example shape inside `units//behavior.yaml`: + +```yaml +unit: order_checkout +branches: + - source_loc: src/order/CheckoutAction.java:42 + condition: "cart.isEmpty()" + behavior: "return to cart page with validation message" + must_preserve: true +side_effects: + - source_loc: src/order/CheckoutService.java:88 + effect: "creates audit log entry after successful payment authorization" + must_preserve: true +error_paths: + - source_loc: src/order/PaymentClient.java:117 + trigger: "payment gateway timeout" + outcome: "surface retryable checkout error; order remains pending" + must_preserve: true +``` + +## API surface signals + +Look for: +- REST/SOAP/GraphQL/gRPC routes, methods, params, request bodies, response bodies, status/error contracts +- public RPC/service methods, CLI inputs/outputs, web routes, form posts +- auth/authorization requirements that are part of the wire contract +- DTO serialization names and versioning rules +- existing OpenAPI/Swagger/proto/schema files + +For each contract, include source locations and whether the contract is frozen, must be preserved, or may change by user intent. + +## Integration point signals + +Look for: +- external HTTP/REST/SOAP client calls +- message producers and handlers: Kafka, RabbitMQ, SQS, JMS, etc. +- third-party SDKs: payment, email/SMS, OAuth, analytics +- file transfer/storage calls: S3, FTP/SFTP, NFS +- resilience behavior: timeout, retry, circuit breaker, idempotency, service discovery + +Record HTTP/SOAP/gRPC/message/file/SDK contracts, auth, payload shape, retry/error semantics, deliberate partial-migration cuts where one side is frozen, and implicit state created by integrations. + +Do not document deployable runtime resources here; build/deploy/runtime topology belongs outside architecture analysis. + +## Data model signals + +Look for: +- entity/DTO/model classes and their source locations +- table/collection names, key fields, relationships, enum/string-value contracts +- validation rules and serialization names that must appear in the target +- dynamic model access patterns: `get("field")`, maps, reflection + +Record persisted schemas, external data contracts, ORM annotations/XML mappings, validation annotations, serialization aliases, framework binding names, shared DTO/entity/base-model classes, and implicit data passed through session/request/thread/static state. + +Avoid domain redesign, aggregate decisions, bounded-context decisions, or target schema recommendations. + +## Tech stack signals + +Look for: +- framework/runtime versions and plugins +- routing/UI/server framework conventions +- build tooling and module packaging only when it affects runtime reachability or migration boundary +- serialization, ORM, messaging, auth, validation, and i18n libraries + +Record framework wiring, annotations, XML/config bindings, runtime config, dependency-injection hooks, protocol/framework-specific contracts, relevant source/target idiom guide, and build/runtime constraints that genuinely force boundary expansion. + +Do not recommend target stack choices here. Record only existing facts and constraints. diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/infrastructure.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/infrastructure.md deleted file mode 100644 index f66662f..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/infrastructure.md +++ /dev/null @@ -1,11 +0,0 @@ -Analyze runtime resource dependencies and test infrastructure requirements. Focus on: -- **Database dependencies**: DB type, version, schema management tool (Flyway/Liquibase/manual), connection pool config -- **Messaging**: message queues, event buses (Kafka, RabbitMQ, ActiveMQ, etc.) -- **Caching**: Redis, Memcached, in-process caches -- **External services**: third-party APIs, payment gateways, email services, OAuth providers -- **File storage**: local filesystem assumptions, S3, NFS mounts -- **Test execution requirements**: Which dependencies require a real service to run integration tests? Which use in-memory/mock alternatives? Is Docker/Testcontainers currently used? - -> ⚠️ **Document the existing infrastructure only.** Do not recommend target infrastructure choices or test strategies. - -Output: `./infrastructure.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/integration-points.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/integration-points.md deleted file mode 100644 index 50d1414..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/integration-points.md +++ /dev/null @@ -1,10 +0,0 @@ -Analyze external integrations and service boundaries. Focus on: -- External HTTP/REST/SOAP client calls -- Message queue producers/consumers (Kafka, RabbitMQ, SQS, etc.) -- Cache usage (Redis, Memcached, in-memory) -- File storage (S3, local filesystem, FTP) -- Email/SMS/notification services -- Third-party SDK integrations -- Service discovery and circuit breaker patterns - -Output: `./integration-points.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/migration-boundary.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/migration-boundary.md new file mode 100644 index 0000000..34dbc55 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/migration-boundary.md @@ -0,0 +1,49 @@ +# Migration Boundary Artifact + +This reference defines `migration_boundary.yaml`, the rewrite scope contract consumed by planning and implementation. + +### `migration_boundary.yaml` — minimal runnable boundary + rewrite scope contract + +**Prevents**: inventory-driven over-rewrite — the implementation phase treats every discovered framework file or `source_anchor` as a rewrite target, even when acceptance can be satisfied by a smaller runtime-reachable cut. + +**Default policy**: choose the smallest runtime boundary that satisfies the user's acceptance criteria. Expand only when user intent or technical constraints require it. + +**Intent interpretation**: +- Distinguish **transformation acceptance** from **cleanup acceptance**. A migration/rewrite request defines the target behavior, runtime, interface, or deployment state; it does not automatically define a repository-hygiene goal. +- `rewrite/migrate/convert to `, `tests/e2e pass`, `preserve behavior` → `cleanup_required: false` and `strategy: minimal_runtime` by default. +- `cleanup_required: true` only when the user explicitly asks for removal or cleanliness (for example: remove the old stack, no legacy residue, no old runtime, clean rewrite, delete old implementation), or when cleanup is necessary to satisfy a concrete build/runtime/contract/packaging/deployment constraint. +- long-term maintainability/refactor language without clean-removal requirement → `strategy: phased`: first satisfy the target runtime/contract boundary, then defer cleanup. +- technical blockers may expand the boundary only with scoped evidence. Cite the specific mechanism that forces expansion: build graph, runtime loader, ABI/API contract, packaging rule, deployment topology, data ownership, or cutover constraint. Generic claims such as "old and new stacks cannot coexist" are insufficient. + +```yaml +user_intent: + raw_request: "Rewrite this Vue SPA to React 18; E2E tests pass." + inferred_acceptance: ["existing E2E tests pass", "preserve behavior"] + cleanup_required: false +strategy: minimal_runtime # minimal_runtime | full_rewrite | phased +runtime_reachable: # files reachable from the target runtime entrypoint after the cut + - src/client/main.js + - src/client/router/index.js +must_rewrite: # implementation scope; DAG consumes this list, not source_anchors + - path: src/client/main.js + reason: "target runtime entrypoint" + - path: src/client/App.vue + reason: "root component, Vue-specific SFC" + - path: src/client/router/index.js + reason: "Vue Router → React Router rewrite required" +copy_as_is: + - src/client/api/client.js + - src/client/utils/formatting.js +legacy_allowed_to_remain: # files may remain if not runtime-reachable and acceptance does not require cleanup + - src/client/views/LegacyView.vue + - src/client/components/LegacyWidget.vue +defer_cleanup: + - "Remove unused Vue SFCs after runtime cutover is verified." +full_rewrite_reason: null # required when strategy == full_rewrite +implementation_rule: > + Implementation tasks use must_rewrite plus copy_as_is as needed. + They MUST NOT infer rewrite scope from all source_anchors or all framework-specific files. +``` + +**Self-check**: `source_anchors` never used as rewrite scope; every `must_rewrite` row explains why it is runtime-reachable or required by acceptance; every `legacy_allowed_to_remain` row is either unreachable from the target entrypoint or explicitly deferred; `cleanup_required` is false unless explicitly requested by the user or forced by a concrete build/runtime/contract/packaging/deployment constraint; `full_rewrite_reason` present when `strategy: full_rewrite` and cites the specific forcing mechanism, not a generic coexistence claim. + diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/migration-risks.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/migration-risks.md deleted file mode 100644 index 383ac61..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/migration-risks.md +++ /dev/null @@ -1,10 +0,0 @@ -Analyze the codebase for complexity and migration risk. Focus on: -- Business logic complexity per module (lines of code, cyclomatic complexity, coupling) -- Framework-specific patterns tightly intertwined with business logic (hard to extract) -- Session and auth patterns (how user state is managed, security enforcement points) -- Test coverage gaps in high-complexity areas -- Risk classification per module: LOW / MEDIUM / HIGH / CRITICAL - -> ⚠️ **Assess complexity and risk of the existing code only.** Do not suggest how to rewrite or refactor. - -Output: `./migration-risks.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/seams.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/seams.md new file mode 100644 index 0000000..55fca86 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/seams.md @@ -0,0 +1,58 @@ +# Seams Artifact + +This reference defines `seams.yaml`, the partial-migration cut-point and bridge-design contract. + +### `seams.yaml` — partial-migration cut points + bridge design + +**Prevents**: the dominant partial-migration failure — a migrated unit silently breaks where it meets the un-migrated remainder, because the *cut point*, *which side is frozen*, and the *conversion rule* were never recorded. wire_contracts records outward contracts and cross_unit_state records implicit state; **seams records the deliberate knife — where you cut, which side is frozen, and how the bridge converts across it.** + +**A seam is a one-place model of a cut.** Each seam answers: where is the cut, what is on each side, which side is frozen (must NOT be refactored), and — when the two sides speak different protocols/idioms — the exact conversion the bridge performs. + +**What a seam adds that reading the source does not.** Validated empirically (cargotracker routing closed-loop): when the frozen side's source is **visible**, any migrating agent that reads it recovers the behavioral contract on its own — re-stating it as a `frozen_contract` is redundant. The seam's irreducible value is the two things grep/read cannot give you: +- `frozen_side_rule` — the *decision* that this side is frozen and must NOT be refactored/recompiled. Not in the source; it's a phase boundary the architect declares. +- `bridge_points` — the *protocol/idiom conversion design* across the cut (param decomposition, response reconstruction, edge-case handling). A design commitment, not a fact extractable from either side. + +`frozen_contract` is therefore **conditional, not standard issue** (see field rule below). + +**Two sources — `declared` is authoritative, `inferred` is advisory**: +- `declared`: user/architect specified this cut point. Authoritative — design may NOT overrule it. +- `inferred`: analyze discovered a likely seam. Advisory — a candidate for design/user to confirm. +- On conflict (a declared seam contradicts an inferred one), `declared` wins; drop or fold the inferred row, note it. + +**Discovery signals for `inferred` seams** (recognize by judgment, provenance not a gate): protocol boundary (rpc↔rest, sync↔async), framework boundary (Struts action ↔ Spring controller), layer boundary where you intend to keep the old service, external-system edge, and the **reference cliff** in `shared_modules.yaml` (a shared module heavily used on one side, barely on the other). + +```yaml +seams: + - id: src/com/acme/inventory/InventoryService.java:1 # cut point = natural ID + description: "Order service migrated to REST; inventory stays on legacy gRPC, frozen." + source: declared # declared (authoritative) | inferred (advisory) + cut_between: + migrated_side: unit_order_create # being rewritten now + frozen_side: legacy_inventory # stays as-is this phase + protocol_shift: {from: grpc, to: rest} # null when same protocol — bridge is then a thin adapter + frozen_side_rule: "legacy_inventory MUST NOT be refactored or recompiled this phase." + frozen_contract: # CONDITIONAL — emit ONLY when the frozen side's source is NOT visible to the migrating agent, OR a semantic cannot be recovered from the source it can read (private/obfuscated binary dependency, behavior gated by config/data not in source, a contract the public name contradicts). When the frozen source IS visible and self-explanatory, OMIT this — the migrating agent reads it directly; restating it here is redundant context cost. Default to omitting. + - method: "reserve(orderId: string, items: Item[]) -> ReservationResult" + source_loc: InventoryService.java:88 + semantics: > + synchronous; throws InsufficientStockException(stockShortfall); + idempotent on orderId (re-reserve returns same ReservationResult). + must_preserve: true + bridge_points: # the conversion design — ARCHITECT commits it; migrating agent does not invent it + - at: "order_create → inventory call site, OrderController.java:142" + from_form: "gRPC InventoryService.Reserve(ReserveRequest{order_id, repeated Item})" + to_form: "POST /legacy/inventory/reservations body={orderId, items[]}" + mapping_rule: > + ReserveRequest.order_id → body.orderId; + repeated Item{sku,qty} → items[]{sku,qty}; + ReservationResult.reservation_id → 201 Location header. + edge_cases: + - "empty items → 400 (NOT gRPC INVALID_ARGUMENT passthrough)" + - "InsufficientStockException → 409 with {sku, shortfall}" + idempotency_retry: "orderId is the idempotency key; bridge dedupes; safe to retry the POST." + fallback: "bridge timeout → surface 504; do NOT auto-retry the write." + must_preserve: true +``` + +**Self-check**: every `declared` seam present; every seam has a `frozen_side_rule`; `frozen_contract` present **only** where the frozen source is invisible/unrecoverable (absent otherwise — its absence is correct, not a gap); every `protocol_shift != null` seam has ≥1 `bridge_point` with a concrete `mapping_rule` (no `TBD`); declared/inferred conflicts resolved in favor of declared. + diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/shared-modules.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/shared-modules.md new file mode 100644 index 0000000..2168a8e --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/shared-modules.md @@ -0,0 +1,52 @@ +# shared_modules.yaml — Cross-Unit Shared Code Registry + +Records source files/modules used by ≥2 units. Prevents duplication, god-class field drift, and hallucinated fields during migration. + +## Schema + +```yaml +modules: + - name: # logical name for the shared module + kind: + source_loc: "" + used_by_units: # which units reference this module + - + fields: # all public/accessible fields/methods with types + - name: + type: "" + god_class: # true when the module has too many responsibilities + split_candidate: # true when god_class or high fan-out suggests splitting + migration_strategy: + # extract: pull into its own module/package + # wrap: wrap behind an interface for the target + # split: break into cohesive pieces (when god_class/split_candidate) + # copy: copy as-is (stable utility) + # inline: inline into consuming units (small, single-purpose) + notes: "" +``` + +## Relationship to Per-Unit `shared_refs` + +Each unit's `shared_refs` in `unit_graph.yaml` is a **subset whitelist** of this file: +- `shared_refs[].module` must match a `name` in this file. +- `shared_refs[].used_fields` must be a subset of this file's `fields` for that module. + +This two-level design ensures: +1. The global file is the single source of truth for what the module contains. +2. Each unit declares only the fields it actually uses (preventing hallucinated field access). + +**Self-check constraint**: `∀ unit U, ∀ ref in U.shared_refs: ref.used_fields ⊆ {f.name for f in shared_modules[ref.module].fields}` + +## Key Fields + +- **`god_class`**: flags modules with too many responsibilities. Prevents Failure Mode 16 (god-class field drift). +- **`split_candidate`**: advisory flag for design phase. Does not trigger automatic splitting — design decides. +- **`migration_strategy`**: how to handle during migration. This is a recommendation, not a gate. +- **`used_by_units`**: the filter key. Implementation agent for unit X reads rows where `used_by_units` includes X. + +## Self-Check + +- Every module listed in any unit's `shared_refs` exists in this file. +- No module has `used_by_units` with only one unit (by definition, shared = ≥2). +- Every `god_class: true` module has `split_candidate: true`. +- `fields` lists are non-empty. diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/state-routing.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/state-routing.md deleted file mode 100644 index 2b050cf..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/state-routing.md +++ /dev/null @@ -1,9 +0,0 @@ -Analyze frontend state and navigation. Focus on: -- State management solution (Redux, Vuex, Pinia, Zustand, MobX, etc.) -- Global vs local state patterns -- Route definitions and navigation structure -- Route guards / middleware -- Code splitting and lazy loading -- Data fetching patterns (REST hooks, GraphQL queries, SWR, React Query, etc.) - -Output: `./state-routing.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/test-coverage.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/test-coverage.md deleted file mode 100644 index 1d73100..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/test-coverage.md +++ /dev/null @@ -1,12 +0,0 @@ -Analyze the existing test suite. Focus on: -- **Test inventory**: Count and categorize tests (unit / integration / e2e / performance) -- **Coverage baseline**: Which packages/classes have tests? Which are untested? -- **Test framework**: JUnit 4/5, TestNG, Mockito, Spring Test, etc. -- **Integration test dependencies**: What real services do integration tests require? (DB, message broker, external APIs) -- **Test data strategy**: fixtures, DBUnit datasets, factory methods, inline mocks -- **Portability assessment**: Which tests are tightly coupled to the current framework? Which test only business logic independently of the web layer? -- **Gaps**: Business-critical areas with no test coverage - -> ⚠️ **Document the existing test suite only.** Do not recommend target test strategies or frameworks. - -Output: `./test-coverage.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/ui-components.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/ui-components.md deleted file mode 100644 index 9cb97af..0000000 --- a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/ui-components.md +++ /dev/null @@ -1,9 +0,0 @@ -Analyze frontend component architecture. Focus on: -- Component tree and hierarchy -- Shared/reusable components vs page-specific ones -- Design system / component library usage (Material UI, Ant Design, etc.) -- Styling approach (CSS modules, Tailwind, styled-components, SCSS, etc.) -- Form handling patterns -- Accessibility patterns - -Output: `./ui-components.md` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/unit-decomposition.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/unit-decomposition.md new file mode 100644 index 0000000..aacecf7 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/unit-decomposition.md @@ -0,0 +1,34 @@ +# Unit Decomposition Artifact + +This reference defines `units//unit_decomposition.yaml`, the split-candidate artifact for design. + +### `units//unit_decomposition.yaml` (per-unit) + +**Prevents**: analyze prematurely committing to target unit count; design losing the rationale for split candidates. + +**Hard rule**: produces `candidate_splits`, NOT `target_units`. Whole file is design-owned (`commit: false`, self-checked); no per-row decision markers. + +**Split drivers — recognize these** (vocabulary; ≥1 per candidate, classify by judgment): +``` +protocol_split concern_split execution_model_split lifecycle_split +reuse_split data_ownership_split change_cadence_split nfr_split +``` + +```yaml +unit: order_processing +commit: false # HARD: self-checked. Whole file is candidates, not decisions. +candidate_splits: + - id: src/com/acme/order/OrderAction.java:42-120 + drivers: [concern_split, execution_model_split] + rationale: > # MANDATORY ≥40 chars; self-checked + handleRequest interleaves synchronous validation (42-78) with async inventory + reservation (80-120): two execution models, two failure semantics. + source_slice: {file: src/com/acme/order/OrderAction.java, lines: [42, 120]} + +# _meta only when candidate count is unusually high (smell, NOT a truncation trigger): +# _meta: {candidate_count: 11, note: "high split count — unit likely under-defined; revisit boundary"} +``` + +**No `composite_score`**: ranking candidates is design's job — it has cross-unit/strategic context analyze lacks. Emit **all** candidates with drivers+rationale. + +Cohesion machinery such as LCOM4/TCC, co-access clusters, and method-field matrices is not produced here. Structural numbers did not change design decisions. Candidate seams belong in `seams.yaml`; field-level cohesion concerns, when real, surface as `split_candidate` on `shared_modules.yaml`. diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/unit-graph.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/unit-graph.md new file mode 100644 index 0000000..ec34ab7 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/unit-graph.md @@ -0,0 +1,54 @@ +# unit_graph.yaml — Global Unit Index + +The spine of all architecture artifacts. Every per-unit artifact set and every global-artifact filter key derives from this file. + +## Schema + +```yaml +units: + - name: # kebab-case, unique across the graph + trigger: # external trigger category + trigger_detail: "" + source_anchors: # files that prove this unit exists — NOT rewrite targets + - path: + line: # optional entry-point line + exported_signature: # public signatures other units depend on + - method: "" + source_loc: "" + stability: # frozen = exact shape required by callers + dynamic_entrypoints: # classes/methods reached only via reflection, DI, XML, or convention + - target: "" + mechanism: + source_loc: "" # where the dynamic reference is declared + depends_on: # other units this unit calls + - unit: + via: "" + shared_refs: # per-unit subset of shared_modules this unit uses + - module: # must exist in shared_modules.yaml + used_fields: # subset; names must ⊆ shared_modules[module].fields[].name + - +``` + +## Key Fields + +- **`name`**: the join key used by all per-unit directories (`units//`) and global-artifact row filters (`used_by_units`, `unit`, `writer.unit`/`reader.unit`, `cut_between`). +- **`source_anchors`**: evidence that a unit exists. Rule: `source_anchors ≠ rewrite targets`. Implementation scope comes from `migration_boundary.yaml::must_rewrite`. +- **`exported_signature`**: resolved from public method/route signatures only. Prevents Failure Mode 4 (broken caller / signature unsync). +- **`dynamic_entrypoints`**: anything reached via reflection, DI container scan, XML bean definition, naming convention, or annotation processing. Prevents Failure Mode 5 (dead-code removal of reflection/DI class). Include the `mechanism` so the implementation agent knows *how* the class is discovered. +- **`shared_refs`**: this is where per-unit shared-module usage lives. Each entry points to a module in `shared_modules.yaml` and lists the subset of fields this unit actually uses. Self-check constraint: `shared_refs[].used_fields ⊆ {f.name for f in shared_modules[module].fields}`. +- **`depends_on`**: inter-unit call edges. Combined with `exported_signature`, this lets the implementation agent preserve call-site contracts. + +## Uniqueness Invariant + +Each source file appears in at most one unit's `source_anchors`. Files used by ≥2 units belong in `shared_modules.yaml`. + +## Self-Check + +```bash +# unit count (used by per-unit completeness gate) +grep -c '^\s*- name:' artifacts/unit_graph.yaml + +# no duplicate source_anchors across units +grep 'path:' artifacts/unit_graph.yaml | sort | uniq -d +# expect: empty output +``` diff --git a/plugins/github-copilot-modernization/skills/analyzing-architecture/references/wire-contracts.md b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/wire-contracts.md new file mode 100644 index 0000000..f78a0e8 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/analyzing-architecture/references/wire-contracts.md @@ -0,0 +1,46 @@ +# wire_contracts.yaml — External Interface Contracts + +Records outward-facing contracts: REST/SOAP/GraphQL/gRPC endpoints, message queues, SQL/data interfaces, and cross-language semantic traps. Implementation agents use this to preserve exact API shapes and catch semantic divergence. + +## Schema + +```yaml +contracts: + - name: "" + unit: # join key to unit_graph; may be "global" for app-wide contracts + type: + stability: + source_loc: "" + contract: # wire shape + method: "" # when applicable + path: "" + request: "" + response: "" + error: "" + auth: "" + target_contract: # required target-side form; TBD forbidden + method: "" + notes: "" + semantic_divergence: # cross-language / cross-framework gotchas + - field: "" + issue: "" + source_loc: "" +``` + +## Key Fields + +- **`unit`**: the filter key. Implementation agent for unit X reads only rows where `unit == X` (or `unit == global`). +- **`stability: frozen`**: this contract shape must be preserved exactly (Failure Mode 6). +- **`target_contract`**: the required target-side signature. `TBD` is forbidden — if the target form cannot be determined during analysis, the seam/bridge must handle it. +- **`semantic_divergence`**: cross-language gotchas (e.g., Java `null` vs Kotlin non-null, date format drift, enum ordinal vs name). Prevents Failure Mode 8. + +## Scope Boundary + +- **Included**: REST, SOAP, GraphQL, gRPC, message queues (Kafka/JMS/RabbitMQ/SQS), SQL contracts (stored procedures, cross-service queries), file/SDK integration contracts. +- **Excluded**: internal method calls within a unit (those are `behavior.yaml`), ORM entity mappings (those are `bindings.yaml` unless they define a cross-service data contract), deploy/infra topology. + +## Self-Check + +- Every `stability: frozen` row has a non-TBD `target_contract`. +- Every row has a valid `unit` that exists in `unit_graph.yaml` (or is `global`). +- `semantic_divergence` entries have `source_loc`. diff --git a/plugins/github-copilot-modernization/skills/api-service-contracts/SKILL.md b/plugins/github-copilot-modernization/skills/api-service-contracts/SKILL.md index 0b89114..1b109f9 100644 --- a/plugins/github-copilot-modernization/skills/api-service-contracts/SKILL.md +++ b/plugins/github-copilot-modernization/skills/api-service-contracts/SKILL.md @@ -11,6 +11,44 @@ Analyze the project to document all services, API endpoints, communication patte - `workspace-path` (optional): Path to the project to analyze (defaults to current directory) +## ⚠ Mermaid Safety Constraints — read BEFORE you write the ```mermaid block + +Mermaid sequenceDiagram is unforgiving in a few specific ways: one bad alias or one missing `end` crashes the **whole** diagram with `Syntax error in text`, not just the offending line. Stay strictly inside this subset for the sequence diagram in Step 7: + +1. **Chart kind.** `sequenceDiagram` only. Never `sequence-diagram`, never `sequence`. +2. **Participants.** Always declare with the alias form `participant as "Display Label"`. The id must match `[A-Za-z][A-Za-z0-9_]*`. Never omit the id — even a one-word participant should be `participant Client as "Client"`. This is the single biggest cause of past failures. +3. **Arrows.** + - `->>` synchronous request + - `-->>` synchronous response (or async return) + - `-)` async fire-and-forget + - Message text goes after `:` and is plain text — keep it short and on one line. +4. **Blocks.** `alt` / `else` / `opt` / `loop` / `par` / `critical` MUST be closed by `end` on its own line. Every open block must have a matching `end`. Missing `end` is the #2 cause of past failures. +5. **No line breaks anywhere.** The escape `\n` was removed in modern Mermaid. Aliases, message text, and `Note over` content must all be single-line. Split a long note into multiple consecutive `Note over` lines; split a long message into multiple arrows. This is the #1 cause of past failures. +6. **Banned characters inside participant aliases specifically** (message text is more permissive — only `\n` is banned there): + + | Banned in alias | Why it breaks | Replacement | + |---|---|---| + | `\n` (literal two chars) | escape removed | drop | + | `"` (a second double-quote) | closes the alias early | `'` (single quote) | + | `` ` `` (backtick) | breaks alias quoting | drop | + | smart quotes `"` `"` `'` `'` | not ASCII | regular `"` and `'` | + | `:` | confuses with message delimiter | rephrase, e.g. `"REST API (port 8080)"` not `"REST API: port 8080"` | + | `
` | not interpreted inside aliases | rephrase as shorter alias | + +7. **Quote the alias.** `participant Svc as "Order Service"` — never `participant Svc as Order Service` (unquoted multi-word aliases break). + +### Mandatory self-attestation + +Immediately before writing the ` ```mermaid ` opening fence in Step 7, emit this exact one-line HTML comment in the markdown (it does not render — it is for your own visible attestation): + +``` + +``` + +If you cannot truthfully emit that comment, fix the diagram first. + +--- + ## Scope Boundaries — Avoid Redundancy with Other Skills This skill is part of a set of four complementary assessment skills. To avoid content duplication across their output documents, observe these scope rules: @@ -103,18 +141,19 @@ For each service, identify which cross-cutting capabilities it uses and produce ### Step 7: Generate Service Communication Sequence Section -Create a **Mermaid `sequenceDiagram`** and produce the complete `## Service Communication Sequence` section: +Create a **Mermaid `sequenceDiagram`** and produce the complete `## Service Communication Sequence` section (re-read the Safety Constraints above before writing): - Show key actors: Client, API Gateway (if present), Controllers, Services, External Services, Message Brokers - Annotate synchronous calls with solid arrows and asynchronous calls with dashed arrows - Include request/response types where relevant - Show error handling paths for critical flows (circuit breaker, retry) - For gateway aggregation flows, show how multiple downstream calls are composed -Example: +Reference example (this block satisfies every Safety Constraint — match its shape): + ~~~mermaid sequenceDiagram - participant Client + participant Client as "Client" participant Gateway as "API Gateway" participant CustSvc as "Customers Service" participant VisitSvc as "Visits Service" @@ -183,36 +222,23 @@ A brief introduction (1-2 sentences) summarizing the API surface and communicati - Aggregate similar endpoints (e.g., CRUD operations on the same resource) into one table row if needed for brevity - For the service technology matrix, use checkmarks or short labels; omit columns where no service uses the capability -## Mermaid Syntax Rules - -The diagram must parse cleanly under **Mermaid >= 9.x**. Anything outside the legal subset crashes the entire diagram with `Syntax error in text`. - -- Use `sequenceDiagram` -- Avoid special characters (`@`, `#`, `$`, `%`, `&`) in participant labels — use plain text or quoted labels -- Use `->>` for synchronous calls and `-->>` for responses/async messages -- Use `participant` with alias syntax for readable labels: `participant Svc as "OrderService"` -- Use `alt`/`else`/`end` blocks to show circuit breaker fallback paths -- Do not use backticks inside node labels - -### Line breaks — HARD RULE - -- **NEVER use `\n` for line breaks inside participant aliases, messages, or notes.** The literal `\n` escape was removed in modern Mermaid and triggers "Syntax error in text". -- In participant aliases: keep them on a single line, e.g. `participant Svc as "Order Service"` — not `"Order\nService"`. -- In `Note over` / `Note right of`: keep the note on one line, or split into multiple `Note` statements. -- In message arrow labels: keep concise; if you need multiple facts, split into multiple arrows. -- ❌ `participant API as "REST API\n(SubsonicController)"` -- ✅ `participant API as "REST API (SubsonicController)"` +## Common failure patterns observed in past runs -### Self-check before emitting each ```mermaid block +Each row below is something the model actually produced that crashed the diagram. Use the ✅ form. -1. Search the block for the two characters `\n` — remove or split the line. Zero `\n` must remain. -2. Confirm every `alt`/`opt`/`loop`/`par` block is closed by `end`. -3. Confirm every quoted alias is on a single line. +| ❌ Past mistake | ✅ Safe form | Why the ❌ crashed | +|---|---|---| +| `participant API` (no alias) | `participant API as "API"` | Bare participants with later spaces in usage break | +| `participant API as "REST API\n(SubsonicController)"` | `participant API as "REST API (SubsonicController)"` | Literal `\n` in alias | +| `participant API as "REST API: port 8080"` | `participant API as "REST API (port 8080)"` | `:` in alias collides with message delimiter | +| `Note over Client,API: First fact\nSecond fact` | Two consecutive `Note over Client,API: ...` lines | `\n` in note text | +| `alt happy path` ... missing `end` | `alt happy path` ... `end` | Unclosed block | +| `participant Svc as Order Service` (no quotes) | `participant Svc as "Order Service"` | Multi-word alias must be quoted | ## Error Handling - **Unsupported project type**: Output a single line: `> ERROR: Unsupported project type. This skill supports Java, .NET, JavaScript, and TypeScript projects only.` -- **No API endpoints found**: Output: `> ERROR: No recognized API endpoints found at {workspace-path}. Verify the path is correct.` +- **No API endpoints found**: Output: `> ERROR: No recognized API endpoints found at workspace-path. Verify the path is correct.` - **Insufficient info**: Generate a best-effort document from available data. Add a note: `> Note: Some endpoints or communication patterns could not be fully identified.` ## Success Criteria @@ -224,4 +250,5 @@ The diagram must parse cleanly under **Mermaid >= 9.x**. Anything outside the le - Service technology matrix shows per-service capabilities - Communication patterns section describes sync/async patterns, resilience policies, and security posture (authentication, authorization, TLS — explicitly stating if none is configured) - Mermaid sequence diagram renders correctly showing primary request flow with aggregation and fallback +- The ```mermaid block is preceded by the `` attestation comment - File saved to `.github/modernize/assessment/engines/facts/api-service-contracts.md` diff --git a/plugins/github-copilot-modernization/skills/appmod-hooks/SKILL.md b/plugins/github-copilot-modernization/skills/appmod-hooks/SKILL.md index d4847f8..670b9ac 100644 --- a/plugins/github-copilot-modernization/skills/appmod-hooks/SKILL.md +++ b/plugins/github-copilot-modernization/skills/appmod-hooks/SKILL.md @@ -23,6 +23,7 @@ See `references/actions.yml` for the action registry. Actions use dotted namespa ## Execution Rules 1. Actions within a hook point execute **in order** (top to bottom in the registry) -2. An action that fails does NOT block subsequent actions — log the error and continue +2. An action that fails does NOT block subsequent actions — log the error and continue, unless the action explicitly declares itself a quality gate 3. `optional: false` actions MUST execute; `optional: true` actions execute only if their `condition` is met 4. The coordinator executes hook actions **itself** (shell commands for git, file writes for profile) — hooks are NOT delegated to workers +5. A quality-gate action failure blocks dependent dispatch: keep the task pending or create a remediation task, then re-run the hook after the artifact is updated diff --git a/plugins/github-copilot-modernization/skills/appmod-hooks/commands/appmod.board.floor-check.md b/plugins/github-copilot-modernization/skills/appmod-hooks/commands/appmod.board.floor-check.md new file mode 100644 index 0000000..f51f7dc --- /dev/null +++ b/plugins/github-copilot-modernization/skills/appmod-hooks/commands/appmod.board.floor-check.md @@ -0,0 +1,75 @@ +# appmod.board.floor-check + +Quality gate: before any worker is dispatched, ensure the board schedules the mandatory governance fragments that a hand-rolled board commonly drops. Enforces two fragments — **`cve-remediation`** (when the change touches a dependency manifest) and the **completeness / consistency check** (`conformance-review`, plus `feature-parity-signoff` when applicable), the latter **only when the user explicitly requests it**. + +## Purpose + +Fragment selection lives inside `skill(dag-generation)`. When the coordinator hand-rolls the board (common on small / lite projects with `deep_planning=false`), mandatory governance fragments are silently dropped even when warranted. This gate verifies the **outcome** — that the warranted governance task is on the board — **regardless of whether `dag-generation` was invoked**. + +It deliberately checks **only** the governance fragments the LLM tends to skip: + +- **`cve-remediation`** — warranted whenever the change emits or modifies a dependency manifest (implicit; the user need not ask). +- **completeness / consistency** (`conformance-review`, and `feature-parity-signoff` when applicable) — warranted whenever the user **explicitly requests** a completeness / consistency / feature-parity check (any change type: upgrade, migration, rewrite). + +It does NOT check implementation / smoke-test / runtime-validation tasks — those are trusted to the LLM. + +## When + +`before_all`, after the board is written and before the first worker dispatch. + +**Skip the whole gate (run nothing) when:** +- The board contains the deep-planning placeholder `⏳ [Execute + Validate phases — pending deep planning completion]`. The execute+validate tail (including `cve-remediation`, `conformance-review`, and `feature-parity-signoff`) is generated later in §3.2.2; this gate is **re-run there** — once §3.2.2 replaces the placeholder with the real execute+validate tasks, the coordinator re-invokes this floor-check against the now-complete board. So skipping here is safe: the arms are evaluated at §3.2.2, not lost. + +## Inputs available from coordinator context + +- `{{BASE_PATH}}/board.md` — the task list. +- `{{BASE_PATH}}/artifacts/project-profile.yaml` — `assessment.change_type`, `assessment.transformations`, `project` notes. +- The user's original request (`## User Input` in `board.md`). + +## Required behavior + +0. **Verify the board exists.** Confirm `{{BASE_PATH}}/board.md` is present and non-empty. At `before_all` it must already be written (it is the artifact the coordinator is about to dispatch from). If it is **missing or empty**, this is an upstream failure, not a pass condition — fail the gate and block dispatch with: `board.md is missing or empty at before_all; the board must be generated before any worker is dispatched.` Do NOT treat an absent board as "nothing warranted." + +Then run **both** arms below. Reuse each fragment's own `when` / `skip-when` / `override` from `skill(dag-generation)` → `references/task-catalog.md` (do NOT invent new criteria). + +### Arm A — `cve-remediation` (warranted by change nature) + +1. **Decide whether `cve-remediation` is warranted**: + - **Warranted (`when`)**: the planned change will **emit or modify a dependency manifest** (`pom.xml` / `build.gradle` / `*.csproj` / `packages.config` / `package.json` / lockfiles) — true for essentially every brownfield migration / upgrade / rewrite — OR the user mentions security / CVE / vulnerability, OR assessment/arch-analysis flagged vulnerable or EOL dependencies. Judge this from the change nature (`change_type`, `user_ask`, transformations), **not** from a `git diff` — at `before_all` no implementation has run yet, so the working tree shows no manifest change. + - **Not warranted (`skip-when`)**: no dependency manifest is produced or changed (pure config/docs/asset change, or a dependency-free single-file edit); OR the user explicitly opted out of security/CVE work. Do NOT treat **lite scope** as not-warranted — a lite-scope change that still touches a dependency manifest must be scanned (this is exactly the small/lite case this gate exists to catch). + - If **not** warranted → this arm passes silently. +2. **If warranted, check `board.md`** for a `cve-remediation` task. Match case-insensitively on task title / assignment: any task that indicates CVE scanning / vulnerability remediation — e.g. mentions `cve`, `vulnerab`, `dependency scan`, `remediat`, or explicitly invokes `skill(cve-remediation)`. + +### Arm B — completeness / consistency (warranted by explicit user request) + +3. **Decide whether a completeness / consistency check is warranted** — mirror the `dag-generation` "Explicit-request override": + - **Warranted (`when`)**: `user_ask` **explicitly** requests a completeness, consistency, or feature-parity check (e.g. "run a completeness check", "verify nothing was missed / dropped", "enforce consistency", "feature parity sign-off", "make sure the migration / upgrade / rewrite is complete and consistent"). This applies to **any** change type (upgrade, migration, rewrite) — it is the explicit user intent, not the project size or change type, that warrants it. + - **Not warranted**: the user did not explicitly ask for such a check. Implicit completeness is the LLM's to plan; this gate enforces only the **explicit** request. + - If **not** warranted → this arm passes silently. +4. **If warranted, check `board.md`** for a completeness/conformance validation task. Match case-insensitively: any task that indicates completeness / conformance / consistency / feature-parity validation — e.g. mentions `conformance`, `completeness`, `consistency`, `feature parity` / `feature-parity`, or explicitly runs the completeness gate (`skill(quality-gates)` → `references/gate-completeness.md`). When the change is a migration / rewrite that has a `feature-inventory` task, a `feature-parity-signoff` task also counts toward this arm. + +5. **For each warranted arm: present → pass; absent → fail** (see Failure semantics). + +## Pass criteria + +The gate passes when, for **each** arm, either the fragment is not warranted, or the board already contains the corresponding task. + +## Failure semantics + +This action is a **quality gate**. On failure (any warranted arm missing its task) the coordinator MUST NOT dispatch any worker. Append the missing task(s) to `board.md` (do NOT re-run `dag-generation`), then **re-run this floor-check**; dispatch may proceed once it passes. + +**Arm A — append `cve-remediation`** in the Implementation phase (fragment is `after: [implementation]`, `scope: per-group`, implementer/backend role). The task MUST instruct the worker to **use `skill(cve-remediation)`**, so the skill's existing scan→fix→verify workflow is reused — do NOT hand-roll an ad-hoc CVE check. Example board line: + + ```text + - ⏳ t [backend] Scan dependency manifests for CVEs and remediate via skill(cve-remediation) [deps: ] + ``` + +**Arm B — append `conformance-review`** in the Validate phase (fragment is `after: [runtime-validation, test-strategy]`, `scope: global`, teamlead role). The task MUST run the completeness gate (`skill(quality-gates)` → `references/gate-completeness.md`), which performs the change-type-aware consistency check (migration / rewrite → functional-equivalence; upgrade → upgrade-consistency). Example board line: + + ```text + - ⏳ t [teamlead] Completeness & consistency check via skill(quality-gates) gate-completeness [deps: ] + ``` + + When the change is a migration / rewrite with a `feature-inventory` task, also ensure a `feature-parity-signoff` task is present (pm role: verify the feature-inventory checklist is fully covered — no missing endpoints, UI flows, or business rules). + +> Rationale for "append, don't re-plan": the board is trusted for everything else (implementation / smoke-test / validation are the LLM's to plan). This gate surgically restores only the governance tasks that hand-rolled boards drop, rather than regenerating the whole DAG. diff --git a/plugins/github-copilot-modernization/skills/appmod-hooks/commands/appmod.dependency.consumption-check.md b/plugins/github-copilot-modernization/skills/appmod-hooks/commands/appmod.dependency.consumption-check.md new file mode 100644 index 0000000..bfb4774 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/appmod-hooks/commands/appmod.dependency.consumption-check.md @@ -0,0 +1,53 @@ +# appmod.dependency.consumption-check + +Verify that a completed task with dependency artifacts reports how those upstream artifacts were consumed. + +## Purpose + +`dependencyArtifacts` are not just ordering edges. They are input contracts from upstream work. A task that receives dependency artifacts must either consume them or explicitly explain why they were not applicable. + +This action is role-neutral: architecture analysis, feature inventory, API design, test strategy, security findings, UX design, data-model plans, and any other upstream artifact are all treated the same. + +## When + +`after_task`, after the coordinator has verified the worker returned `[DONE]` and the task artifact exists/non-empty, but before dispatching dependent tasks. + +## Inputs available from coordinator context + +- Current task metadata, including the `## Dependency Artifacts` list used for dispatch. +- Current task artifact path, typically `{{BASE_PATH}}/artifacts/-.md`. +- Worker return message, if available. + +## Required behavior + +1. If the current task had **no dependency artifacts**, skip silently. +2. If the only dependency artifact is global clarification context (`clarification.md`), skip silently. The clarification file is a global scenario record, not a task-to-task input contract. +3. Otherwise read the completed task artifact and verify it contains dependency-consumption evidence. +4. Accept any section heading or YAML key whose meaning is "which upstream inputs were consumed / how they map to this output / why an input was not used." Match **case-insensitively and format-insensitively**: ignore Markdown heading markers (`#`/`##`), separator style (spaces, hyphens, underscores), and Title-vs-lower case. The headings the worker spec instructs workers to emit are the canonical forms and MUST match: + - `## Upstream Artifacts Consumed` (canonical — worker output) + - `## Evidence Mapping` (canonical — worker output) + + Equivalent forms that also satisfy the gate (non-exhaustive — judge by meaning, not by literal string): `upstream_artifacts_consumed`, `evidence_mapping`, `Dependencies consumed`, `Dependency artifacts consumed`, `Inputs consumed`, `Constraints applied`, `Dependencies not used`, `Inputs not used`. Do not fail an artifact that expresses consumption/evidence/not-used under a reasonable synonym just because its exact wording is absent from this list. +5. If none are present, mark the hook as failed and return a remediation message: + +```text +Dependency consumption evidence missing. +This task received dependency artifacts but its output does not state which upstream inputs were consumed, which constraints were applied, or why inputs were not used. +Re-run the task or add a remediation task that updates the artifact with: +- Dependency artifacts consumed: -> +- Constraints applied: +- Dependencies not used: -> +``` + +## Pass criteria + +The hook passes when either: + +- no task-specific dependency artifacts were provided; or +- the completed task artifact contains at least one accepted consumption/evidence section/key. + +## Failure semantics + +This action is a quality gate. On failure, the coordinator must not dispatch dependent tasks. Reopen the task as pending or create a remediation task for the same role to update the artifact and, if needed, the implementation/tests. + +Do not judge whether the consumption is semantically correct. This hook only enforces that the worker makes dependency usage explicit. Semantic correctness belongs to review/validation tasks. diff --git a/plugins/github-copilot-modernization/skills/appmod-hooks/references/actions.yml b/plugins/github-copilot-modernization/skills/appmod-hooks/references/actions.yml index 7e3ad62..2414bfd 100644 --- a/plugins/github-copilot-modernization/skills/appmod-hooks/references/actions.yml +++ b/plugins/github-copilot-modernization/skills/appmod-hooks/references/actions.yml @@ -2,7 +2,11 @@ # Each action points to a command file via `file:`. # Namespace: appmod.. -before_all: [] +before_all: + - id: appmod.board.floor-check + file: commands/appmod.board.floor-check.md + description: "Quality gate: ensure board.md schedules the warranted governance fragments — cve-remediation (when a dependency manifest changes) and conformance-review/feature-parity-signoff (when the user explicitly requests a completeness/consistency check); blocks first dispatch and appends the missing task(s) on failure" + optional: false before_task: - id: appmod.profile.read @@ -11,6 +15,11 @@ before_task: optional: false after_task: + - id: appmod.dependency.consumption-check + file: commands/appmod.dependency.consumption-check.md + description: "Gate completed tasks with dependency artifacts: worker output must state upstream artifacts consumed, constraints applied, or explicit not-used rationale" + optional: false + - id: appmod.profile.sync file: commands/appmod.profile.sync.md description: "Update progress_sync: increment completed_tasks, check phase completion, set timestamps, commit if phase done" diff --git a/plugins/github-copilot-modernization/skills/architecture-diagram/SKILL.md b/plugins/github-copilot-modernization/skills/architecture-diagram/SKILL.md index dffa82d..bec869a 100644 --- a/plugins/github-copilot-modernization/skills/architecture-diagram/SKILL.md +++ b/plugins/github-copilot-modernization/skills/architecture-diagram/SKILL.md @@ -11,11 +11,49 @@ This skill generates a two-layer architecture visualization: a high-level applic - `workspace-path` (optional): Path to the project to analyze (defaults to current directory) +## ⚠ Mermaid Safety Constraints — read BEFORE you write any ```mermaid block + +Mermaid is unforgiving: one illegal character anywhere in a block crashes the **whole** diagram with `Syntax error in text`, not just the offending line. There is no partial rendering. Stay strictly inside this subset: + +1. **Chart kind.** Only `flowchart TD` (Step 1) or `flowchart LR` (Step 2). Never `graph TD`, never mixed. +2. **Subgraph form.** Always `subgraph ["display label"]`. The id must match `[A-Za-z][A-Za-z0-9_]*` (no spaces, no punctuation). NEVER use the anonymous form `subgraph "label"` — it crashes whenever the label contains `(`, `)`, `:`, `/`, `-`, etc., and the parser error appears on an unrelated line. +3. **Node form.** Only one of: `Id["label"]` (rectangle), `Id(("label"))` (circle), `Id[("label")]` (cylinder for data stores). Pick one shape per node — do not stack brackets. +4. **Arrow form.** Solid `-->`, dotted `-.->`. If you put a label on an arrow it MUST be double-quoted: `-->|"label"|`. Never bare `-->|label|`. +5. **No line breaks in labels.** The escape `\n` was removed in modern Mermaid and is the #1 cause of failures. Use `
` in flowcharts when you really must break a line. Strongly prefer single-line ≤ 60-char labels — put detail in the Inventory / Stack tables instead. +6. **Banned characters inside any label or subgraph title.** Use the ASCII replacement: + + | Banned | Why it breaks | Replacement | + |---|---|---| + | `\n` (literal two chars) | escape removed | `
` or drop | + | `—` (em-dash, U+2014) | parser treats as edge | `-` (ASCII hyphen) | + | `–` (en-dash, U+2013) | parser treats as edge | `-` | + | `{` `}` (e.g. `{id}`) | opens an entity block | drop braces — write `id` or `:id` | + | `"` inside a label | closes the label early | `'` (single quote) | + | `\|` inside a label | breaks edge-label parser | rephrase | + | `@` `#` `$` `%` `&` | unsafe in many positions | rephrase or drop | + | `(` `)` outside a `["..."]` quoted label | unbalanced parens crash | only inside the quoted label | + | smart quotes `"` `"` `'` `'` | not ASCII | regular `"` and `'` | + +7. **Unique node IDs across the whole file.** If Layer 1 has `DB`, Layer 2 cannot also have `DB`. Use `DB1` and `DB2` (or `AppDb`, `ComponentDb`). +8. **`subgraph` must be closed by a matching `end` on its own line.** No `end`, no diagram. + +### Mandatory self-attestation + +Immediately before writing each ` ```mermaid ` opening fence, emit this exact one-line HTML comment in the markdown (the comment will not render — it is for your own visible attestation that you have re-checked the block): + +``` +|"label"|, all subgraphs closed by end, ids unique --> +``` + +If you cannot truthfully emit that comment, fix the diagram first. + +--- + ## Execution Steps ### Step 1: Generate Application Architecture Section -Analyze the project and produce the complete `## Application Architecture` section in one pass: +Analyze the project and produce the complete `## Application Architecture` section in one pass. **Analysis:** - Examine build files (Java: pom.xml, build.gradle; .NET: *.csproj, *.sln; JS/TS: package.json, tsconfig.json) @@ -23,7 +61,7 @@ Analyze the project and produce the complete `## Application Architecture` secti - Scan key source files to extract: framework, major dependencies, data access patterns, external integrations, technology stack - Identify application layers (UI, Business Logic, Data Access), data storage technologies, and external service dependencies -**Diagram — Mermaid `flowchart TD`:** +**Diagram — Mermaid `flowchart TD`** (re-read the Safety Constraints above before writing): - Application layers with technology info (use `subgraph` for grouping) - Data storage components (specific names like "PostgreSQL", "Redis") - External service integrations @@ -31,8 +69,9 @@ Analyze the project and produce the complete `## Application Architecture` secti **Do NOT include**: individual classes/methods or migration directions. -Example: +Reference example (this block satisfies every Safety Constraint — match its shape): +|"label"|, all subgraphs closed by end, ids unique --> ~~~mermaid flowchart TD subgraph Client["Client Layer"] @@ -63,13 +102,15 @@ flowchart TD ~~~ **Textual explanations (write immediately after the diagram):** -- **Technology Stack Summary table**: Layer | Technology | Version | Purpose (e.g., Presentation | ASP.NET MVC 5 | 5.2.7 | Server-side web framework) +- **Technology Stack Summary table**: Layer | Technology | Version | Purpose - **Data Storage & External Services**: A short paragraph describing what databases, caches, message brokers, or external APIs are used and how they fit into the architecture -- **Key Architectural Decisions**: 1-3 bullet points on notable patterns (e.g., "Uses repository pattern with EF6 for data access", "Autofac provides DI with module-based registration") +- **Key Architectural Decisions**: 1-3 bullet points on notable patterns (e.g., "Uses repository pattern with EF6", "Autofac DI with module-based registration") + +> ⚠ Move detail OUT of node labels and INTO this table. A diagram with short labels and a rich table renders; a diagram with long labels does not. ### Step 2: Generate Component Relationships Section -Analyze component interactions and produce the complete `## Component Relationships` section in one pass: +Analyze component interactions and produce the complete `## Component Relationships` section in one pass. **Analysis:** - Identify key component types by framework conventions: @@ -84,31 +125,33 @@ Analyze component interactions and produce the complete `## Component Relationsh - Map data access patterns (service-to-repository, DbContext usage) - Detect cross-cutting concerns (middleware, interceptors, filters) -**Diagram — Mermaid `flowchart LR`:** +**Diagram — Mermaid `flowchart LR`** (re-read the Safety Constraints above before writing): - Components grouped by architectural layer using `subgraph` (Presentation, Business Logic, Data Access, Infrastructure) - Interaction arrows with brief labels - Cross-cutting concerns +- **Use IDs that do NOT collide with Step 1's diagram** (e.g., prefix with `c` for Component: `cWeb`, `cService`). **Do NOT include**: method signatures, private helpers, or external dependencies (covered by dependency-map skill). -Example: +Reference example (satisfies every Safety Constraint): +|"label"|, all subgraphs closed by end, ids unique --> ~~~mermaid flowchart LR - subgraph Presentation + subgraph PresentationLayer["Presentation"] UserCtrl["UserController"] OrderCtrl["OrderController"] end - subgraph Business["Business Logic"] + subgraph BusinessLayer["Business Logic"] UserSvc["UserService"] OrderSvc["OrderService"] NotifSvc["NotificationService"] end - subgraph DataAccess["Data Access"] + subgraph DataAccessLayer["Data Access"] UserRepo["UserRepository"] OrderRepo["OrderRepository"] end - subgraph Infra["Infrastructure"] + subgraph InfraLayer["Infrastructure"] AuthFilter["AuthenticationFilter"] LogMiddleware["LoggingMiddleware"] end @@ -121,11 +164,11 @@ flowchart LR OrderSvc -->|"queries"| OrderRepo AuthFilter -.->|"intercepts"| UserCtrl AuthFilter -.->|"intercepts"| OrderCtrl - LogMiddleware -.->|"wraps"| Presentation + LogMiddleware -.->|"wraps"| PresentationLayer ~~~ **Textual explanation (write immediately after the diagram):** -- **Component Inventory table**: Component | Layer | Type | Responsibility (e.g., CatalogController | Presentation | MVC Controller | Handles catalog browsing and CRUD) +- **Component Inventory table**: Component | Layer | Type | Responsibility ### Step 3: Save Output @@ -163,39 +206,27 @@ A brief introduction (1-2 sentences). ## Scaling Rules -- If the project has **more than 30 components**, aggregate by package/namespace (e.g., show `com.example.orders` as one node instead of listing every class) -- Keep each diagram under **40 nodes** to ensure readability and GitHub rendering compatibility -- For multi-module projects, focus on inter-module boundaries in Layer 1 and key components within the most important modules in Layer 2 - -## Mermaid Syntax Rules - -The diagram must parse cleanly under **Mermaid >= 9.x** (the version used by GitHub, VS Code, Obsidian, and every modern renderer). Anything outside the legal subset crashes the entire diagram with `Syntax error in text`, not just the offending line. - -- Use `flowchart TD` for Layer 1 and `flowchart LR` for Layer 2 -- Avoid special characters (`@`, `#`, `$`, `%`, `&`) in node labels — use plain text -- Always quote arrow labels with double quotes: `-->|"label"|` -- Use `subgraph` for grouping, with a display name in quotes if it contains spaces -- Verify all node IDs are unique across the entire diagram - -### Line breaks in node labels — HARD RULE +- If the project has **more than 30 components**, aggregate by package/namespace (e.g., show `com.example.orders` as one node instead of listing every class) — this also keeps labels short and safe. +- Keep each diagram under **40 nodes** to ensure readability and GitHub rendering compatibility. +- For multi-module projects, focus on inter-module boundaries in Layer 1 and key components within the most important modules in Layer 2. -- **NEVER use `\n` for line breaks inside node labels.** The literal `\n` escape was removed in modern Mermaid and is the #1 cause of "Syntax error in text" — every node containing `\n` will fail to render. -- **Use `
` instead** for an explicit line break: `Node["First line
Second line"]`. -- If a label is long, prefer a single concise phrase over multi-line. Move details into the Component Inventory / Technology Stack tables that follow the diagram. -- ❌ `MediaLib["Media Library\n(MediaScannerService\nMediaFileService)"]` -- ✅ `MediaLib["Media Library
MediaScannerService
MediaFileService"]` -- ✅ `MediaLib["Media Library"]` (and list the sub-components in the inventory table) +## Common failure patterns observed in past runs -### Self-check before emitting each ```mermaid block +Each row below is something the model actually produced and crashed the diagram. Use the ✅ form. -1. Search the block for the two characters `\n` — if found, replace each with `
` (or remove). Zero `\n` must remain. -2. Confirm every node ID is unique and every `subgraph` is closed by `end`. -3. Confirm every arrow label is double-quoted. +| ❌ Past mistake | ✅ Safe form | Why the ❌ crashed | +|---|---|---| +| `subgraph "Spring Boot Application (port 8080)"` | `subgraph SpringApp["Spring Boot Application (port 8080)"]` | Anonymous subgraph + parentheses in title | +| `HC["HomeController\n GET / — gallery page"]` | `HC["HomeController GET /"]` (move detail to table) | Literal `\n` + em-dash | +| `PHOTOS_TABLE["PHOTOS table\n id (UUID PK)\n photo_data (BLOB)"]` | `PHOTOS_TABLE["PHOTOS"]` (columns belong in a table) | `\n` and overlong label | +| `PFC["PhotoFileController\n GET /photo/{id}"]` | `PFC["PhotoFileController GET /photo/:id"]` | `\n` + `{id}` | +| `Spring["Spring Boot\n2.7.18"]` | `Spring["Spring Boot 2.7.18"]` | `\n` | +| `A -->|fetches users| B` | `A -->|"fetches users"| B` | Bare (unquoted) arrow label | ## Error Handling - **Unsupported project type**: Output a single line: `> ERROR: Unsupported project type. This skill supports Java, .NET, JavaScript, and TypeScript projects only.` -- **No source code found**: Output: `> ERROR: No recognized source files found at {workspace-path}. Verify the path is correct.` +- **No source code found**: Output: `> ERROR: No recognized source files found at workspace-path. Verify the path is correct.` - **Insufficient info**: Generate a best-effort diagram from available data. Add a note inside the diagram: `Note["Some components could not be identified"]` ## Success Criteria @@ -204,4 +235,5 @@ The diagram must parse cleanly under **Mermaid >= 9.x** (the version used by Git - Layer 1 is accompanied by Technology Stack Summary table, Data Storage & External Services paragraph, and Key Architectural Decisions - Layer 2 Mermaid diagram renders correctly showing component interactions grouped by architectural layer - Layer 2 is accompanied by Component Inventory table +- Every ```mermaid block is preceded by the `` attestation comment - File saved to `.github/modernize/assessment/engines/facts/architecture-diagram.md` diff --git a/plugins/github-copilot-modernization/skills/business-workflows/SKILL.md b/plugins/github-copilot-modernization/skills/business-workflows/SKILL.md index 9a4b01e..2458426 100644 --- a/plugins/github-copilot-modernization/skills/business-workflows/SKILL.md +++ b/plugins/github-copilot-modernization/skills/business-workflows/SKILL.md @@ -11,6 +11,44 @@ Analyze the project to document business processes end-to-end, domain entities, - `workspace-path` (optional): Path to the project to analyze (defaults to current directory) +## ⚠ Mermaid Safety Constraints — read BEFORE you write the ```mermaid block + +Mermaid sequenceDiagram is unforgiving in a few specific ways: one bad alias or one missing `end` crashes the **whole** diagram with `Syntax error in text`, not just the offending line. Stay strictly inside this subset for the sequence diagram: + +1. **Chart kind.** `sequenceDiagram` only. Never `sequence-diagram`, never `sequence`. +2. **Participants.** Always declare with the alias form `participant as "Display Label"`. The id must match `[A-Za-z][A-Za-z0-9_]*`. Never omit the id — even a one-word participant should be `participant Owner as "Owner"`. This is the single biggest cause of past failures. +3. **Arrows.** + - `->>` synchronous request + - `-->>` synchronous response + - `-)` async fire-and-forget + - Message text goes after `:` and is plain text — keep it short and on one line. +4. **Blocks.** `alt` / `else` / `opt` / `loop` / `par` / `critical` MUST be closed by `end` on its own line. Every open block must have a matching `end`. Missing `end` is the #2 cause of past failures. +5. **No line breaks anywhere.** The escape `\n` was removed in modern Mermaid. Aliases, message text, and `Note over` content must all be single-line. Split a long note into multiple consecutive `Note over` lines; split a long message into multiple arrows. This is the #1 cause of past failures. +6. **Banned characters inside participant aliases specifically** (message text is more permissive — only `\n` is banned there): + + | Banned in alias | Why it breaks | Replacement | + |---|---|---| + | `\n` (literal two chars) | escape removed | drop | + | `"` (a second double-quote) | closes the alias early | `'` (single quote) | + | `` ` `` (backtick) | breaks alias quoting | drop | + | smart quotes `"` `"` `'` `'` | not ASCII | regular `"` and `'` | + | `:` | confuses with message delimiter | rephrase, e.g. `"Order Service (v2)"` not `"Order Service: v2"` | + | `
` | not interpreted inside aliases | rephrase as shorter alias | + +7. **Quote the alias.** `participant Svc as "Order Service"` — never `participant Svc as Order Service` (unquoted multi-word aliases break). + +### Mandatory self-attestation + +Immediately before writing the ` ```mermaid ` opening fence, emit this exact one-line HTML comment in the markdown (it does not render — it is for your own visible attestation): + +``` + +``` + +If you cannot truthfully emit that comment, fix the diagram first. + +--- + ## Scope Boundaries — Avoid Redundancy with Other Skills This skill is part of a set of four complementary assessment skills. To avoid content duplication across their output documents, observe these scope rules: @@ -78,11 +116,12 @@ Create a **Mermaid `sequenceDiagram`** showing the primary business workflow end - Use `alt`/`else` blocks to show circuit breaker fallback paths that affect business outcomes - Show cross-service data aggregation flows -Example: +Reference example (this block satisfies every Safety Constraint — match its shape): + ~~~mermaid sequenceDiagram - participant Owner + participant Owner as "Owner" participant Gateway as "API Gateway" participant CustSvc as "Customer Service" participant VisitSvc as "Visit Service" @@ -172,35 +211,23 @@ A brief introduction (1-2 sentences) summarizing the application's business doma - For multi-module projects, focus on the primary end-to-end business workflow that spans modules - Aggregate minor CRUD operations and show only workflows that involve business logic beyond simple create/read/update/delete -## Mermaid Syntax Rules - -The diagram must parse cleanly under **Mermaid >= 9.x**. Anything outside the legal subset crashes the entire diagram with `Syntax error in text`. - -- Use `sequenceDiagram` -- Avoid special characters (`@`, `#`, `$`, `%`, `&`) in participant labels — use plain text or quoted labels -- Use `->>` for synchronous calls and `-->>` for responses -- Use `participant` with alias syntax for readable labels: `participant Svc as "OrderService"` -- Use `Note over` for annotations about business decisions or fallback behavior -- Use `alt`/`else`/`end` blocks for decision points and circuit breaker fallbacks -- Do not use backticks inside participant labels - -### Line breaks — HARD RULE - -- **NEVER use `\n` for line breaks inside participant aliases, messages, or notes.** The literal `\n` escape was removed in modern Mermaid and triggers "Syntax error in text". -- Keep aliases on a single line: `participant Tx as "Transcoding Service"` — not `"Transcoding\nService"`. -- For multi-fact notes, emit multiple `Note over` statements instead of `\n`-separated text. -- ❌ `Note over Client,API: First fact\nSecond fact` -- ✅ Two consecutive `Note over Client,API: ...` lines. +## Common failure patterns observed in past runs -### Self-check before emitting each ```mermaid block +Each row below is something the model actually produced that crashed the diagram. Use the ✅ form. -1. Search the block for the two characters `\n` — remove or split. Zero `\n` must remain. -2. Confirm every `alt`/`opt`/`loop`/`par` block is closed by `end`. +| ❌ Past mistake | ✅ Safe form | Why the ❌ crashed | +|---|---|---| +| `participant Owner` (no alias) | `participant Owner as "Owner"` | Bare participants can break when used later with spaces | +| `participant Tx as "Transcoding\nService"` | `participant Tx as "Transcoding Service"` | Literal `\n` in alias | +| `participant API as "REST API: v2"` | `participant API as "REST API (v2)"` | `:` in alias collides with message delimiter | +| `Note over Client,API: First fact\nSecond fact` | Two consecutive `Note over Client,API: ...` lines | `\n` in note text | +| `alt happy path` ... missing `end` | `alt happy path` ... `end` | Unclosed block | +| `participant Svc as Order Service` (unquoted) | `participant Svc as "Order Service"` | Multi-word alias must be quoted | ## Error Handling - **Unsupported project type**: Output a single line: `> ERROR: Unsupported project type. This skill supports Java, .NET, JavaScript, and TypeScript projects only.` -- **No business logic found**: Output: `> ERROR: No recognized business logic or workflows found at {workspace-path}. The project may be a library or framework without business processes.` +- **No business logic found**: Output: `> ERROR: No recognized business logic or workflows found at workspace-path. The project may be a library or framework without business processes.` - **Insufficient info**: Generate a best-effort document from available data. Add a note: `> Note: Some workflows or business rules could not be fully traced.` ## Success Criteria @@ -211,4 +238,5 @@ The diagram must parse cleanly under **Mermaid >= 9.x**. Anything outside the le - Cross-service data flows describe aggregation/composition patterns with fallback behavior - Mermaid sequence diagram renders correctly showing end-to-end business workflow with `alt`/`else` blocks for fallbacks - Business rules section summarizes validation, decision logic, state transitions, and constraints +- The ```mermaid block is preceded by the `` attestation comment - File saved to `.github/modernize/assessment/engines/facts/business-workflows.md` diff --git a/plugins/github-copilot-modernization/skills/cve-remediation/SKILL.md b/plugins/github-copilot-modernization/skills/cve-remediation/SKILL.md new file mode 100644 index 0000000..c321498 --- /dev/null +++ b/plugins/github-copilot-modernization/skills/cve-remediation/SKILL.md @@ -0,0 +1,367 @@ +--- +name: cve-remediation +description: | + Scan dependency manifests against known CVEs and remediate by upgrading vulnerable + dependencies to patched versions, then rebuild and re-scan to confirm. Self-contained + scan→fix→verify loop for any project with a dependency manifest. + + Use when: a cve-remediation task is dispatched; dependency set changed (version bump, + new framework); assessment flagged vulnerable or EOL dependencies; or user asked to + "fix CVEs", "patch vulnerabilities", or "dependency security". + + Triggers: "cve", "remediate cve", "fix cves", "patch vulnerable dependencies", + "vulnerability scanning", "dependency security", "vulnerable dependencies", + "security advisories", "npm audit", "pnpm audit", "maven audit", "gradle audit", + "dependency scan", "vulnerability remediation". + + NOT for: security audit of auth/input/secrets/OWASP code paths (use security-review). +--- + +# CVE Remediation + +This skill is the implementer-owned scan→fix→verify loop for dependency CVEs. It is +dispatched as an **execute-phase** task to an implementer role (backend), runs +**per group**, and produces patched dependency manifests plus an audit trail. + +## Ownership & boundaries + +- **This is implementer work, not audit work.** The `security` role audits and escalates + but does NOT fix. This skill performs the *fix* (editing dependency manifests), so it is + owned by the implementer who owns those manifests — not by the `security` role. +- **The rebuild/re-scan is a self-check, not a quality gate.** It is the implementer + confirming their own change took — analogous to compiling after editing code. It must + NOT masquerade as the project's security gate, and it stays within the **Implementation** + phase label. The independent gates (`smoke-test`, `runtime-validation`, and the + coordinator's verdict rules) remain separate and unchanged. +- **Scope is per-group.** Each in-scope group has its own dependency manifests; remediate + the manifests belonging to the dispatched group. + +## Scanning: tool-first with LLM fallback + +CVE scanning is performed by the shared **`appmod-cve-assessment`** tool, which all +modernization surfaces (VS Code extension, IntelliJ plugin, Copilot CLI plugin, MCP server) +expose. It queries the GitHub Security Advisories API for a set of package ecosystems and +writes structured findings to a result file. Using the shared tool keeps scanning +consistent across products and emits the standard telemetry. + +- **Primary — `appmod-cve-assessment` tool** (preferred whenever it applies). Consult the + tool's own `ecosystem` parameter for the ecosystems it currently accepts — treat that + schema as the source of truth rather than assuming a fixed list, since it may grow over + time. +- **Fallback — LLM-only scan** when the tool path does **not** apply. This covers **two** + cases, both signalled by the tool itself: + 1. **Tool unavailable** — `appmod-cve-assessment` is not registered in the current + runtime (e.g. a standalone Copilot CLI / rearchitecture runtime without the tool wired + up). + 2. **Ecosystem not accepted** — the tool rejects the project's ecosystem (its + `ecosystem` parameter does not accept it / input validation fails). Whichever + ecosystems the tool does not yet cover fall here automatically. + + In either case the model identifies known CVEs for the listed dependency versions from + its own knowledge and writes the **same findings schema** (see Step 3). This is + best-effort — model knowledge has a training cutoff and may miss recent advisories — so + prefer the tool whenever it applies. + +Map the project's package manager to the ecosystem identifier the tool accepts (for example, +Maven and Gradle both resolve to the same JVM identifier). If the tool does not accept any +identifier for the project's ecosystem, go straight to the LLM-only fallback. + +## Artifact path + +The coordinator provides an artifact path for this task. Throughout this skill, +`{{ARTIFACT_PATH}}` refers to that directory. Write all reports and the fix summary there. + +## Workflow + +### Step 1: Precheck — detect project type and build tool + +Before running any build or dependency commands, verify that the required build tool is +available. + +1. **Detect the project type** by examining the group's project root: + + | File(s) found | Project type | Tool ecosystem id (common mapping) | + |---|---|---| + | `pom.xml` | Maven (Java) | maven | + | `build.gradle` or `build.gradle.kts` | Gradle (Java) | maven | + | `*.sln` | .NET solution | nuget | + | `*.csproj` | C# project | nuget | + | `packages.config` | Legacy .NET | nuget | + | `package.json` | Node.js (npm/pnpm/yarn) | npm | + | any other manifest (e.g. `requirements.txt`, `pyproject.toml`, `go.mod`, `Cargo.toml`, `Gemfile`, `composer.json`) | other | whatever the tool accepts, else LLM-only fallback (Step 2c) | + + > The ecosystem ids above are the **common** mappings at the time of writing. The tool's + > `ecosystem` parameter is authoritative — if it accepts an identifier for the project's + > ecosystem, use the tool; otherwise use the LLM-only fallback. + +2. **Resolve the build command** — prefer project-local wrappers over global tools: + + | Project type | Check order (prefer first match) | Fallback | + |---|---|---| + | Maven | `./mvnw` (Unix) or `mvnw.cmd` (Windows) | `mvn` on PATH | + | Gradle | `./gradlew` (Unix) or `gradlew.bat` (Windows) | `gradle` on PATH | + | .NET | `dotnet --version` | — | + | Node.js | `npm` / `pnpm` / `yarn` (only needed to *apply* fixes) | — | + +3. **If the required tool is not found**, stop and report the error. Do not proceed. + +### Step 2: Scan dependencies against CVE databases + +The scan also serves as **detection**: a clean result (an empty findings array) means the +group has no known dependency CVEs and the task exits cheaply. + +#### Step 2a: Collect dependency coordinates and locations + +Extract the dependency coordinates for the group, capturing for each one the +workspace-relative file path and 1-based line number where it is declared. Prefer +**resolved** versions (which include transitive dependencies — the common source of CVEs) +and fall back to manifest-declared versions when a resolver is unavailable. + +| Ecosystem | Coordinate format | Where to read | +|---|---|---| +| `maven` | `groupId:artifactId:version` | `mvn dependency:list` / `gradle dependencies` (resolved), else `pom.xml` / `build.gradle` / `gradle.properties` | +| `nuget` | `PackageName@version` | `dotnet list package` (resolved), else `*.csproj` / `Directory.Packages.props` / `packages.config` | +| `npm` | `package-name@version` (e.g. `express@4.18.2`, `@angular/core@16.0.0`) | lockfile (`package-lock.json`, `pnpm-lock.yaml`, `yarn.lock` — resolved, includes transitives), else `package.json` | + +> These are the common ecosystems and their coordinate formats. For the authoritative +> coordinate format expected by each ecosystem the tool accepts, consult the tool's +> `dependencies` / `ecosystem` parameter descriptions. For an ecosystem the tool does not +> accept (LLM-only fallback), use that ecosystem's natural coordinate form (e.g. +> `package==version` for Python) read from its manifest or lockfile. + +#### Step 2b: Scan with the `appmod-cve-assessment` tool (primary) + +Call `appmod-cve-assessment` with: + +- `cveResultFilePath` — an **absolute** path of `{{ARTIFACT_PATH}}/cve-report-N.json` + (start at `1`, increment each scan to preserve history). +- `ecosystem` — the identifier the tool accepts for the project's package ecosystem (see + the tool's `ecosystem` parameter for the accepted values). +- `dependencies` — the coordinate array from Step 2a. +- `dependencyLocations` — `{ coordinate, filePath, lineNumber }` for each dependency. + +The tool fetches CVEs, writes the findings JSON to `cveResultFilePath`, and returns a +summary message. For very large dependency sets, scan in batches, writing each batch to its +own numbered report. **If the tool is unavailable, or it rejects the `ecosystem` as not +accepted, switch to the LLM-only fallback (Step 2c).** + +#### Step 2c: LLM-only scan (fallback — tool unavailable OR ecosystem not accepted) + +Use this path when the tool cannot do the scan — either because `appmod-cve-assessment` is +**not available** in the current runtime, or because the tool **does not accept** the +project's ecosystem (its `ecosystem` parameter rejects it). Perform the scan from model +knowledge instead: for each coordinate + version, identify known CVEs and write the **same +findings schema** (see Step 3) to `{{ARTIFACT_PATH}}/cve-report-N.json`. Note in the fix +summary that the LLM fallback was used (and why), since it is best-effort and may miss +recent advisories. + +**Large projects**: if scanning is slow, run it in a **background (async) terminal** and, +while it runs, review and fix vulnerabilities already known from earlier runs. Address any +newly discovered issues once it completes. + +### Step 3: Review the report and identify vulnerable dependencies + +Parse the latest `cve-report-N.json` and examine each finding: + +1. Read the report file and parse the JSON (an array of findings; `[]` means no CVEs). +2. Group by `severity`: **critical** > **high** > **medium** > **low**. +3. For each finding note: the affected dependency + current version and the upgrade target + — both are in the `evidence.explanation` (the `Affected dependencies` and + `Recommended fix` lines) — plus the CVE identifier (`id`) and `name`. +4. Prioritize critical and high severity for immediate remediation. + +Report schema (one object per CVE): +```json +[ + { + "id": "CVE-2022-22965", + "name": "Spring Framework RCE via Data Binding on JDK 9+", + "status": "FOUND", + "category": "CVE", + "severity": "critical", + "storyPoint": 1, + "evidence": { + "files": ["pom.xml:20"], + "explanation": "[CVE-2022-22965](https://github.com/advisories/GHSA-36p3-wjmg-h94x): Spring Framework RCE via Data Binding on JDK 9+\n\nSeverity: CRITICAL\n\nAffected dependencies:\n - org.springframework:spring-core@5.3.9\n\nRecommended fix:\n - Upgrade org.springframework:spring-core to 5.3.18 or later" + } + } +] +``` + +> A finding whose `explanation` contains **no** `Recommended fix` line has no upstream patch +> available (unfixable). An empty array (`[]`) means no known CVEs were found. + +### Step 4: Update vulnerable dependencies to secure versions + +For each vulnerable dependency, update to a secure version (the upgrade target from the +`Recommended fix` line) using the appropriate method. + +**Fix principles** (consistent with the security agent): +- **Direct upgrade, not a framework upgrade.** Bump the affected dependency **directly** to + the patched version — no stepping through intermediate versions. CVE remediation is a + targeted patch, not a version migration. +- **Respect a pinned target version.** If the user's request pins a target version/line for + a dependency (e.g. "upgrade Spring Framework to 6.2.18"), remediate **within** it — a + patch bump within that line to clear a CVE is fine. Only when no in-range patch exists do + you leave it — record it as a follow-up (Step 7.3) rather than forcing a major jump. +- **Minimal changes.** Change only what is needed to clear the CVE. Do not refactor, + reformat, or make unrelated edits. +- **Batch related fixes.** When a single upgrade clears several CVEs (e.g. a shared + BOM/parent), apply it once for all of them. + +#### Java — Maven (pom.xml) +1. Find the version (may be in ``, ``, or inline ``). +2. Update to the patched version (or the latest stable if the patched version is also outdated). +3. If the version is inherited from a parent POM (e.g. Spring Boot starter parent), update the parent version instead. + +> **Where CVEs hide — check BOM overrides first.** Pay special attention to dependencies +> that explicitly declare a `` tag *overriding* a managed BOM (e.g. the Spring Boot +> dependencies BOM). These inline overrides bypass BOM management and are the most common +> source of missed CVE vulnerabilities — bumping the BOM/parent version alone will not patch +> them. Cross-check the `` tags in each sub-module's `pom.xml` against the +> vulnerable dependencies and update the override (or remove it to fall back to the managed +> version) as needed. + +#### Java — Gradle (build.gradle) +1. Find the version in `build.gradle` or `gradle.properties`. +2. Update the version string to the patched version. +3. If using a BOM or platform dependency, update the BOM version. + +#### .NET (csproj) +1. Find ``. +2. Update the `Version` attribute to the patched version. +3. If versions are managed centrally via `Directory.Packages.props`, update them there instead. +4. Alternatively: `dotnet add package PackageName --version X.Y.Z`. + +#### Node.js — npm / pnpm / yarn (package.json) +1. Find the version in `package.json` under `dependencies` or `devDependencies`. +2. Update the range to the patched version (e.g. `"^4.17.21"`), then **regenerate the + lockfile** so the resolved transitive versions update too: + - npm: `npm install pkg@X.Y.Z` (or edit `package.json` then `npm install`) + - pnpm: `pnpm add pkg@X.Y.Z` (or edit then `pnpm install`) + - yarn: `yarn add pkg@X.Y.Z` (or edit then `yarn install`) +3. **Transitive dependencies** (the common case for npm CVEs) are pulled in by other + packages and have no direct entry in `package.json`. Force a patched version with an + override instead of a direct edit: + - npm: add an `"overrides"` block (`{ "overrides": { "pkg": "X.Y.Z" } }`) + - pnpm: add `"pnpm": { "overrides": { "pkg": "X.Y.Z" } }` + - yarn (Berry): add a `"resolutions"` block + Then re-run the install command to regenerate the lockfile. + +### Step 5: Re-scan to confirm issues are resolved (self-check) + +Re-run the Step 2 scan and write the output with the next sequential number. Then compare +the new report against the previous one and **exit the loop** when ANY of these hold: + +- **Clean** — the report is an empty array (`[]`). Success. +- **Only unfixable CVEs remain** — every remaining finding has no `Recommended fix` line in + its `explanation` (no upstream patch available). Success; record these in the summary as + accepted/unfixable. +- **No progress** — after a fix attempt the **same** fixable CVEs persist — compare by CVE + **id**, not by raw count. A fix that resolves the targeted CVE but surfaces a *different, + newly-disclosed* CVE is **progress, not a stall**: bump to the highest recommended + **released** version and keep going. Treat it as no-progress (and stop) only when the same + CVE id keeps reappearing despite a fix, **or** when the only way forward is a recommended + version that is **not yet released** (no installable artifact) — record those as + accepted/stuck in the summary. Stop here rather than looping forever. + +> **Fixable vs. unfixable — important.** A CVE is *unfixable* **only** when it has no +> `Recommended fix` line (no patched version exists upstream). A CVE that requires a +> **major-version upgrade** still has a patched version, so it is **fixable** — leaving it +> means remediation is **incomplete** (the *No progress* / deferred path, surfaced as a +> Step 7 follow-up), **not** the *Only unfixable* success. Treat the run as fully successful +> only when **no fixable CVEs remain**. This matches the security agent's rule: do not claim +> success while patchable CVEs are still outstanding. + +Otherwise — if the count dropped but fixable CVEs (those with a `Recommended fix`) still +remain — return to Step 4, fix the newly reported CVEs, and re-scan, incrementing the report +number each time. This loop is the implementer's own confirmation — not a separate gate. + +### Step 6: Build and test after updates + +Use the build command resolved in Step 1. + +```shell +./mvnw clean verify # Maven +./gradlew clean build # Gradle +dotnet build && dotnet test # .NET +npm install && npm test # Node.js (npm) — use pnpm/yarn equivalents as appropriate +``` + +Verify the build completes, existing tests pass, and the app starts (if applicable). If the +build fails due to breaking API changes from an upgrade, apply the necessary code fixes and +re-run the build. **Cap this at 3 fix attempts** — if the build still fails, stop, keep the +dependency changes that scanned clean, and document the build issue in the summary (Step 7) +rather than looping. Keep this within the Implementation phase — it is a build recheck, not +a new phase. + +### Step 7: Document the changes + +1. Write a summary of CVEs fixed (CVE/GHSA ID, dependency + version, patched version, + severity, brief description) to `{{ARTIFACT_PATH}}/cve-fix-summary.md`. Note whether the + scan used the `appmod-cve-assessment` tool or the LLM-only fallback. +2. Run a final scan and write it as the final report + (`{{ARTIFACT_PATH}}/final-cve-report.json`). All intermediate numbered reports are + preserved for audit history. +3. If any fix requires a **major** version upgrade (breaking-change risk), record it as a + follow-up item in the summary rather than forcing it silently — and surface it to the + coordinator so dependent tasks and reviews are aware. + +--- + +## Environment Setup + +### Prerequisites + +1. **Build tool** (one of): `mvn` / `mvnw`, `gradle` / `gradlew`, or `dotnet` — to apply + fixes and rebuild. +2. **Node.js package manager** (`npm` / `pnpm` / `yarn`) — only needed to apply Node.js fixes. +3. **GitHub token** (optional, recommended for the tool path — raises the GitHub Security + Advisories rate limit from 60 to 5000 req/hr): export `GITHUB_TOKEN` (or `GITHUB_PAT`). + The `appmod-cve-assessment` tool reads it from the environment when present. + +--- + +## Error Handling + +| Error | Cause | Solution | +|-------|-------|----------| +| `appmod-cve-assessment` not available | Tool not registered in the current runtime | Use the **LLM-only fallback** (Step 2c) | +| Tool rejects the `ecosystem` | The tool does not accept this project's ecosystem | Use the **LLM-only fallback** (Step 2c) | +| Rate limit / HTTP 403 from the tool | Too many advisory API calls without auth | Set `GITHUB_TOKEN` / `GITHUB_PAT` in the environment | +| Empty / malformed coordinate rejected | Wrong coordinate format | maven: `groupId:artifactId:version`; nuget: `PackageName@version`; npm: `package-name@version` | +| `Maven/Gradle not found` | Build tool not in PATH | Install or ensure the wrapper (`mvnw`/`gradlew`) exists | +| `dotnet CLI not found` | .NET SDK not installed | Install .NET SDK | +| `No supported project files found` | Unrecognized project type | Ensure the root has pom.xml, build.gradle, *.sln, *.csproj, packages.config, or package.json | + +--- + +## Troubleshooting + +### List dependencies manually (to build the coordinate list for Step 2a) + +```bash +# Maven +mvn dependency:list -DoutputFile=deps.txt -q +grep -E "^ [a-zA-Z]" deps.txt | sed 's/^ //' | awk -F: '{print $1":"$2":"$4}' > coordinates.txt + +# Gradle +./gradlew dependencies --configuration compileClasspath > deps.txt + +# .NET +dotnet list package > deps.txt + +# Node.js (npm) — from package-lock.json (lockfileVersion 2/3) +jq -r '.packages | to_entries[] | select(.key|startswith("node_modules/")) | select(.value.version) | "\(.key|sub(".*node_modules/";""))@\(.value.version)"' package-lock.json | sort -u > coordinates.txt +``` + +Feed the resulting coordinates (with their file/line locations) into the +`appmod-cve-assessment` tool, or — when the tool is unavailable — into the LLM-only +fallback scan. + +### Rate limit errors + +Set a GitHub token in the environment so the tool authenticates its advisory API calls: +```bash +export GITHUB_TOKEN=$(gh auth token) +``` diff --git a/plugins/github-copilot-modernization/skills/dag-generation/SKILL.md b/plugins/github-copilot-modernization/skills/dag-generation/SKILL.md index 703782e..4f11dd6 100644 --- a/plugins/github-copilot-modernization/skills/dag-generation/SKILL.md +++ b/plugins/github-copilot-modernization/skills/dag-generation/SKILL.md @@ -33,6 +33,8 @@ Select fragments from the task catalog and produce a DAG. 1. **Project profile** — read from `{{BASE_PATH}}/artifacts/project-profile.yaml` (project.loc, project.languages, project.modules, assessment.change_type, assessment.grouping_needed) 2. **user_ask** — natural-language migration target (passed by coordinator) +If `user_ask` names an explicit target stack or version, preserve it verbatim in every selected task. Do not replace, downgrade, or reinterpret the requested version based on model familiarity or LTS defaults. + ### Decision Procedure #### Step 1: Determine deep_planning @@ -57,6 +59,8 @@ Read `references/task-catalog.md`. For each fragment, decide include/exclude bas - Project profile (LOC, modules) - `deep_planning` decision from Step 1 (drives `implementation-plan` selection) +Always include `target-env-prep` when the target runtime/framework/language/tooling differs from source or when the user specifies an explicit target version. This is an environment preparation task: it must install/provision/activate the requested target when possible, produce a preparation artifact, and run before scaffold/implementation/build/test tasks. It normally has no dependency on architecture/source analysis and should run in parallel with those tasks. This is true even for small projects and even when `deep_planning: false`. + Respect `when` / `skip-when` conditions and `after` ordering from the catalog. **⛔ Skip-when enforcement (mandatory post-selection gate):** @@ -65,6 +69,9 @@ After initial selection, sweep every selected fragment and check its `skip-when` This gate catches cases where the initial selection included fragments that looked relevant but conflict with the deep_planning decision or project scale. +**✅ Explicit-request override (runs AFTER skip-when enforcement — highest precedence):** +If `user_ask` explicitly requests a completeness, consistency, or feature-parity check (e.g. "run a completeness check", "verify nothing was missed", "enforce consistency", "feature parity sign-off"), force-include the completeness/conformance validation fragment (`conformance-review`, and `feature-parity-signoff` when applicable) **even if its `skip-when` condition matched and removed it above**. User intent overrides the size/type heuristic. This override is **one-directional** — it can only ADD a gate the heuristics dropped, never remove one they selected. It must run after the skip-when sweep, otherwise the sweep would strip the fragment back out (e.g. `skip-when: same-stack upgrade`). + Fragment selection is an internal decision — do NOT output the selection rationale to the user. The DAG itself is the user-facing result. #### Step 3: Generate DAG diff --git a/plugins/github-copilot-modernization/skills/dag-generation/references/dag-rules.md b/plugins/github-copilot-modernization/skills/dag-generation/references/dag-rules.md index b3615c2..c659894 100644 --- a/plugins/github-copilot-modernization/skills/dag-generation/references/dag-rules.md +++ b/plugins/github-copilot-modernization/skills/dag-generation/references/dag-rules.md @@ -37,8 +37,9 @@ Each task-catalog fragment has a `scope` field (`per-group` or `global`). 2. If no implementation plan, derive execute tasks from architecture analysis + pipeline fragments. 3. **Minimum dependency principle**: before adding edge D→T, verify T needs an artifact D produces. No deps based on phase grouping or role association. Do NOT make a task depend on ALL tasks in a prior group when it only needs output from ONE of them. 4. **Correct dependencies**: if a task consumes another's output, it MUST depend on it. UI pages calling APIs MUST depend on the API tasks, not just the scaffold. A task reading database tables MUST depend on the migration task. -5. **Scaffold gate (rewrite only)**: when the change_type is rewrite and a scaffold task creates the new project structure, any task that writes source code files MUST depend on the scaffold task. Does not apply to upgrade or extract. -6. **Output-to-consumer mapping**: for each role, identify what it produces and who needs that output. Only create a task if its output is consumed by another role, or if it's the final deliverable. +5. **Target environment preparation gate**: when `target-env-prep` is selected, emit it as a standalone execute-phase task before scaffold/implementation/build/test tasks. It is a preparation task, not analysis. By default it has no dependency on analysis/design tasks: it needs the user-specified target stack and local environment only, so it should run in parallel with architecture/source analysis unless the task explicitly needs an upstream artifact. Every scaffold, implementation, build, test, runtime-validation, or other target-stack task MUST depend on it. If its artifact reports `BLOCKED`, no dependent implementation/build/test task is ready. +6. **Scaffold gate (rewrite only)**: when the change_type is rewrite and a scaffold task creates the new project structure, any task that writes source code files MUST depend on the scaffold task. Does not apply to upgrade or extract. +7. **Output-to-consumer mapping**: for each role, identify what it produces and who needs that output. Only create a task if its output is consumed by another role, or if it's the final deliverable. ## Parallelism diff --git a/plugins/github-copilot-modernization/skills/dag-generation/references/task-catalog.md b/plugins/github-copilot-modernization/skills/dag-generation/references/task-catalog.md index 8ac0b0a..34baec2 100644 --- a/plugins/github-copilot-modernization/skills/dag-generation/references/task-catalog.md +++ b/plugins/github-copilot-modernization/skills/dag-generation/references/task-catalog.md @@ -67,9 +67,17 @@ LLM uses this to select task fragments for DAG generation. Each fragment is `{de > **Implementation tasks are NOT selected from this catalog.** When `implementation-plan` is selected in the plan phase, the worker producing that plan decomposes the implementation into concrete tasks — the coordinator dispatches from that breakdown. When `implementation-plan` is skipped (small projects), the coordinator decomposes implementation tasks itself based on plan-phase outputs. The fragments below are **auxiliary** execute-phase tasks that may be selected alongside implementation tasks. +### target-env-prep +- **desc**: Prepare the target toolchain/environment before implementation, not merely check readiness. Install, provision, or activate the requested runtime/framework/language/build/test prerequisites when the current environment permits it (examples: JDK for Java/Spring Boot upgrades, Node.js/npm for Angular/React/WinForms-to-web migrations, .NET SDK for .NET target versions, Python/Go/Ruby toolchains, browser/E2E prerequisites when required). Distinguish **installed** toolchains from the **active** default toolchain and from the toolchain that planned build/test commands will actually use. Produce exact preparation actions taken, installed versions, active versions, command-resolution evidence, activation commands/env vars for downstream tasks, missing tools, and blockers if the requested target cannot be prepared in the current environment. +- **scope**: global +- **when**: Always selected for any migration/upgrade/rewrite that names or implies a target runtime, SDK, language, framework, package manager, build tool, browser tool, database/container dependency, or target version. Examples: Java/JDK upgrades (including Spring Boot targets), WinForms-to-Angular/React migrations (Node.js/npm), .NET target framework changes (.NET SDK), Python/Go/Ruby runtime changes, browser/E2E validation targets, and any user-specified version such as JDK 25 or Spring Boot 4.0. Select even for small projects and even when `deep_planning: false`. +- **skip when**: Metadata/documentation-only changes; pure code refactor that does not change runtime/build/test toolchain. +- **hard rules**: Preserve user-specified target versions verbatim. Do not downgrade or substitute target stack versions based on familiarity or LTS defaults. This must be a standalone execute-phase task before scaffold/implementation/build/test work; do not merge it into analysis or architecture tasks. Do not make target-env-prep depend on architecture/source-analysis tasks unless it explicitly needs an upstream artifact; it normally runs in parallel with them. Downstream scaffold/implementation/build/test tasks must depend on the target-env-prep artifact and may proceed only when it reports `READY` with concrete command evidence. If the requested target cannot be installed, provisioned, or activated in the current environment, mark `BLOCKED`, keep the requested target in downstream plans, and stop before implementation instead of silently substituting a different target. A target is not prepared merely because it is installed somewhere; it is prepared only when the active shell and planned build/test commands resolve to the requested version, or when the artifact gives exact activation commands/env vars that downstream tasks must use. + ### scaffold - **desc**: Set up target project structure + infrastructure (build files, CI skeleton, base config). For cross-stack migrations that produce a new codebase; not needed for in-place modifications. - **scope**: global +- **after**: target-env-prep - **when**: Cross-stack rewrite producing new project structure - **skip when**: In-place modification; same-stack upgrade @@ -86,6 +94,13 @@ LLM uses this to select task fragments for DAG generation. Each fragment is `{de - **when**: User requests CI/CD or deployment; new infrastructure needed - **skip when**: No deployment/infra requirements specified; user only asks for code migration +### cve-remediation +- **desc**: Scan dependency manifests against known CVEs (GitHub Security Advisories for Maven/Gradle, NuGet Vulnerability API for .NET) and remediate by upgrading vulnerable dependencies to patched versions; rebuild and re-scan to confirm the remediation took. Self-contained scan→fix→verify loop owned by the implementer — the scan also serves as detection, so a clean project exits cheaply. Reports findings to `cve-fix-summary` + a scan-report history. The internal rebuild/re-scan is the implementer's own self-check, NOT a separate quality gate, and stays within the Implementation phase label. +- **scope**: per-group +- **after**: [implementation] +- **when**: The migrated/generated code emits or modifies a dependency manifest (pom.xml/build.gradle/*.csproj/packages.config). This INCLUDES cross-stack rewrites that adopt a brand-new framework — do NOT assume a fresh stack is CVE-free: agents routinely pin stale or even EOL framework versions (e.g. a Struts→Spring rewrite landing on Spring Boot 2.7, or a Java EE→Spring rewrite landing on Spring Boot 3.2), whose transitive trees carry known advisories. The scan is the only objective check that catches this, and it exits cheaply when the tree is clean. Also always selected when the user mentions security/CVE/vulnerability, or assessment/arch-analysis flagged vulnerable or EOL dependencies. +- **skip when**: No dependency manifest is produced or changed (e.g. pure config/docs/asset change, or a single-file dependency-free edit); OR the user explicitly opted out of security/CVE work. Do NOT skip merely because the target framework is "new" or "latest" — that assumption is unreliable and is exactly what this scan exists to verify. Do NOT skip merely because the project is lite scope — a lite-scope change that still touches a dependency manifest must be scanned. + --- ## Validate Phase @@ -99,6 +114,7 @@ LLM uses this to select task fragments for DAG generation. Each fragment is `{de ### security-review - **desc**: Security audit — auth flows, input validation, secrets handling, dependency vulnerabilities, OWASP concerns. - **scope**: global +- **after**: cve-remediation - **when**: App has auth/security flows; user requests security audit; public-facing API - **skip when**: No auth/security in source app; user didn't request security review @@ -129,6 +145,7 @@ LLM uses this to select task fragments for DAG generation. Each fragment is `{de - **after**: feature-inventory, runtime-validation - **when**: feature-inventory was selected - **skip when**: feature-inventory was skipped +- **override**: force-included if `user_ask` explicitly requests a completeness/consistency/feature-parity check (see dag-generation SKILL "Explicit-request override"), regardless of skip-when. ### conformance-review - **desc**: Validate that tests executed according to strategy, all quality gates passed, and no regressions remain. @@ -136,6 +153,7 @@ LLM uses this to select task fragments for DAG generation. Each fragment is `{de - **after**: runtime-validation, test-strategy - **when**: Multiple validation steps exist; need final rollup - **skip when**: Only runtime-validation in validate phase (conformance adds no value as separate step) +- **override**: force-included if `user_ask` explicitly requests a completeness/consistency check (see dag-generation SKILL "Explicit-request override"), regardless of skip-when. --- @@ -154,12 +172,12 @@ Select tasks based on `change_type` (upgrade | extract | rewrite), `user_ask`, a These illustrate the expected scale of fragment selection across different project profiles. They are NOT templates to copy — derive your selection from the project's actual characteristics. The point is calibrating your judgment: a 1K LOC upgrade should not produce the same ceremony as a 200K LOC rewrite. -- **1.4K LOC, 1 module, rewrite (cross-stack migration)**: ~4 fragments. Most ceremony is overhead — single worker holds full context, features are obvious from code, target architecture is straightforward. deep_planning MUST be false. Skip coordination fragments (constitution, implementation-plan, quality-gate-plan, test-strategy), inventory fragments (feature-inventory, feature-parity-signoff), and detailed review fragments (arch-design, arch-review, security-review) unless the project has specific complexity signals (auth flows, data model changes, etc.). +- **1.4K LOC, 1 module, rewrite (cross-stack migration)**: ~5 fragments. Most ceremony is overhead — single worker holds full context, features are obvious from code, target architecture is straightforward. deep_planning MUST be false. Skip coordination fragments (constitution, implementation-plan, quality-gate-plan, test-strategy), inventory fragments (feature-inventory, feature-parity-signoff), and detailed review fragments (arch-design, arch-review, security-review) unless the project has specific complexity signals (auth flows, data model changes, etc.). Include cve-remediation — even a small cross-stack rewrite adopts a new dependency manifest that must be scanned. -- **12K LOC, single module, upgrade (version bump)**: ~3 fragments. Same-stack upgrade needs analysis, an implementation plan to sequence changes, and runtime validation. No new architecture, no feature changes, no DB changes. +- **12K LOC, single module, upgrade (version bump)**: ~4 fragments. Same-stack upgrade needs analysis, an implementation plan to sequence changes, cve-remediation (the version bump alters the dependency set, so re-scan and patch), and runtime validation. No new architecture, no feature changes, no DB changes. -- **50K LOC, 3 modules, rewrite (cross-stack)**: ~13 fragments. Multiple modules and cross-stack migration justify full ceremony — coordination, inventory, architecture, implementation planning, reviews, and validation. +- **50K LOC, 3 modules, rewrite (cross-stack)**: ~14 fragments. Multiple modules and cross-stack migration justify full ceremony — coordination, inventory, architecture, implementation planning, cve-remediation (the new stack adopts dependencies that must be CVE-scanned), reviews, and validation. -- **200K LOC, 8 modules, extract (module separation)**: ~14 fragments. Large-scale extraction with new service boundaries needs nearly all fragments except feature inventory (scope is one module with known API). +- **200K LOC, 8 modules, extract (module separation)**: ~15 fragments. Large-scale extraction with new service boundaries needs nearly all fragments except feature inventory (scope is one module with known API); cve-remediation applies because the extracted modules carry their dependency sets forward. -- **80K LOC, upgrade (dependency bump only)**: ~1 fragment. Pure dependency update — only runtime-validation needed to gate the build. +- **80K LOC, upgrade (dependency bump only)**: ~2 fragments. Pure dependency update — cve-remediation (the bump is exactly the dependency-set change that warrants a CVE scan; a clean scan is a cheap no-op) plus runtime-validation to gate the build. diff --git a/plugins/github-copilot-modernization/skills/data-architecture/SKILL.md b/plugins/github-copilot-modernization/skills/data-architecture/SKILL.md index 618a3f6..230d9dc 100644 --- a/plugins/github-copilot-modernization/skills/data-architecture/SKILL.md +++ b/plugins/github-copilot-modernization/skills/data-architecture/SKILL.md @@ -11,6 +11,61 @@ Analyze the project to document database configuration, entity models, data owne - `workspace-path` (optional): Path to the project to analyze (defaults to current directory) +## ⚠ Mermaid Safety Constraints — read BEFORE you write the ```mermaid block + +Mermaid `erDiagram` has a stricter grammar than flowchart. One bad attribute line or one stray `{` crashes the **whole** diagram with `Syntax error in text`. Stay strictly inside this subset: + +1. **Chart kind.** `erDiagram` only. +2. **Attribute grammar — exact shape.** Every attribute line inside an entity body MUST match: + + ``` + [] [""] + ``` + + - `` / ``: single tokens, plain text (letters, digits, underscore). No spaces, no backticks, no `@#$%&`. + - ``: optional. **Exactly one of** `PK`, `FK`, `UK` — never two, never combined. Compound tokens like `PK_FK`, `PKFK`, `PK/FK` crash the parser. + - ``: optional, must be a double-quoted string on one line. Free text, but obey rule 4. +3. **Relationships.** ` -- : "label"`. Each side independently picks `||` (exactly one), `|o`/`o|` (zero or one), `}o`/`o{` (zero or many), or `}|`/`|{` (one or many). The open side of `o`/`}`/`{` faces inward toward `--`. Always quote the label. +4. **Banned characters inside any quoted description or relationship label:** + + | Banned | Why it breaks | Replacement | + |---|---|---| + | `\n` (literal two chars) | escape removed | drop, or shorten | + | `{` `}` | opens an entity block | use `<...>` for placeholders, e.g. `"Redis key /basket/"` | + | `"` (a second double-quote) | closes description early | `'` (single quote) | + | `` ` `` (backtick) | not part of grammar | drop | + | `—` `–` (em/en dash) | parser may treat as edge | `-` (ASCII hyphen) | + | smart quotes `"` `"` `'` `'` | not ASCII | regular `"` and `'` | + | `@` `#` `$` `%` `&` | unsafe in names/descriptions | rephrase or drop | + +5. **Composite PK that is also FK.** Mark every column as `PK` only and note the FK role inside the quoted description. The FK relationship is already shown by the cardinality arrow — duplicating it as a second key marker crashes the parser. + +### Canonical attribute examples (copy these shapes) + +``` +int Id PK +string Name +int OwnerId FK +int InstructorId PK "also FK to Person (shared PK)" +int CourseId PK "composite PK; FK to Course" +int StudentId PK "composite PK; FK to Person" +string Email UK "unique" +decimal Budget "money column" +bytes RowVersion "concurrency token" +``` + +### Mandatory self-attestation + +Immediately before writing the ` ```mermaid ` opening fence, emit this exact one-line HTML comment in the markdown (it does not render — it is for your own visible attestation): + +``` + +``` + +If you cannot truthfully emit that comment, fix the diagram first. + +--- + ## Scope Boundaries — Avoid Redundancy with Other Skills This skill is part of a set of four complementary assessment skills. To avoid content duplication across their output documents, observe these scope rules: @@ -69,8 +124,9 @@ Identify: - Include relationship labels - Annotate which service owns each entity group (use comments or subgraph labels) -Example: +Reference example (this block satisfies every Safety Constraint — match its shape): + ~~~mermaid erDiagram Owner ||--o{ Pet : "has" @@ -198,68 +254,23 @@ A brief introduction (1-2 sentences) summarizing the data layer. - Collapse join tables into relationship annotations rather than showing them as separate entities - In the repository methods table, focus on non-CRUD custom methods; omit standard inherited methods -## Mermaid Syntax Rules - -Use `erDiagram`. The diagram must parse cleanly under the official Mermaid grammar — anything outside it crashes the whole diagram, not just the offending line. Stay inside the minimal legal subset below. - -### Attribute grammar - -Every attribute line inside an entity body MUST follow exactly this shape: - -``` - [] [""] -``` - -- `` and ``: single tokens, plain text (letters, digits, underscore). No spaces, no backticks, no `@#$%&`. -- ``: optional. **Exactly one of** `PK`, `FK`, `UK` — never two, never combined. Compound tokens like `PK_FK`, `PKFK`, `PK/FK` are not part of the grammar. -- ``: optional, must be a double-quoted string. The description is free text BUT must not contain `{`, `}`, or unescaped double quotes. - -### Canonical attribute examples (copy these shapes) - -``` -int Id PK -string Name -int OwnerId FK -int InstructorId PK "also FK to Person (shared PK)" -int CourseId PK "composite PK; FK to Course" -int StudentId PK "composite PK; FK to Person" -string Email UK "unique" -decimal Budget "money column" -bytes RowVersion "concurrency token" -``` - -Rule of thumb for **composite primary keys whose columns are also foreign keys** (join tables like `CourseAssignment`, shared-PK one-to-one tables like `OfficeAssignment`): mark every column as `PK` only, and note the FK role in the quoted description. The FK relationship itself is already conveyed by the cardinality arrows between entities — duplicating it as a second key marker is what crashes the parser. - -### Relationships - -- Cardinality is written as `--`, where each side independently picks one of: - - `||` — exactly one - - `|o` / `o|` — zero or one - - `}o` / `o{` — zero or many - - `}|` / `|{` — one or many - The "open" side of `o`/`}`/`{` always faces inward (toward the `--`). All resulting combinations are legal, e.g. `||--o{` (one-to-many), `||--||` (one-to-one), `}o--o{` (many-to-many), `}o--||` (many-to-one), `|o--o{` (zero-or-one to many), `||--o|` (one to zero-or-one). -- Always quote the label: `Owner ||--o{ Pet : "has"`. -- The label is free text but must not contain `{`, `}`, or unescaped double quotes. - -### Hard prohibitions (these crash the whole diagram, not just one line) - -1. **No `{` or `}` inside any quoted description or label.** Mermaid's ER parser treats `{` as the entity-body opener even inside quotes. Use `<...>` for placeholders, or rephrase in plain words. - - ❌ `string Key PK "Redis key /basket/{BuyerId}"` - - ✅ `string Key PK "Redis key /basket/"` -2. **No more than one key marker per attribute.** See the grammar above. -3. **No backticks, no special characters (`@#$%&`) in entity names, attribute names, or types.** -4. **No `\n` anywhere in the diagram.** The literal `\n` escape was removed in modern Mermaid (>= 9.x) and triggers "Syntax error in text". Keep every quoted description on a single line; if you need to express more, shorten the prose or split it across multiple attributes. - - ❌ `string Roles "comma-separated\nROLE_USER, ROLE_ADMIN"` - - ✅ `string Roles "comma-separated; ROLE_USER, ROLE_ADMIN"` +## Common failure patterns observed in past runs -### Self-check before emitting the diagram +Each row below is something the model actually produced that crashed the diagram. Use the ✅ form. -Before writing the ```` ```mermaid ```` block, walk every attribute line and verify it matches ` [] [""]` with **at most one** key token. Walk every quoted string and verify it contains no `{` or `}`. If a description needs to express two key roles (e.g., composite PK that is also FK), encode the second role as plain text inside the quoted description — never as a second token before the quote. +| ❌ Past mistake | ✅ Safe form | Why the ❌ crashed | +|---|---|---| +| `int OwnerId PK_FK` | `int OwnerId PK "FK to Owner"` | Compound key marker is not in grammar | +| `int OwnerId PK FK` | `int OwnerId PK "also FK to Owner"` | Two key markers on one line | +| `string Key PK "Redis key /basket/{BuyerId}"` | `string Key PK "Redis key /basket/"` | `{` opens an entity block even inside quotes | +| `string Roles "comma-separated\nROLE_USER, ROLE_ADMIN"` | `string Roles "comma-separated; ROLE_USER, ROLE_ADMIN"` | Literal `\n` | +| `string user-name` | `string userName` | `-` not allowed in attribute name | +| `Owner ||--o{ Pet : has` | `Owner ||--o{ Pet : "has"` | Relationship label must be quoted | ## Error Handling - **Unsupported project type**: Output a single line: `> ERROR: Unsupported project type. This skill supports Java, .NET, JavaScript, and TypeScript projects only.` -- **No data access layer found**: Output: `> ERROR: No recognized data access patterns or entities found at {workspace-path}. Verify the path is correct.` +- **No data access layer found**: Output: `> ERROR: No recognized data access patterns or entities found at workspace-path. Verify the path is correct.` - **Insufficient info**: Generate a best-effort diagram from available data. Add a note: `> Note: Some entities or relationships could not be fully identified.` ## Success Criteria @@ -271,4 +282,5 @@ Before writing the ```` ```mermaid ```` block, walk every attribute line and ver - Caching strategy section describes cache providers, patterns, and rationale - Data ownership boundaries describe shared vs isolated stores and cross-service data access patterns - Data Classification & Sensitivity table identifies PII/PHI/PCI fields and documents presence or absence of controls +- The ```mermaid block is preceded by the `` attestation comment - File saved to `.github/modernize/assessment/engines/facts/data-architecture.md` diff --git a/plugins/github-copilot-modernization/skills/dependency-map/SKILL.md b/plugins/github-copilot-modernization/skills/dependency-map/SKILL.md index 6b74fab..b347373 100644 --- a/plugins/github-copilot-modernization/skills/dependency-map/SKILL.md +++ b/plugins/github-copilot-modernization/skills/dependency-map/SKILL.md @@ -13,6 +13,44 @@ This skill focuses exclusively on **declared external dependencies** (libraries, - `workspace-path` (optional): Path to the project to analyze (defaults to current directory) +## ⚠ Mermaid Safety Constraints — read BEFORE you write the ```mermaid block + +Mermaid is unforgiving: one illegal character anywhere in the block crashes the **whole** diagram with `Syntax error in text`, not just the offending line. Stay strictly inside this subset: + +1. **Chart kind.** `flowchart LR` only. +2. **Subgraph form.** Always `subgraph ["display label"]` (id matches `[A-Za-z][A-Za-z0-9_]*`, no spaces, no punctuation). NEVER use the anonymous form `subgraph "label"` — it crashes whenever the label contains `(`, `)`, `/`, `-`, etc. +3. **Node form.** Use `Id["label"]` for libraries; pick one shape per node — do not stack brackets. +4. **Arrow form.** Solid `-->`, dotted `-.->` for transitive/indirect. Arrow labels MUST be double-quoted: `-->|"persistence"|`. Never bare `-->|persistence|`. +5. **No line breaks in labels.** The escape `\n` was removed in modern Mermaid and is the #1 cause of failures. Keep labels on one line (e.g., `"Spring Boot 2.7.18"` not `"Spring Boot\n2.7.18"`). +6. **Banned characters inside any label or subgraph title.** Use the ASCII replacement: + + | Banned | Why it breaks | Replacement | + |---|---|---| + | `\n` (literal two chars) | escape removed | drop, or `
` | + | `—` (em-dash, U+2014) | parser treats as edge | `-` (ASCII hyphen) | + | `–` (en-dash, U+2013) | parser treats as edge | `-` | + | `{` `}` | opens an entity block | drop braces | + | `"` inside a label | closes the label early | `'` (single quote) | + | `\|` inside a label | breaks edge-label parser | rephrase | + | `@` `#` `$` `%` `&` | unsafe | rephrase or drop | + | `(` `)` outside `["..."]` | unbalanced parens crash | only inside the quoted label | + | smart quotes `"` `"` `'` `'` | not ASCII | regular `"` and `'` | + +7. **Unique node IDs across the whole diagram.** No two nodes/subgraphs may share an id. +8. **`subgraph` must be closed by a matching `end` on its own line.** + +### Mandatory self-attestation + +Immediately before writing the ` ```mermaid ` opening fence, emit this exact one-line HTML comment in the markdown (it does not render — it is for your own visible attestation): + +``` +|"label"|, all subgraphs closed by end, ids unique --> +``` + +If you cannot truthfully emit that comment, fix the diagram first. + +--- + ## Execution Steps ### Step 1: Generate Dependencies Section @@ -53,15 +91,16 @@ Rules: - If a dependency doesn't fit any category, put it under "Utilities" - Collect test-scoped dependencies separately for the Test Dependencies section (Step 2) -**Diagram — Mermaid `flowchart LR`:** +**Diagram — Mermaid `flowchart LR`** (re-read the Safety Constraints above before writing): - Application as the central left-side node -- One `subgraph` per functional category -- Each dependency as a node showing name and version: `Lib["Library Name v1.2.3"]` +- One `subgraph` per functional category, using the `subgraph Id["display label"]` form +- Each dependency as a node showing name and version: `Lib["Library Name 1.2.3"]` (single line, no `\n`) - Arrows from Application to each category subgraph - If a BOM/parent POM manages versions, show it as a separate node linked to the dependencies it governs -Example: +Reference example (this block satisfies every Safety Constraint — match its shape): +|"label"|, all subgraphs closed by end, ids unique --> ~~~mermaid flowchart LR App["MyApplication"] @@ -74,7 +113,7 @@ flowchart LR Hibernate["Hibernate 5.6"] PgDriver["PostgreSQL Driver 42.6"] end - subgraph Messaging + subgraph Messaging["Messaging"] Kafka["Kafka Client 3.4"] end subgraph Cache["Caching"] @@ -154,35 +193,22 @@ Total test-scope dependencies: N - Keep the diagram under **40 nodes** to ensure readability and GitHub rendering compatibility - For multi-module projects (e.g., multi-module Maven/Gradle, multi-project .sln), show shared dependencies once and module-specific dependencies grouped by module -## Mermaid Syntax Rules - -The diagram must parse cleanly under **Mermaid >= 9.x**. Anything outside the legal subset crashes the entire diagram with `Syntax error in text`. - -- Use `flowchart LR` -- Avoid special characters (`@`, `#`, `$`, `%`, `&`) in node labels — use plain text -- Always quote arrow labels with double quotes: `-->|"label"|` -- Use `subgraph` for grouping, with a display name in quotes if it contains spaces -- Use `-.->` (dotted arrow) for transitive/indirect relationships -- Verify all node IDs are unique across the entire diagram - -### Line breaks in node labels — HARD RULE - -- **NEVER use `\n` for line breaks inside node labels.** The literal `\n` escape was removed in modern Mermaid and is the #1 cause of "Syntax error in text". -- **Use `
` instead**: `Node["First line
Second line"]`. -- Prefer single-line labels; move detail into the inventory table. -- ❌ `Spring["Spring Boot\n2.5.12"]` -- ✅ `Spring["Spring Boot
2.5.12"]` or `Spring["Spring Boot 2.5.12"]` +## Common failure patterns observed in past runs -### Self-check before emitting each ```mermaid block +Each row below is something the model actually produced that crashed the diagram. Use the ✅ form. -1. Search the block for the two characters `\n` — replace each with `
`. Zero `\n` must remain. -2. Confirm every node ID is unique and every `subgraph` is closed by `end`. -3. Confirm every arrow label is double-quoted. +| ❌ Past mistake | ✅ Safe form | Why the ❌ crashed | +|---|---|---| +| `subgraph "Database / ORM"` | `subgraph DB["Database / ORM"]` | Anonymous subgraph + `/` in title | +| `Spring["Spring Boot\n2.5.12"]` | `Spring["Spring Boot 2.5.12"]` | Literal `\n` | +| `App -->\|persistence\| DB` | `App -->\|"persistence"\| DB` | Bare arrow label | +| `Lib["Spring Boot — 2.5"]` | `Lib["Spring Boot - 2.5"]` | em-dash inside label | +| `Lib["foo {bar}"]` | `Lib["foo bar"]` | `{}` inside label | ## Error Handling - **Unsupported project type**: Output a single line: `> ERROR: Unsupported project type. This skill supports Java, .NET, JavaScript, and TypeScript projects only.` -- **No build files found**: Output: `> ERROR: No recognized build files found at {workspace-path}. Verify the path is correct.` +- **No build files found**: Output: `> ERROR: No recognized build files found at workspace-path. Verify the path is correct.` - **Incomplete dependency info**: Generate a best-effort diagram from available data. Add a note inside the diagram: `Note["Some dependencies could not be fully resolved"]` ## Success Criteria @@ -193,4 +219,5 @@ The diagram must parse cleanly under **Mermaid >= 9.x**. Anything outside the le - Version & Compatibility Risks paragraph highlights outdated or end-of-life dependencies - Notable Observations lists 2-4 noteworthy findings - Test Dependencies section lists detected test frameworks with versions and total count +- The ```mermaid block is preceded by the `` attestation comment - File saved to `.github/modernize/assessment/engines/facts/dependency-map.md` diff --git a/plugins/github-copilot-modernization/skills/implementing-code/SKILL.md b/plugins/github-copilot-modernization/skills/implementing-code/SKILL.md index 3cd86a0..733def5 100644 --- a/plugins/github-copilot-modernization/skills/implementing-code/SKILL.md +++ b/plugins/github-copilot-modernization/skills/implementing-code/SKILL.md @@ -63,6 +63,40 @@ From the task breakdown artifact, find tasks that match your current assignment: Unmet dependency not in current batch → report as blocked. +### Step 5.5: Discover Required API Endpoints (web applications only) + +**Before writing any implementation code**, check whether an endpoint contract test script exists in the project root: + +```bash +for script in api-test.sh api-check.sh smoke.sh health-check.sh test-api.sh; do + test -f "./$script" && echo "found: $script" && break +done +``` + +**If a script is found:** +1. Read its full contents to extract every endpoint it exercises: URL paths, HTTP methods, expected status codes, and expected response shape. +2. Treat every such endpoint as a **mandatory acceptance criterion** for this batch — equivalent to an explicit REQ in the feature spec. Missing even one will cause the post-build evaluation to fail. +3. Cross-check each endpoint against the matched task list. If any script endpoint is absent from the task breakdown, add it as an implicit sub-task in your execution plan (e.g., `T_API_dashboard: Implement GET /api/dashboard`). +4. Record the discovered endpoint list at the top of `batch-report.yaml` under `required_endpoints`: + ```yaml + required_endpoints: + - method: GET + path: /api/dashboard + source: api-test.sh + status: pending # updated to "implemented" when the endpoint is wired up and verified + - method: GET + path: /api/items + source: api-test.sh + status: pending + ``` + +**If no script is found:** +- Check `clarification.md` (if present) for any explicitly listed required API endpoints. +- Check the feature spec for API contract sections. +- If neither source provides an endpoint list, proceed without this step. + +This discovery step ensures that evaluation scripts are treated as first-class requirements from the start of implementation — not discovered only at post-build verification when fixes are costly. + ### Step 6: Execute **Ordering:** @@ -98,6 +132,99 @@ For tasks marked `[GUIDELINE:skill-name]`: - Halt on non-parallel task failure - For `[P]` tasks: continue successful ones, report failures +### Step 6.5: JS/TS Scaffolding Validation Gate (JS/TS projects only) + +**After any scaffolding step that generates or modifies a `package.json`**, execute this gate before proceeding to the next task. This gate is MANDATORY for all JavaScript and TypeScript projects. + +#### 6.5.1 — Verify required npm scripts + +Check that `package.json` contains BOTH a `build` script AND a `test` script: + +```bash +node -e " + const pkg = require('./package.json'); + const missing = ['build','test'].filter(s => !pkg.scripts || !pkg.scripts[s]); + if (missing.length) { console.error('MISSING scripts:', missing.join(', ')); process.exit(1); } + console.log('scripts OK: build=' + pkg.scripts.build + ', test=' + pkg.scripts.test); +" +``` + +**If either script is missing**, inject it immediately — do NOT defer: + +| Framework | Missing `test` script | Missing `build` script | +|-----------|----------------------|----------------------| +| Angular (`@angular/core` in deps) | `"test": "ng test --watch=false --browsers=ChromeHeadless"` | `"build": "ng build"` | +| React / Vite | `"test": "vitest run"` or `"test": "react-scripts test --watchAll=false"` | `"build": "vite build"` | +| React / CRA | `"test": "react-scripts test --watchAll=false --ci"` | `"build": "react-scripts build"` | +| Vue / Vite | `"test": "vitest run"` | `"build": "vite build"` | +| Next.js | `"test": "jest --ci"` | `"build": "next build"` | +| NestJS / Node | `"test": "jest --ci"` | `"build": "nest build"` | +| Generic TS | `"test": "jest --ci"` | `"build": "tsc"` | + +After injecting, confirm the script was written and re-verify with the check above. + +#### 6.5.2 — Run the test script + +After confirming both scripts exist, execute: + +```bash +npm test +``` + +(or `yarn test` / `pnpm test` if the project uses those package managers) + +- **Exit code 0**: gate passes — proceed. +- **Exit code != 0**: enter a remediation loop (max 3 iterations): + 1. Read the failure output to identify the root cause (missing browser binary, missing test files, misconfigured jest config, etc.) + 2. Fix the cause (install missing dev dependency, create a minimal placeholder test, fix config) + 3. Re-run `npm test` + 4. If still failing after 3 iterations, record the failure in `batch-report.yaml` under `warnings` with severity HIGH and continue — do NOT block the entire batch: + ```yaml + warnings: + - severity: HIGH + message: "npm test failed after 3 remediation attempts — " + ``` + +> **Why this gate exists**: Eval harnesses run `npm test` unconditionally. A scaffolded project without a `test` script causes an immediate fatal error (`npm error Missing script: "test"`) that masks all other results. Catching this at scaffolding time costs ~5 seconds; missing it causes total batch failure. + +### Step 6.6: API Endpoint Verification (web-application backends only) + +For any batch that produces or modifies a web-application backend (Spring Boot, Express, NestJS, FastAPI, Django, ASP.NET Core, Go HTTP servers, and similar), verify endpoints **respond correctly at runtime** before the batch is considered complete. A passing build proves only compilation, not that routes are wired up. + +#### 6.6.1 — Run the discovered endpoint contract (if any) + +If Step 5.5 found an endpoint contract script (`api-test.sh`, `api-check.sh`, …), start the application if not already running, then execute it: + +```bash +bash ./api-test.sh # or whichever script Step 5.5 recorded +echo "api-test exit code: $?" +``` + +- **Exit code 0** → all endpoint assertions pass → proceed. +- **Exit code != 0** → enter the fix loop (max 3 iterations): + 1. Read the output to identify which endpoints returned errors (missing route, wrong status code, unexpected body). + 2. Implement or correct the backend endpoint(s): add the missing controller/handler, fix route mapping, return the expected response shape. + 3. Re-run the script. Repeat until exit code is 0 or 3 iterations are exhausted. + 4. If still failing after 3 iterations, record the failure in `batch-report.yaml` under `warnings` with severity HIGH and escalate via `[notify:coordinator]` — do NOT mark the affected endpoints `implemented`. + +#### 6.6.2 — Manual probe (no script found) + +If no endpoint contract script exists, probe every REST endpoint the batch implemented after starting the application: + +```bash +curl -sf -o /dev/null -w "%{http_code}" http://localhost: +``` + +Every implemented endpoint MUST return a 2xx status code. A 404 or 500 means the route is not wired correctly — fix before completing the batch. Record probe results in the batch report under `endpoint_probes`: + +```yaml +endpoint_probes: + - method: GET + path: /api/dashboard + status: 200 + result: PASS +``` + ### Step 7: Write Checkpoint After ALL tasks in this batch complete, write `checkpoints/tasks-to-impl.yaml` using `templates/tasks-to-impl-checkpoint-template.yaml`. This is REQUIRED — completeness gate reads it to verify traceability. @@ -106,6 +233,13 @@ After ALL tasks in this batch complete, write `checkpoints/tasks-to-impl.yaml` u Generate batch result report per `references/batch-report-format.md` (YAML format). +**If `required_endpoints` was populated in Step 5.5**, update each entry's `status` to `"implemented"` once the corresponding endpoint is wired up and verified to respond correctly. Any entry still `"pending"` at report time is a gap — record it under `warnings` with severity HIGH: +```yaml +warnings: + - severity: HIGH + message: "Endpoint GET /api/dashboard was required by api-test.sh but not implemented in this batch" +``` + ## Resources ### References diff --git a/plugins/github-copilot-modernization/skills/implementing-code/references/batch-report-format.md b/plugins/github-copilot-modernization/skills/implementing-code/references/batch-report-format.md index 7fd6865..5db0054 100644 --- a/plugins/github-copilot-modernization/skills/implementing-code/references/batch-report-format.md +++ b/plugins/github-copilot-modernization/skills/implementing-code/references/batch-report-format.md @@ -18,6 +18,21 @@ files_changed: - file: "path/to/another-file" change_type: "modified" description: "Brief description of change" +# Role-neutral upstream artifact consumption (required when dependency artifacts exist) +upstream_artifacts_consumed: + T001: + - artifact: "artifacts/t1-architect-wire-contracts.md" + used_for: "HTTP endpoint contracts and JSON response keys" + - artifact: "artifacts/units/request_action/behavior.yaml" + used_for: "request form branches and save side effects" +evidence_mapping: + T001: + - upstream: "artifacts/t1-architect-wire-contracts.md#POST /saverequest" + output: "src/main/java/.../RequestController.java#saveRequest" + evidence: "RequestControllerTest#saveRequest_*" + - upstream: "artifacts/units/request_action/behavior.yaml#branches" + output: "src/main/java/.../RequestController.java" + evidence: "branch tests pass" # Source-anchored traceability (rewrite mode only) source_references: T001: @@ -62,6 +77,8 @@ test_results: | `completed_tasks` | Yes | List of task IDs successfully completed | | `blocked_tasks` | Yes | Tasks that couldn't run due to unmet dependencies | | `files_changed` | Yes | Per-task list of files created/modified/deleted | +| `upstream_artifacts_consumed` | When dependency artifacts exist | Upstream artifacts read by each task and what each was used for | +| `evidence_mapping` | When dependency artifacts exist | Upstream artifact contract/row/section → this task's output and verification evidence | | `source_references` | Rewrite mode | Source-anchored traceability: files read, branches/validations/errors/side-effects preserved | | `traceability` | Yes | REQ → Plan → Task → Files mapping | | `protocol_violation` | Yes | Whether any constitution principle was violated | diff --git a/plugins/github-copilot-modernization/skills/quality-gates/references/gate-completeness.md b/plugins/github-copilot-modernization/skills/quality-gates/references/gate-completeness.md index 0e59a07..39fd368 100644 --- a/plugins/github-copilot-modernization/skills/quality-gates/references/gate-completeness.md +++ b/plugins/github-copilot-modernization/skills/quality-gates/references/gate-completeness.md @@ -2,6 +2,8 @@ **Load**: constitution, feature spec, plan.md, implementation files, all 3 checkpoints (spec-to-plan.yaml, plan-to-tasks.yaml, tasks-to-impl.yaml) +> **Lite-path note (`deep_planning: false`):** small projects run without a planning phase, so `plan.md`, the feature spec, and the `spec-to-plan` / `plan-to-tasks` checkpoints are legitimately never produced. When they are absent because the lite path skipped `implementation-plan`, treat them as **N/A — not missing/CRITICAL**, and validate against the task list and implementation evidence instead. This note applies throughout the Checklist and Process below. + ## Build Verdict (blocking — evaluate FIRST) Judge build status ONLY from the `## Smoke Test Verdict` block in the smoke-test artifact. @@ -14,13 +16,13 @@ A worker's prose ("build passed") or self-applied label is NOT evidence. Read `b ## Checklist -- [ ] All plan items have corresponding implementation files → *CRITICAL if missing* +- [ ] All plan items have corresponding implementation files → *CRITICAL if missing (lite path with no plan.md: verify implementation evidence against the task list / `tasks-to-impl.yaml` instead)* - [ ] Build succeeds, tests pass — build half: see **Build Verdict** section above; tests half: verify test results in implementation artifacts → *CRITICAL if failure* - [ ] Constitution followed in implementation → *CRITICAL if violated* - [ ] All P1 requirements fulfilled → *CRITICAL if unmet* - [ ] Every implementation task artifact includes `## Test Results` with pass/fail/skip counts and test command → *CRITICAL if missing or failed > 0* - [ ] Testing strategy executed as planned: primary validation stack used; fallback only with documented blocker evidence → *CRITICAL if primary stack skipped without documented failure evidence (exact command + exact error output + explanation why it cannot be resolved). "H2 already worked" or "setup was complex" are not valid blockers. Partial strategy execution (e.g., integration but no E2E when E2E was planned) is also CRITICAL unless a documented, reproducible technical blocker prevented execution.* -- [ ] Functional equivalence verified *(brownfield only: migration and rewrite)* → *CRITICAL if unverified* +- [ ] Consistency verified *(brownfield; change-type-aware)* → *CRITICAL if unverified*: migration / rewrite → functional equivalence (`references/functional-equivalence.md`); upgrade → upgrade-consistency, i.e. no residual old version/API, no mixed old/new across modules (`references/upgrade-consistency.md`) ## Constitution Hardstop Rule @@ -32,12 +34,16 @@ A worker's prose ("build passed") or self-applied label is NOT evidence. Read `b - `checkpoints/spec-to-plan.yaml` - `checkpoints/plan-to-tasks.yaml` - `checkpoints/tasks-to-impl.yaml` + + **Lite-path tolerance (`deep_planning: false`):** when `implementation-plan` was not selected, `spec-to-plan.yaml` and `plan-to-tasks.yaml` are never written — treat them as **N/A, not CRITICAL**, and verify only `tasks-to-impl.yaml` (or equivalent implementation evidence). Do NOT fail the gate for plan-phase checkpoints that the lite path legitimately never creates. 2. Check plan.md Requirement Mapping table Implementation Evidence column 3. Verify all referenced implementation files exist 4. Evaluate build per the **Build Verdict** section (blocking — must be done before advancing); verify tests per implementation task artifacts 5. Verify constitution compliance in code 6. Confirm P1 requirements are implemented -7. For brownfield (migration and rewrite): verify functional equivalence per `references/functional-equivalence.md` +7. Verify the change-type-appropriate consistency check (brownfield): + - migration / rewrite → functional equivalence per `references/functional-equivalence.md` + - upgrade → upgrade-consistency per `references/upgrade-consistency.md` (target version reached everywhere, no residual old API, no mixed old/new versions, deprecated symbols replaced) 8. Verify testing strategy conformance: - Compare planned validation stack (from plan.md testing strategy) against actual test evidence - If primary stack was specified (e.g. Playwright, Testcontainers), confirm it was **actually attempted** (look for dependency in pom.xml/package.json, test files using those tools, or documented installation attempt with error) diff --git a/plugins/github-copilot-modernization/skills/quality-gates/references/upgrade-consistency.md b/plugins/github-copilot-modernization/skills/quality-gates/references/upgrade-consistency.md new file mode 100644 index 0000000..8ff047b --- /dev/null +++ b/plugins/github-copilot-modernization/skills/quality-gates/references/upgrade-consistency.md @@ -0,0 +1,85 @@ +--- +name: Upgrade Consistency Verification +description: Verify that a same-stack upgrade (version bump / SDK or dependency swap) was applied completely and consistently — no residual old version, no mixed old/new usage, every call site migrated. +mode: upgrade +--- + +## Overview + +This is the **consistency arm** of the completeness gate for `change_type: upgrade` +(version bumps, SDK swaps, dependency updates, same-stack modernization — e.g. an +Azure SDK major-version upgrade). Unlike a cross-stack rewrite, an upgrade does not +need *functional equivalence* of rewritten business logic; what matters is that the +upgrade was applied **completely and consistently** across the whole codebase, leaving +no partial-migration state behind. + +A small/simple upgrade is exactly where this check matters most: the multi-agent +ceremony is light, so it is easy to bump a dependency in one module and silently leave +the old API in use elsewhere. + +## When to Use + +- **Mode**: `change_type: upgrade` (same-stack version bump / dependency or SDK swap) +- **Phase**: Completeness Check — invoked by the completeness gate (`gate-completeness.md` step 7) +- **Prerequisites**: Implementation complete; build/smoke-test evidence available +- **Inputs**: the user-specified target (library + version), the project profile + (`assessment.transformations` fromStack/toStack and versions), and the changed files + +## Consistency Definition + +An upgrade is **consistent** when: +- The target library/SDK is at the requested version **everywhere** it is declared +- No source file still imports, references, or calls the **old** API/package/namespace +- No module is left on the **old** version (no mixed old/new across the build) +- Every deprecated/removed symbol from the old version has been replaced +- Configuration, properties, and build/dependency metadata match the new version +- The full build and tests are green (per the **Build Verdict** in `gate-completeness.md`) + +## Checklist (all CRITICAL unless noted) + +- [ ] **Target version reached** — every declaration of the upgraded artifact is at the + requested target version. No declaration left at the old version. → *CRITICAL* +- [ ] **No residual old API** — no remaining imports / package references / namespace + usages / API calls belonging to the old version anywhere in source. Grep the old + package/namespace and confirm zero in-scope hits (excluding generated/vendored code). → *CRITICAL* +- [ ] **No mixed old/new versions** — a single coherent version across all modules and + the dependency/BOM graph; no module still resolves the old version transitively + where it is used directly. → *CRITICAL* +- [ ] **Deprecated/removed symbols replaced** — every symbol removed or deprecated by the + target version has a migrated replacement; none left calling a removed API. → *CRITICAL* +- [ ] **Config & metadata updated** — properties, config files, and build descriptors + reference the new version's expected keys/coordinates, not the old ones. → *HIGH* +- [ ] **Build & tests green** — full root-level build passes and tests pass per the + Build Verdict; no module excluded to make the upgrade "pass". → *CRITICAL* +- [ ] **Target version preserved** — the delivered version matches the user-requested + target verbatim; it was not downgraded or substituted to an LTS/familiar default. → *CRITICAL* + +## Verification Strategy + +1. Read the requested target (library + version) from `user_ask` and + `assessment.transformations` (fromStack/fromStackVersion → toStack/toStackVersion). +2. **Version sweep** — enumerate every place the artifact's version is declared + (e.g. `pom.xml`/`build.gradle`/BOM, `package.json`, lockfiles, `.csproj`). Confirm + all are at the target version; record any left at the old version. +3. **Residual-old sweep** — search the source tree for the old package / namespace / + import / API symbols. Any in-scope hit is a partial migration → CRITICAL. +4. **Mixed-version check** — resolve the effective dependency graph; flag any module + that still uses the old version directly while others use the new one. +5. **Deprecation check** — for symbols removed/deprecated between source and target + versions, confirm each usage was migrated to the replacement. +6. **Config/metadata check** — verify config keys, coordinates, and build descriptors + match the new version. +7. **Build/test confirmation** — confirm the Build Verdict is PASS and tests passed. + +## Report + +Write findings into `migration-summary.md` (the completeness gate's report). Include a +short table of: declarations swept (old→new), residual-old hits (file:line), mixed-version +modules, unmigrated deprecated symbols, and the build/test verdict. + +## Verdict + +- **PASS**: target version reached everywhere, zero residual-old usage, no mixed + versions, deprecated symbols replaced, build/tests green, requested version preserved. +- **FAIL**: any residual old API, any module left on the old version, any unmigrated + removed/deprecated symbol, a downgraded/substituted target, or a non-green/scoped build. diff --git a/plugins/github-copilot-modernization/skills/runtime-validation/SKILL.md b/plugins/github-copilot-modernization/skills/runtime-validation/SKILL.md index 773ec0c..b24c99a 100644 --- a/plugins/github-copilot-modernization/skills/runtime-validation/SKILL.md +++ b/plugins/github-copilot-modernization/skills/runtime-validation/SKILL.md @@ -146,7 +146,9 @@ Beyond Docker and Node.js, verify these additional prerequisites when they apply ### 1.3.2 Legacy Test Asset Inventory -Before finalising the testing strategy, check whether legacy E2E or integration tests are available. They may come from two sources — check both: +Before finalising the testing strategy, identify the project's **canonical test command** — the top-level command used in CI (e.g., `yarn test:unit`, `mvn test`, `pnpm test`, `npm test`). Find it in the root `package.json` scripts, `pom.xml`, or `build.gradle`. Run it first and record the baseline passing count. All new tests written during migration must be reachable by this same command — do not introduce a separate test tool that bypasses it. If a new framework is added (e.g., vitest alongside Jest), it must be wired into the canonical command so both run together. + +Then check whether legacy E2E or integration tests are available. They may come from two sources — check both: **Source 1 — User-provided tests** The user may directly supply test files or paste test code in their request. These take priority over anything discovered on disk. Accept them as-is and skip the file-scan for the journeys they already cover. @@ -407,10 +409,30 @@ Proceed directly to writing new tests from the testing strategy's critical journ When tests fail: 1. Capture test output + application logs -2. Correlate errors to identify root cause -3. **If source code bug** → escalate to responsible role via `[notify:role]`. Do NOT modify production code. -4. **If test code issue** → fix and retry -5. Max 3 fix iterations → escalate remaining via `[notify:coordinator]` +2. **Capture the test command's exit code** (`$?` on Unix/macOS/Linux, `$LASTEXITCODE` on PowerShell). Record it in the evidence block. A non-zero exit code means the tier is FAIL even if some tests passed. +3. Correlate errors to identify root cause +4. **If source code bug** → escalate to responsible role via `[notify:role]`. Do NOT modify production code. +5. **If test code issue** → fix and retry +6. Max 3 fix iterations → escalate remaining via `[notify:coordinator]` + +**Exit-code gate (MANDATORY):** After every test command completes, evaluate: +- `rc == 0` → tier may be PASS (verify pass/fail counts are consistent) +- `rc != 0` → tier is **FAIL**, regardless of reported pass count. Enter fix loop. If after 3 iterations the exit code is still non-zero, write `overall: FAIL` and escalate immediately: + ``` + [notify:coordinator] CRITICAL: Test command exited rc= after 3 fix iterations — task cannot complete. + Command: + Exit code: + Failing tests: + Counts: + ``` + Do NOT write `[DONE]` while any tier has `rc != 0` without a matching waiver. + +**Waiver exception:** The ONLY condition under which `rc != 0` does not block the DONE signal is when ALL of the following hold: +1. A named waiver artifact (e.g., `known-defects.md`) was produced by a prior task and is listed in this task's `## Dependency Artifacts`. +2. Every failing test is listed by name in that artifact with a note that the defect predates this migration. +3. No new failures are present beyond those listed in the waiver. + +If all three conditions hold, record the waiver artifact name in the verdict block and proceed. Otherwise, FAIL. ### 2.4 Step 4: Evidence & Verdict @@ -424,11 +446,15 @@ environment: infra-tier: PRIMARY(Docker-based)|FALLBACK(embedded/in-memory) — browser-tier: PRIMARY(Playwright)|FALLBACK(MockMvc)|SKIPPED — startup: PASS|FAIL — , , -integration: PASS|FAIL|UNVERIFIED — , -e2e: PASS|FAIL|PARTIAL|UNVERIFIED — , , +integration: PASS|FAIL|UNVERIFIED — , , , +e2e: PASS|FAIL|PARTIAL|UNVERIFIED — , , , , overall: PASS|FAIL|NEEDS_SIGNOFF — ``` +> **`exit_code` is REQUIRED in every tier line.** A tier is FAIL if `exit_code != 0`, regardless of the pass/fail counts. Do not omit `exit_code`. +> +> **PASS verdict requires `exit_code: 0`.** If any tier has `exit_code != 0` and no named waiver artifact covers all failing tests by name, `overall` MUST be `FAIL`. A partial-pass count (e.g., "59/61 passed") with `exit_code: 2` is still `FAIL`. + Also produce `runtime-validation-report.md`: ```markdown @@ -438,11 +464,11 @@ Also produce `runtime-validation-report.md`: **Target**: [project path] ## Summary -| Step | Status | Details | -|------|--------|---------| -| Startup | ✓ PASS | Started in 8.3s, /actuator/health → 200 | -| Integration Tests | ✓ PASS | 3 test files, 12 tests, all green | -| E2E Tests | N/A | No browser UI — skipped per testing strategy | +| Step | Status | Exit Code | Details | +|------|--------|-----------|---------| +| Startup | ✓ PASS | n/a | Started in 8.3s, /actuator/health → 200 | +| Integration Tests | ✓ PASS | 0 | 3 test files, 12 tests, all green | +| E2E Tests | N/A | n/a | No browser UI — skipped per testing strategy | **Overall**: PASS