Copilot review instructions added: AEM specific

.github/copilot-instructions.md

@@ -0,0 +1,107 @@
+# AEM Core Forms Components — Copilot Review Instructions
+
+## Project Context
+This is Adobe's open-source AEM Core Forms Components repository. The tech stack is Java (Sling Models, OSGi services), HTL (HTML Template Language), JavaScript, Maven, and JCR content packages. Components follow the AEM Core Components architecture with the Proxy + Delegation pattern.
+
+## Review Philosophy
+- Only comment when you have HIGH CONFIDENCE (>80%) that an issue exists
+- Focus on violations of the specific standards below, not generic style preferences
+- Do NOT flag formatting issues — we use `eclipse-formatter.xml` and `mvn -Pformat-code` for that
+- Be specific: reference which standard is violated and how to fix it
+- Prioritize: security > accessibility > architectural patterns > code quality > suggestions
+
+## CI Pipeline Context
+Our CI already runs: Maven build, code formatting check (eclipse-formatter.xml), `it.only`/`describe.only` detection in test files, and base branch auto-sync. Do NOT flag issues that CI will catch.
+
+---
+
+## Java / Sling Model Standards
+
+### Annotations (CRITICAL — flag every violation)
+- Use `@ValueMapValue` for JCR property injection, NEVER generic `@Inject` — @Inject is ambiguous and resolves from multiple sources unpredictably
+- Sling Models MUST have `@Model` annotation with explicit `adaptables`, `adapters`, and `resourceType`
+- Every Sling Model must implement an interface (adapter pattern) — never expose impl class to HTL
+- Use `@PostConstruct` for init logic, never constructors
+- Use `DefaultInjectionStrategy.OPTIONAL` only when properties are genuinely optional
+
+### Architecture Patterns
+- Proxy Component Pattern is mandatory: content `sling:resourceType` must NEVER contain version numbers
+- Sling Model implementations go in `*.internal.models.*` package — internal packages must NOT be OSGi-exported
+- Use delegation via `@Self @Via(type = ResourceSuperType.class)` when extending core components, not class inheritance
+- Service user mappings for resource resolver access — NEVER `getAdministrativeResourceResolver()` (deprecated, security risk)
+
+### Error Handling
+- No empty catch blocks — log with context (SLF4J Logger, not System.out)
+- No catching generic `Exception` — catch specific exception types
+- No `e.printStackTrace()` — use Logger
+
+### Testing
+- New Java classes MUST have JUnit tests using `AemContext` / `AemContextExtension`
+- Bug fixes must include regression tests
+- Test both success and error paths
+- No `Thread.sleep()` in tests — use mocks or async utilities
+
+---
+
+## HTL (HTML Template Language) Standards
+
+### Data Binding
+- `data-sly-use` must reference the Sling Model INTERFACE fully qualified class name, never the implementation class
+- Complex logic belongs in the Sling Model, not in HTL expressions
+- All user content must use proper XSS context: `@context='html'`, `@context='uri'`, etc.
+- NEVER use `@context='unsafe'` — this disables all XSS protection
+
+### Accessibility (CRITICAL — this project has active a11y remediation)
+- Every form field MUST have a programmatic label (`<label for="">` or `aria-label`)
+- Radio/checkbox groups MUST use `<fieldset>` + `<legend>`
+- Interactive elements must be keyboard operable (Tab, Enter, Space, Arrows)
+- Expandable panels must use `aria-expanded` that updates on state change
+- Validation errors must link to field via `aria-describedby`
+- Focus must move to first visible field on form/panel load
+
+### HTML Structure
+- Follow BEM naming: `.cmp-<componentname>__<element>`
+- Include `data-cmp-is` for JS initialization hooks
+- No inline JavaScript — use clientlibs
+- No hardcoded strings — use `${'key' @ i18n}` for internationalization
+
+---
+
+## JavaScript Standards
+- ES6+ syntax required (const/let, arrow functions, template literals)
+- No jQuery for new code — vanilla JS only
+- Null-check DOM elements before accessing properties
+- Custom functions must follow `@aemforms/af-core` API patterns
+- Form model changes go through Adaptive Forms rule engine, not direct DOM manipulation
+
+---
+
+## Security Checklist (flag as HIGH severity)
+- No secrets, API keys, or tokens in code
+- No SQL/JCR query string concatenation — use QueryBuilder with parameterized queries
+- All user input must be validated and sanitized server-side
+- Authentication checks on every protected servlet/endpoint
+- No `eval()` or `Function()` constructor
+- CSRF token validation on all POST endpoints
+- File uploads must validate type, size, and content
+
+---
+
+## Maven / Build Standards
+- No SNAPSHOT dependencies in release/master branch
+- Dependency versions declared in parent `pom.xml`
+- Use `bnd-maven-plugin` for OSGi manifests (NOT `maven-bundle-plugin`)
+- AEM SDK dependencies use `provided` scope
+- No skipping tests
+
+---
+
+## Banned Patterns (flag immediately)
+- `@Inject` for JCR properties → use `@ValueMapValue`
+- `getAdministrativeResourceResolver()` → use service user
+- `System.out.println` → use SLF4J Logger
+- `commons-lang` v2 → use `commons-lang3`
+- `org.json` → use `com.google.gson` or `javax.json`
+- `@context='unsafe'` in HTL → use proper XSS context
+- Version numbers in content `sling:resourceType`
+- `maven-bundle-plugin` → use `bnd-maven-plugin`

.github/instructions/content-xml-dialogs.instructions.md

@@ -0,0 +1,23 @@
+---
+applyTo: "**/.content.xml,**/_cq_dialog/**,**/_cq_design_dialog/**"
+---
+
+# Content XML & Dialog Review Rules
+
+## Proxy Component Pattern (CRITICAL)
+- `sling:resourceType` on content nodes must NEVER contain version numbers (e.g., `/v1/`, `/v2/`)
+- Version numbers are only allowed in `sling:resourceSuperType` on the proxy component node
+- If this is a new component definition, verify the proxy pattern is followed
+
+## Dialog Structure
+- Dialog root must be `cq:dialog` with `sling:resourceType="cq/gui/components/authoring/dialog"`
+- Use tabs for dialogs with more than 5 fields
+- Field names (`./propertyName`) must be camelCase
+- Required fields must have `required="{Boolean}true"`
+- Non-obvious fields should include `fieldDescription` help text
+
+## Content Package Rules
+- Use Granite UI components (Coral 3) for dialog fields
+- Proper XML namespace declarations required
+- No hardcoded `/content/*` paths — use relative paths or path browser with `rootPath`
+- `jcr:primaryType` must be set correctly (`nt:unstructured` for dialog nodes)

.github/instructions/htl-templates.instructions.md

@@ -0,0 +1,30 @@
+---
+applyTo: "**/*.html"
+---
+
+# HTL Template Review Rules
+
+## Data Binding
+- `data-sly-use` must reference the Sling Model INTERFACE (e.g., `com.adobe.aem.forms.core.components.models.TextInput`), never the implementation class (`*Impl`)
+- Complex logic must be in the Sling Model — if an HTL expression has more than a simple property access or boolean check, flag it
+- No inline JavaScript — all JS must go through clientlibs
+
+## XSS Protection (CRITICAL)
+- All dynamic content must use proper display context: `@context='html'`, `@context='text'`, `@context='uri'`
+- Flag ANY use of `@context='unsafe'` as a security error — this disables XSS protection entirely
+- User-generated content without explicit context defaults to text but should be explicit
+
+## Accessibility (CRITICAL — Active remediation in progress)
+- Every `<input>`, `<select>`, `<textarea>` MUST have an associated `<label for="">` or `aria-label`
+- Radio button groups and checkbox groups must be wrapped in `<fieldset>` with `<legend>`
+- Panels that expand/collapse must have `aria-expanded` attribute that toggles
+- Error messages must be linked to their field via `aria-describedby`
+- All interactive elements must be keyboard operable — check for `tabindex`, key event handlers
+- Focus management: new panels/forms should focus the first visible field
+
+## HTML Structure
+- Root element must use BEM class naming: `.cmp-<componentname>`
+- Child elements: `.cmp-<componentname>__<element>`
+- Include `data-cmp-is` attribute for JavaScript initialization
+- No hardcoded user-facing strings — use `${'key' @ i18n}` for internationalization
+- Support Style System via `${properties.cssClass}`

.github/instructions/java-sling-models.instructions.md

@@ -0,0 +1,36 @@
+---
+applyTo: "**/*.java"
+---
+
+# Java & Sling Model Review Rules
+
+## Sling Model Checklist
+When reviewing Java files that contain `@Model` annotation:
+
+1. Verify `@Model` has explicit `adaptables`, `adapters`, and `resourceType` parameters
+2. Flag ANY use of `@Inject` for JCR properties — must use `@ValueMapValue` instead
+3. Verify the model implements an interface from `*.models.*` package
+4. Verify the implementation is in `*.internal.models.*` package
+5. Check that `resourceType` in `@Model` matches the component's `sling:resourceType`
+6. If extending a core component, verify delegation pattern with `@Self @Via(type = ResourceSuperType.class)`
+
+## Servlet Checklist
+When reviewing files extending `SlingSafeMethodsServlet` or `SlingAllMethodsServlet`:
+
+1. Must use `@SlingServletResourceTypes` (not path-based registration)
+2. Must explicitly declare supported HTTP methods
+3. POST servlets must validate CSRF tokens
+4. Response content type must be explicitly set
+5. All input parameters must be validated and sanitized
+
+## Security — Flag as Error
+- `getAdministrativeResourceResolver()` — banned, use service user
+- `System.out.println` or `e.printStackTrace()` — use SLF4J Logger
+- Empty catch blocks `catch(Exception e) {}` — must log the error
+- Catching generic `Exception` — catch specific types
+- String concatenation in JCR queries — use QueryBuilder API
+
+## Testing
+- New or modified classes should have corresponding `*Test.java` files
+- Tests must use `@ExtendWith(AemContextExtension.class)`
+- No `Thread.sleep()` in tests

.github/instructions/javascript.instructions.md

@@ -0,0 +1,21 @@
+---
+applyTo: "**/*.js"
+---
+
+# JavaScript Review Rules
+
+## General
+- ES6+ syntax required: `const`/`let` (never `var`), arrow functions, template literals
+- No jQuery for new code — use vanilla JavaScript
+- Always null-check DOM elements before accessing properties or methods
+- Event listeners should use event delegation where possible
+- All client-side code must be wrapped in AEM clientlib categories
+
+## Adaptive Forms Specific
+- Custom functions must follow `@aemforms/af-core` API patterns and function registry
+- Form model changes must go through the Adaptive Forms rule engine — no direct DOM manipulation of form state
+- Client-side validation must complement server-side validation, never replace it
+- All custom functions must handle null/undefined input gracefully
+
+## Internationalization
+- All user-facing strings must use `Granite.I18n.get()` — no hardcoded strings

.github/instructions/maven-pom.instructions.md

@@ -0,0 +1,21 @@
+---
+applyTo: "**/pom.xml"
+---
+
+# Maven POM Review Rules
+
+## Dependencies
+- No SNAPSHOT dependencies in master/release branches
+- All dependency versions must be declared in the parent `pom.xml`, not in individual modules
+- AEM SDK dependencies must use `provided` scope
+- Flag addition of `commons-lang` (v2) — must use `commons-lang3`
+- Flag addition of `org.json` — must use `com.google.gson` or `javax.json`
+- Flag addition of `com.google.guava` — use Java standard library equivalents
+
+## Build Plugins
+- OSGi bundle manifests must use `bnd-maven-plugin` — flag if `maven-bundle-plugin` is used
+- Do not skip tests (`-DskipTests` should not be in any profile used for CI)
+
+## Content Packages
+- Content packages must follow FileVault packaging conventions
+- Verify `content-package-maven-plugin` configuration is correct

.github/instructions/test-files.instructions.md

@@ -0,0 +1,25 @@
+---
+applyTo: "**/test/**,**/*Test.java,**/*Spec.js,**/*.spec.*"
+---
+
+# Test File Review Rules
+
+## Java Tests
+- Must use `@ExtendWith(AemContextExtension.class)` — not JUnit 4 `@RunWith`
+- Use `AemContext` for mock AEM environment setup
+- Mock external services with `context.registerService()` — never call real services
+- Test method names must be descriptive: `testGetValue_whenPropertyMissing_returnsNull()`
+- Each test verifies ONE behavior
+- No test-to-test dependencies — each test must run independently
+- Test both success AND error paths
+
+## Banned in Tests
+- No `it.only` or `describe.only` — our CI catches this but flag it anyway
+- No `Thread.sleep()` — use mocks or async utilities
+- No hardcoded AEM instance URLs (like `http://localhost:4502`)
+- No tests requiring external network connectivity
+
+## Coverage
+- New Sling Models must have tests covering: property injection, @PostConstruct logic, edge cases (null values)
+- New servlets must have tests covering: valid request, invalid input, missing auth
+- Accessibility-related changes must have tests verifying ARIA attribute rendering