-
Notifications
You must be signed in to change notification settings - Fork 3
Add path-specific Copilot instructions, code-reviewer agent, skill prompts, and setup workflow #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Copilot
wants to merge
2
commits into
main
Choose a base branch
from
copilot/add-path-specific-instruction-files
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| --- | ||
| name: 'Code Reviewer' | ||
| description: 'Code review specialist for Three Rivers Bank. Reviews PRs for Java/Spring Boot best practices, React patterns, security, testing coverage, and project conventions.' | ||
| model: Claude Sonnet 4 | ||
| tools: ['codebase', 'search', 'changes'] | ||
| --- | ||
|
|
||
| ## Your Mission | ||
|
|
||
| You are a thorough code review specialist for the Three Rivers Bank credit card comparison platform. Your job is to review code changes for quality, security, and adherence to project standards. Provide clear, actionable feedback with severity levels. | ||
|
|
||
| Always respect the `applyTo`-scoped instruction files in `.github/instructions/` β those files define the authoritative rules for each area of the codebase. | ||
|
|
||
| --- | ||
|
|
||
| ## Review Checklist | ||
|
|
||
| ### π΄ Critical β Must Fix Before Merge | ||
|
|
||
| #### Java / Spring Boot | ||
| - [ ] No wildcard imports (`import java.util.*`, etc.) β every import must be explicit | ||
| - [ ] No JPA entities exposed directly in controller responses or request bodies β use DTOs | ||
| - [ ] No hardcoded secrets, credentials, or API keys anywhere in source code | ||
| - [ ] No `System.out.println` β use SLF4J with parameterized messages | ||
| - [ ] No hardcoded BIAN API base URLs β must come from `application.yml` | ||
| - [ ] All BIAN API calls go through the Resilience4j circuit breaker | ||
|
|
||
| #### Security | ||
| - [ ] No SQL injection risks β all queries use parameterized statements or Spring Data JPA | ||
| - [ ] No XSS vulnerabilities in React components | ||
| - [ ] No secrets or credentials embedded in Docker image layers | ||
|
|
||
| --- | ||
|
|
||
| ### π‘ Warning β Should Fix | ||
|
|
||
| #### Java / Spring Boot | ||
| - [ ] Constructor-based dependency injection with `private final` fields | ||
| - [ ] `@Transactional` on service methods that modify data | ||
| - [ ] `@ControllerAdvice` global exception handler returns structured error responses | ||
| - [ ] SLF4J logger declared as `private static final Logger` | ||
| - [ ] Java Bean Validation (`@Valid`, `@NotNull`, `@Size`) used on DTOs | ||
| - [ ] Configuration in `application.yml`; `@ConfigurationProperties` for type-safe binding | ||
|
|
||
| #### React / Frontend | ||
| - [ ] React Query used for server state β no raw `fetch`/`axios` outside query functions | ||
| - [ ] MUI components use the Three Rivers Bank theme from `frontend/src/theme.js` | ||
| - [ ] Functional components with hooks only β no class components | ||
| - [ ] Page components in `pages/`, reusable components in `components/{cards,common,layout}` | ||
|
|
||
| #### Testing | ||
| - [ ] New features or bug fixes include corresponding tests | ||
| - [ ] Test naming follows `methodName_should_expectedBehavior_when_scenario` | ||
| - [ ] Controller tests use `@WebMvcTest`; repository tests use `@DataJpaTest` | ||
| - [ ] Mockito used for unit test mocks; WireMock used for BIAN API integration mocks | ||
| - [ ] No wildcard imports in test files | ||
|
|
||
| #### Docker / Infrastructure | ||
| - [ ] Dockerfiles use multi-stage builds | ||
| - [ ] Base images pinned to explicit versions (not `:latest`) | ||
| - [ ] Containers run as non-root users | ||
| - [ ] `HEALTHCHECK` instructions present | ||
|
|
||
| --- | ||
|
|
||
| ### π’ Suggestion β Nice to Have | ||
|
|
||
| - [ ] `@DisplayName` annotations on JUnit test methods for readability | ||
| - [ ] `assertAll` used to group related assertions | ||
| - [ ] Javadoc on all `public` and `protected` members (`@param`, `@return`, `@throws`) | ||
| - [ ] `@param` and `@return` JSDoc on React component props | ||
| - [ ] Consistent use of Three Rivers Bank brand colors (`#003366`, `#008080`) | ||
|
|
||
| --- | ||
|
|
||
| ## Output Format | ||
|
|
||
| Structure your review as: | ||
|
|
||
| 1. **Summary** β one paragraph overview of the changes and overall quality | ||
| 2. **Critical Issues** π΄ β list each issue with file, line reference, explanation, and suggested fix | ||
| 3. **Warnings** π‘ β list each issue with file, line reference, and suggested improvement | ||
| 4. **Suggestions** π’ β optional improvements that would enhance quality | ||
| 5. **Verdict** β `β Approved`, `β οΈ Approved with suggestions`, or `β Changes requested` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,26 @@ | ||||||
| name: "Copilot Setup Steps" | ||||||
| on: workflow_dispatch | ||||||
|
|
||||||
| jobs: | ||||||
| copilot-setup: | ||||||
| runs-on: ubuntu-latest | ||||||
| steps: | ||||||
| - uses: actions/checkout@v4 | ||||||
| - name: Set up JDK 17 | ||||||
| uses: actions/setup-java@v4 | ||||||
| with: | ||||||
| java-version: '17' | ||||||
| distribution: 'temurin' | ||||||
| - name: Cache Maven packages | ||||||
| uses: actions/cache@v4 | ||||||
| with: | ||||||
| path: ~/.m2 | ||||||
| key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }} | ||||||
| - name: Build backend | ||||||
| run: cd backend && mvn clean install -DskipTests | ||||||
| - name: Set up Node.js | ||||||
| uses: actions/setup-node@v4 | ||||||
| with: | ||||||
| node-version: '18' | ||||||
|
||||||
| node-version: '18' | |
| node-version: '20' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| --- | ||
| description: 'Standards that Copilot should enforce during pull request code reviews' | ||
| applyTo: '**' | ||
| --- | ||
|
|
||
| # Code Review Standards | ||
|
|
||
| ## Security | ||
|
|
||
| - Flag any hardcoded secrets, passwords, API keys, or tokens β these must come from environment variables or a secrets manager. | ||
| - Check for SQL injection risks; all queries must use parameterized statements or Spring Data JPA. | ||
| - Check for XSS vulnerabilities in React components; sanitize any user-supplied HTML. | ||
|
|
||
| ## Java / Spring Boot | ||
|
|
||
| - **Reject wildcard imports** β every Java file must use explicit, per-class imports. No `import java.util.*` or similar. | ||
| - Verify DTOs are used in all controller request/response bodies β never expose JPA entities directly. | ||
| - Check that all BIAN API calls go through the Resilience4j circuit breaker. | ||
| - Flag any hardcoded BIAN API URLs β base URLs must come from `application.yml` configuration. | ||
| - Flag any `System.out.println` usage β use SLF4J logger with parameterized messages instead. | ||
| - Confirm `@Transactional` is present on service methods that modify data. | ||
|
|
||
| ## React / Frontend | ||
|
|
||
| - Verify React components use React Query for server state β flag raw `fetch` or `axios` calls outside query functions. | ||
| - Check that MUI components use the Three Rivers Bank theme from `frontend/src/theme.js`. | ||
|
|
||
| ## Error Handling & Logging | ||
|
|
||
| - Ensure exceptions are handled explicitly and not silently swallowed. | ||
| - Verify SLF4J is used for logging in all Java classes, not `System.out`. | ||
|
|
||
| ## Testing | ||
|
|
||
| - Verify that new features or bug fixes are accompanied by tests. | ||
| - Check that test naming follows the `methodName_should_expectedBehavior_when_scenario` convention. | ||
| - Confirm controller tests use `@WebMvcTest` and repository tests use `@DataJpaTest`. | ||
|
|
||
| ## Docker / Infrastructure | ||
|
|
||
| - Verify Dockerfiles use multi-stage builds. | ||
| - Check that base images are pinned to explicit versions, not `:latest`. | ||
| - Confirm no secrets or credentials are embedded in Docker images. | ||
| - Verify health check instructions are present. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| --- | ||
| description: 'Dockerfile and Docker best practices' | ||
| applyTo: 'docker/**' | ||
| --- | ||
|
|
||
| # Docker / Dockerfile Standards | ||
|
|
||
| ## Multi-Stage Builds | ||
|
|
||
| - Use multi-stage builds to minimize final image size. | ||
| - Separate the build stage from the runtime stage. | ||
| - Only copy artifacts needed at runtime into the final stage. | ||
|
|
||
| ## Base Image Versions | ||
|
|
||
| - Pin base image versions explicitly β do **not** use `:latest` for production images. | ||
| - Example: `FROM eclipse-temurin:17-jre-jammy` instead of `FROM eclipse-temurin:latest`. | ||
|
|
||
| ## Non-Root Users | ||
|
|
||
| - Run container processes as a non-root user. | ||
| - Create a dedicated application user in the Dockerfile and switch to it with `USER`. | ||
|
|
||
| ## Image Naming | ||
|
|
||
| - Follow the naming convention: `threeriversbank/{backend|frontend}:latest`. | ||
| - Tag release images with a version in addition to `latest`. | ||
|
|
||
| ## Backend (Java) | ||
|
|
||
| - Use a Java 17+ JRE runtime image (not JDK) in the final stage. | ||
| - H2 in-memory database is embedded β no external database container needed for the backend. | ||
|
|
||
| ## Frontend (Nginx) | ||
|
|
||
| - Serve the Vite static build output via Nginx in the final stage. | ||
| - Copy only the `dist/` directory into the Nginx image. | ||
|
|
||
| ## Health Checks | ||
|
|
||
| - Add a `HEALTHCHECK` instruction to every service image. | ||
| - Backend health check should target `/actuator/health`. | ||
| - Frontend health check should verify the Nginx process is responding. | ||
|
|
||
| ## Secrets | ||
|
|
||
| - Never embed secrets, credentials, or API keys in Dockerfile instructions or image layers. | ||
| - Use environment variables (injected at runtime) or a secrets manager for sensitive values. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| --- | ||
| description: 'Documentation and Javadoc standards' | ||
| applyTo: 'backend/src/main/**/*.java' | ||
| --- | ||
|
|
||
| # Java Documentation (Javadoc) Standards | ||
|
|
||
| ## Coverage | ||
|
|
||
| - All `public` and `protected` members (classes, methods, constructors, fields) must have Javadoc comments. | ||
| - Document package-private and private members when they are complex or non-obvious. | ||
|
|
||
| ## Summary Sentence | ||
|
|
||
| - The first sentence of a Javadoc comment is the summary description. | ||
| - It must be concise, describe what the member does, and end with a period. | ||
|
|
||
| ## Standard Tags | ||
|
|
||
| - Use `@param <name>` for every method parameter. Descriptions start with a lowercase letter and do not end with a period. | ||
| - Use `@return` for methods with a non-void return type. | ||
| - Use `@throws` (or `@exception`) for every checked exception a method declares, and for significant unchecked exceptions. | ||
| - Use `@see` to reference related types or members. | ||
| - Use `@since` to indicate the version when a feature was introduced. | ||
|
|
||
| ## Inline Tags | ||
|
|
||
| - Use `{@code SomeType}` for inline references to code elements (types, methods, variables). | ||
| - Use `<pre>{@code ... }</pre>` for multi-line code examples. | ||
|
|
||
| ## Inheritance | ||
|
|
||
| - Use `{@inheritDoc}` to inherit documentation from a superclass or interface. | ||
| - If the subclass overrides behavior significantly, document the differences rather than using `{@inheritDoc}`. | ||
|
|
||
| ## Generic Types | ||
|
|
||
| - Use `@param <T>` to document type parameters on generic classes or methods. | ||
|
|
||
| ## Deprecation | ||
|
|
||
| - Use `@deprecated` to mark members that should no longer be used. | ||
| - Always include an explanation and reference to the preferred alternative in the `@deprecated` description. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| --- | ||
| description: 'JUnit 5 testing standards for the Spring Boot backend' | ||
| applyTo: 'backend/src/test/**/*.java' | ||
| --- | ||
|
|
||
| # JUnit 5 Testing Standards | ||
|
|
||
| ## Imports | ||
|
|
||
| - **NEVER use wildcard imports** in test files. Use explicit imports for every class. | ||
|
|
||
| ## Test Structure | ||
|
|
||
| - Follow the **Arrange-Act-Assert (AAA)** pattern in every test method. | ||
| - Test class naming: `{Class}Test.java` (e.g., `CreditCardServiceTest.java`). | ||
| - Test method naming: `methodName_should_expectedBehavior_when_scenario`. | ||
| - Use `@DisplayName` annotations for human-readable test descriptions. | ||
|
|
||
| ## Test Slices | ||
|
|
||
| - Use `@WebMvcTest` for controller tests β loads only the web layer. | ||
| - Use `@DataJpaTest` for repository tests β loads only the JPA layer. | ||
| - Use `@SpringBootTest` only for full integration tests that need the entire context. | ||
|
|
||
| ## Mocking | ||
|
|
||
| - Use Mockito for mocking dependencies: `@Mock`, `@InjectMocks`, `@MockBean`. | ||
| - Use WireMock to mock external BIAN API calls in integration tests. | ||
|
|
||
| ## Assertions | ||
|
|
||
| - Keep tests focused on a single behavior per method when practical. | ||
| - Use `assertAll` to group related assertions so all failures are reported together. | ||
| - Use descriptive failure messages in assertions. | ||
| - Use `assertThrows` to verify exception behavior. | ||
|
|
||
| ## Test Organization | ||
|
|
||
| - Place test files under `backend/src/test/java` mirroring the main source structure. | ||
| - Use `@Nested` inner classes to group related test scenarios. | ||
| - Use `@Tag` to categorize tests (e.g., `@Tag("unit")`, `@Tag("integration")`). | ||
| - Use `@Disabled` with an explanatory reason when temporarily skipping tests. | ||
|
|
||
| ## Data-Driven Tests | ||
|
|
||
| - Use `@ParameterizedTest` with `@ValueSource`, `@CsvSource`, or `@MethodSource` for data-driven scenarios. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| --- | ||
| description: 'Java coding standards for the Three Rivers Bank Spring Boot backend' | ||
| applyTo: 'backend/**/*.java' | ||
| --- | ||
|
|
||
| # Java / Spring Boot Coding Standards | ||
|
|
||
| ## Imports | ||
|
|
||
| - **NEVER use wildcard imports** (e.g., `import java.util.*`). Always use explicit, per-class imports. This is a hard enterprise requirement. | ||
|
|
||
| ## Dependency Injection | ||
|
|
||
| - Use constructor-based dependency injection for all required dependencies. | ||
| - Declare injected fields as `private final`. | ||
| - Do not use field injection (`@Autowired` on fields). | ||
|
|
||
| ## Component Stereotypes | ||
|
|
||
| - Use `@Service` for business-logic classes. | ||
| - Use `@Repository` for data-access classes. | ||
| - Use `@RestController` for REST endpoint classes. | ||
|
|
||
| ## DTOs | ||
|
|
||
| - Never expose JPA entities directly in controller responses or request bodies. | ||
| - Use DTOs (`model/dto`) for all API inputs and outputs. | ||
| - Map between entities and DTOs in the service layer. | ||
|
|
||
| ## Service Layer | ||
|
|
||
| - Encapsulate all business logic in `@Service` classes; keep controllers thin. | ||
| - Annotate service methods that modify data with `@Transactional`. | ||
| - Services must be stateless. | ||
|
|
||
| ## Logging | ||
|
|
||
| - Use SLF4J for all logging: `private static final Logger logger = LoggerFactory.getLogger(MyClass.class);` | ||
| - Use parameterized messages: `logger.info("Processing card {}", cardId);` β never string concatenation. | ||
| - Do **not** use `System.out.println`. | ||
|
|
||
| ## Configuration | ||
|
|
||
| - Store all configuration in `application.yml`. | ||
| - Use `@ConfigurationProperties` for type-safe binding of configuration properties. | ||
| - Do not hardcode URLs, credentials, or environment-specific values in source code. | ||
|
|
||
| ## Validation | ||
|
|
||
| - Apply Java Bean Validation annotations (`@Valid`, `@NotNull`, `@Size`, etc.) on DTO classes. | ||
| - Trigger validation on controller method parameters with `@Valid`. | ||
|
|
||
| ## Error Handling | ||
|
|
||
| - Implement a single global exception handler using `@ControllerAdvice` and `@ExceptionHandler`. | ||
| - Return consistent, structured error responses from the global handler. | ||
|
|
||
| ## External API Calls (BIAN) | ||
|
|
||
| - All calls to the BIAN API must go through the Resilience4j circuit breaker. | ||
| - Never call the BIAN API directly from a controller or repository. | ||
| - BIAN API base URLs must come from configuration, never hardcoded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| --- | ||
| description: 'React coding standards for the Three Rivers Bank frontend' | ||
| applyTo: 'frontend/src/**/*.{jsx,js}' | ||
| --- | ||
|
|
||
| # React / Frontend Coding Standards | ||
|
|
||
| ## State Management | ||
|
|
||
| - Use **React Query (TanStack Query)** for all server state β do NOT use Redux or raw `fetch`/`axios` calls outside of query functions. | ||
| - Use React built-in hooks (`useState`, `useReducer`) for local UI state only. | ||
|
|
||
| ## UI Components | ||
|
|
||
| - Use **Material-UI (MUI)** components for all UI elements. | ||
| - Import and apply the custom Three Rivers Bank theme from `frontend/src/theme.js`. | ||
| - Use the bank's brand colors: Navy `#003366` (primary) and Teal `#008080` (secondary). | ||
|
|
||
| ## Routing | ||
|
|
||
| - Use **React Router v6** for all client-side navigation. | ||
| - Use `<Link>` and `useNavigate` for programmatic navigation. | ||
|
|
||
| ## Component Style | ||
|
|
||
| - Use **functional components with hooks** exclusively β no class components. | ||
| - Page-level components go in `frontend/src/pages/`. | ||
| - Reusable components go in `frontend/src/components/{cards,common,layout}`. | ||
|
|
||
| ## Props Documentation | ||
|
|
||
| - Document component props with JSDoc `@param` comments or PropTypes declarations. | ||
|
|
||
| ## Theme Usage | ||
|
|
||
| - Always import the theme from `frontend/src/theme.js` when referencing design tokens. | ||
| - Do not hardcode color values that are already defined in the theme. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.github/copilot-setup-steps.ymlis written as a GitHub Actions workflow (on: workflow_dispatch,jobs:), but itβs not located under.github/workflows/, so GitHub will not load or allow dispatching it. Either move it to.github/workflows/(and keep the workflow syntax), or (if this file is intended for Copilot coding-agent setup steps) switch it to the Copilot setup-steps file format and remove the Actions-specificon/jobskeys.