fix(ci): fix mvnw permission denied and next lint workspace flag#19
fix(ci): fix mvnw permission denied and next lint workspace flag#19DongDuong2001 merged 16 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCI workflows switched to running npm and mvn directly (no workspace/wrapper flags); frontend adds an ESLint ESM config and a small cleanup; governance-api adds jackson and H2 deps, introduces a TestSecurityConfig that provides a stub JwtDecoder, adds test application.yml, and tightens test annotations to use the test profile with no web environment. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
74-77: Correct fix — consider also persisting the executable bit in git to avoid the need for this step.The
chmod +xstep is the right workaround and is correctly sequenced before the Maven test run. However, the underlying issue is thatmvnwwas not committed with the executable bit set in git.Git tracks the executable bit for regular files, meaning you can commit that change so it is preserved across clones, checkouts, and pulls — no manual
chmod +xneeded. Run the following once and commit:git update-index --chmod=+x services/governance-api/mvnw git commit -m "chore: mark mvnw as executable"After that commit lands on
main, thechmod +xCI step becomes redundant. You can keep it as a defensive guard, but removing it would simplify the workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 74 - 77, The mvnw wrapper in the governance-api service was committed without the executable bit, causing the CI to run a chmod +x step; fix this by setting and committing the executable bit for the mvnw file in the repository (use git update-index --chmod=+x for services/governance-api/mvnw and commit that change) so future clones preserve the executable flag and the CI chmod step becomes unnecessary (you may keep the CI step as a defensive guard if desired).
46-46: LGTM —-wis the correct shorthand; consider aligning with line 49 for consistency.In npm,
-wis the documented shorthand for--workspace, so this change is functionally equivalent to the previous--workspace=apps/webform and the lint invocation will target the correct workspace.The adjacent build step on Line 49 still uses
--workspace=apps/web(long form). Keeping both flags in the same style would improve readability.♻️ Optional: align Line 49 to the same short form
- run: npm run build --workspace=apps/web + run: npm run build -w apps/web🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 46, Update the other npm workspace invocation to match the short flag style: locate the run step that currently uses "--workspace=apps/web" (the build step adjacent to the lint step) and replace the long flag with the short shorthand "-w apps/web" so both npm commands use the same flag style for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 74-77: The mvnw wrapper in the governance-api service was
committed without the executable bit, causing the CI to run a chmod +x step; fix
this by setting and committing the executable bit for the mvnw file in the
repository (use git update-index --chmod=+x for services/governance-api/mvnw and
commit that change) so future clones preserve the executable flag and the CI
chmod step becomes unnecessary (you may keep the CI step as a defensive guard if
desired).
- Line 46: Update the other npm workspace invocation to match the short flag
style: locate the run step that currently uses "--workspace=apps/web" (the build
step adjacent to the lint step) and replace the long flag with the short
shorthand "-w apps/web" so both npm commands use the same flag style for
consistency.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
74-77: Correct fix — consider also committing the executable bit to git.The
chmod +xstep unblocks CI, but the root cause is that git doesn't track the executable bit onmvnw. Runningchmodon every CI run is a recurring workaround rather than a permanent fix. Committing the bit once via:git update-index --chmod=+x services/governance-api/mvnw git commit -m "chore: set mvnw executable bit"would eliminate this step entirely from the workflow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 74 - 77, The CI step named "Make mvnw executable" is a workaround; permanently set the executable bit on services/governance-api/mvnw in the repository and remove that chmod step from the workflow. On your local checkout, use Git to mark services/governance-api/mvnw as executable (using the git index chmod operation) and commit that change with a clear message, then update the .github/workflows/ci.yml by deleting the "Make mvnw executable" step so CI no longer needs to chmod the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 74-77: The CI step named "Make mvnw executable" is a workaround;
permanently set the executable bit on services/governance-api/mvnw in the
repository and remove that chmod step from the workflow. On your local checkout,
use Git to mark services/governance-api/mvnw as executable (using the git index
chmod operation) and commit that change with a clear message, then update the
.github/workflows/ci.yml by deleting the "Make mvnw executable" step so CI no
longer needs to chmod the file.
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
- Add WebEnvironment.NONE and @activeprofiles('test') to GovernanceApiApplicationTests to prevent ReactorResourceFactory from failing when loading the full application context in CI - Change ESLint step to run from repo root with --workspace=apps/web instead of working-directory to fix 'invalid project directory' error caused by npm appending '/lint' to the path
- ci.yml: simplify lint step to pm run lint from root; root package.json already delegates to apps/web workspace, so passing --workspace flag was forwarding it as a directory arg to next lint - Add TestSecurityConfig with a stub @primary JwtDecoder so SecurityConfig's .oauth2ResourceServer() does not attempt to fetch the OIDC discovery document from the issuer URI at startup in CI - Import TestSecurityConfig in GovernanceApiApplicationTests - Update test application.yml: replace autoconfigure exclusion with a localhost stub issuer-uri, and add nexusguard.service-token and nexusguard.cors.allowed-origins stubs for @value injection
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/eslint.config.mjs (1)
8-16: Consider migrating to the native flat config approach.The current
FlatCompatbridge works, but the current Next.js docs now recommend usingdefineConfigwith a direct import ofeslint-config-next/core-web-vitals, which is native flat config and doesn't require the@eslint/eslintrccompatibility shim.eslint-config-next/typescriptfor TypeScript-specific rules can be used alongside it.♻️ Proposed native flat config equivalent
-import { dirname } from "path"; -import { fileURLToPath } from "url"; -import { FlatCompat } from "@eslint/eslintrc"; - -const __filename = fileURLToPath(import.meta.url); -const __dirname = dirname(__filename); - -const compat = new FlatCompat({ - baseDirectory: __dirname, -}); - -const eslintConfig = [ - ...compat.extends("next/core-web-vitals", "next/typescript"), -]; - -export default eslintConfig; +import { defineConfig } from "eslint/config"; +import nextVitals from "eslint-config-next/core-web-vitals"; +import nextTypescript from "eslint-config-next/typescript"; + +export default defineConfig([ + ...nextVitals, + ...nextTypescript, +]);This removes the
@eslint/eslintrcdependency and the__filename/__dirnameshim entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/eslint.config.mjs` around lines 8 - 16, Replace the compatibility shim using FlatCompat and compat.extends by switching to the native flat config: import defineConfig and the Next core flat configs (e.g., "eslint-config-next/core-web-vitals" and "eslint-config-next/typescript") and export a defineConfig(...) value instead of the eslintConfig array; update the symbols in this file (remove FlatCompat, compat, and eslintConfig) and use defineConfig to compose the Next configs directly so you no longer depend on `@eslint/eslintrc` or __dirname/__filename shims.services/governance-api/src/test/resources/application.yml (1)
15-21: Redundant dialect configuration — one of the two entries can be removed.
spring.jpa.database-platformandspring.jpa.properties.hibernate.dialectboth resolve to the same underlying Hibernate setting. Furthermore, Spring Boot auto-detects the H2 dialect when it sees ajdbc:h2:memURL, so both entries are technically unnecessary here.♻️ Proposed cleanup
jpa: hibernate: ddl-auto: create-drop - database-platform: org.hibernate.dialect.H2Dialect - properties: - hibernate: - dialect: org.hibernate.dialect.H2Dialect🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/governance-api/src/test/resources/application.yml` around lines 15 - 21, Remove the redundant Hibernate dialect configuration in the YAML by keeping only one of the two keys: either spring.jpa.database-platform or spring.jpa.properties.hibernate.dialect; preferred is to remove spring.jpa.properties.hibernate.dialect (or remove spring.jpa.database-platform) so only a single dialect setting remains alongside jpa.hibernate.ddl-auto, and rely on Spring Boot H2 autodetection when using an in-memory jdbc:h2:mem URL.services/governance-api/src/test/java/com/nexusguard/config/TestSecurityConfig.java (1)
20-35: LGTM —@PrimarystubJwtDecodercorrectly suppresses OIDC discovery.The
@TestConfiguration+@Primarypattern is correct: since this bean is registered via@Importbefore auto-configurations run,JwtDecoderAutoConfiguration's@ConditionalOnMissingBean(JwtDecoder.class)will back off without attempting to fetch the OIDC discovery document from the stubissuer-uri.Optional nit: the
issclaim can be set more idiomatically using the builder's.claim("iss", ...)instead of.claims(claims -> claims.putAll(Map.of(...))).✨ Optional readability tweak
- return token -> Jwt.withTokenValue(token) - .header("alg", "none") - .claim("sub", "test-user") - .issuedAt(Instant.now()) - .expiresAt(Instant.now().plusSeconds(3600)) - .claims(claims -> claims.putAll(Map.of("iss", "https://auth.nexusguard.io"))) - .build(); + return token -> Jwt.withTokenValue(token) + .header("alg", "none") + .claim("sub", "test-user") + .claim("iss", "https://auth.nexusguard.io") + .issuedAt(Instant.now()) + .expiresAt(Instant.now().plusSeconds(3600)) + .build();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/governance-api/src/test/java/com/nexusguard/config/TestSecurityConfig.java` around lines 20 - 35, Update the Jwt construction in TestSecurityConfig.jwtDecoder() to set the issuer claim using the builder's .claim("iss", "https://auth.nexusguard.io") instead of mutating the claims map via .claims(claims -> claims.putAll(Map.of("iss", "..."))); locate the JwtDecoder bean method named jwtDecoder in class TestSecurityConfig and replace the .claims(...) call with a direct .claim(...) call to make the intent more idiomatic and readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/eslint.config.mjs`:
- Around line 1-3: The FlatCompat import from `@eslint/eslintrc` is relied on but
that package is only a transitive dependency; add "@eslint/eslintrc" to apps/web
package.json devDependencies with a version compatible with your
eslint-config-next (or pin a recent stable release), run your package manager to
install, and commit the updated package.json and lockfile so FlatCompat import
remains stable.
In `@services/governance-api/src/test/resources/application.yml`:
- Around line 7-34: Spring Boot is auto-running SQL scripts (including
services/governance-api/src/main/resources/schema.sql) against the H2 test
datasource which fails due to PostgreSQL-specific DDL; fix by adding the
property spring.sql.init.mode: never to the test application.yml (inside the
existing spring: block) so the SQL initializer is disabled for tests and the
PostgreSQL schema.sql is not executed during ApplicationContext startup.
---
Nitpick comments:
In `@apps/web/eslint.config.mjs`:
- Around line 8-16: Replace the compatibility shim using FlatCompat and
compat.extends by switching to the native flat config: import defineConfig and
the Next core flat configs (e.g., "eslint-config-next/core-web-vitals" and
"eslint-config-next/typescript") and export a defineConfig(...) value instead of
the eslintConfig array; update the symbols in this file (remove FlatCompat,
compat, and eslintConfig) and use defineConfig to compose the Next configs
directly so you no longer depend on `@eslint/eslintrc` or __dirname/__filename
shims.
In
`@services/governance-api/src/test/java/com/nexusguard/config/TestSecurityConfig.java`:
- Around line 20-35: Update the Jwt construction in
TestSecurityConfig.jwtDecoder() to set the issuer claim using the builder's
.claim("iss", "https://auth.nexusguard.io") instead of mutating the claims map
via .claims(claims -> claims.putAll(Map.of("iss", "..."))); locate the
JwtDecoder bean method named jwtDecoder in class TestSecurityConfig and replace
the .claims(...) call with a direct .claim(...) call to make the intent more
idiomatic and readable.
In `@services/governance-api/src/test/resources/application.yml`:
- Around line 15-21: Remove the redundant Hibernate dialect configuration in the
YAML by keeping only one of the two keys: either spring.jpa.database-platform or
spring.jpa.properties.hibernate.dialect; preferred is to remove
spring.jpa.properties.hibernate.dialect (or remove spring.jpa.database-platform)
so only a single dialect setting remains alongside jpa.hibernate.ddl-auto, and
rely on Spring Boot H2 autodetection when using an in-memory jdbc:h2:mem URL.
| import { dirname } from "path"; | ||
| import { fileURLToPath } from "url"; | ||
| import { FlatCompat } from "@eslint/eslintrc"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat apps/web/package.json | jq '.devDependencies | has("@eslint/eslintrc")'Repository: lab68dev/nexus-guard
Length of output: 69
🏁 Script executed:
cat apps/web/package.json | jq '.dependencies, .devDependencies, .peerDependencies'Repository: lab68dev/nexus-guard
Length of output: 488
Add @eslint/eslintrc as an explicit devDependency.
FlatCompat is imported directly from @eslint/eslintrc (line 3), but @eslint/eslintrc is not explicitly listed in devDependencies. It is only pulled in transitively via eslint-config-next. A future dependency update could remove this transitive dependency and break this import. Add @eslint/eslintrc as an explicit devDependency in apps/web/package.json.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/eslint.config.mjs` around lines 1 - 3, The FlatCompat import from
`@eslint/eslintrc` is relied on but that package is only a transitive dependency;
add "@eslint/eslintrc" to apps/web package.json devDependencies with a version
compatible with your eslint-config-next (or pin a recent stable release), run
your package manager to install, and commit the updated package.json and
lockfile so FlatCompat import remains stable.
| spring: | ||
| # ── In-memory H2 (replaces Neon PostgreSQL in CI) ── | ||
| datasource: | ||
| url: jdbc:h2:mem:nexusguard_test;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE | ||
| username: sa | ||
| password: | ||
| driver-class-name: org.h2.Driver | ||
|
|
||
| jpa: | ||
| hibernate: | ||
| ddl-auto: create-drop | ||
| database-platform: org.hibernate.dialect.H2Dialect | ||
| properties: | ||
| hibernate: | ||
| dialect: org.hibernate.dialect.H2Dialect | ||
|
|
||
| # ── Stub JWT issuer URI (prevents network call to real auth server) ── | ||
| security: | ||
| oauth2: | ||
| resourceserver: | ||
| jwt: | ||
| issuer-uri: http://localhost:9999/auth | ||
|
|
||
| # ── Disable Flyway / Liquibase if present ── | ||
| flyway: | ||
| enabled: false | ||
| liquibase: | ||
| enabled: false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for Spring Boot SQL init scripts and Hibernate import scripts in main resources
echo "=== Spring SQL init scripts (schema.sql / data.sql) ==="
fd -e sql . services/governance-api/src/main/resources --exec echo {} 2>/dev/null || echo "No SQL files found or directory doesn't exist"
echo ""
echo "=== Hibernate import.sql ==="
fd -g "import.sql" services/governance-api/src/main/resources --exec echo {} 2>/dev/null || echo "No import.sql found"
echo ""
echo "=== Any `@Sql` annotations referencing SQL files ==="
rg --type java -n '@Sql' services/governance-api/src/ 2>/dev/null || echo "No `@Sql` annotations found"
echo ""
echo "=== Check if main resources directory exists ==="
ls -la services/governance-api/src/main/resources/ 2>/dev/null || echo "Directory not found"
echo ""
echo "=== List all files in main resources ==="
find services/governance-api/src/main/resources/ -type f 2>/dev/null | head -20 || echo "Unable to list files"Repository: lab68dev/nexus-guard
Length of output: 853
🏁 Script executed:
cat -n services/governance-api/src/main/resources/schema.sqlRepository: lab68dev/nexus-guard
Length of output: 3920
Add spring.sql.init.mode: never to disable Spring's auto-execution of PostgreSQL schema.
The codebase contains services/governance-api/src/main/resources/schema.sql with PostgreSQL-specific DDL that H2 cannot execute: CREATE EXTENSION "pgcrypto", gen_random_uuid(), JSONB data type, TEXT[] arrays, and ALTER TABLE ENABLE ROW LEVEL SECURITY. Spring Boot auto-detects embedded databases and runs these scripts by default, causing the ApplicationContext load failure.
Add the following to the test configuration:
🐛 Proposed fix
# ── Disable Flyway / Liquibase if present ──
flyway:
enabled: false
liquibase:
enabled: false
+
+ # ── Disable Spring's script-based SQL init (schema.sql / data.sql) ──
+ sql:
+ init:
+ mode: never📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spring: | |
| # ── In-memory H2 (replaces Neon PostgreSQL in CI) ── | |
| datasource: | |
| url: jdbc:h2:mem:nexusguard_test;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE | |
| username: sa | |
| password: | |
| driver-class-name: org.h2.Driver | |
| jpa: | |
| hibernate: | |
| ddl-auto: create-drop | |
| database-platform: org.hibernate.dialect.H2Dialect | |
| properties: | |
| hibernate: | |
| dialect: org.hibernate.dialect.H2Dialect | |
| # ── Stub JWT issuer URI (prevents network call to real auth server) ── | |
| security: | |
| oauth2: | |
| resourceserver: | |
| jwt: | |
| issuer-uri: http://localhost:9999/auth | |
| # ── Disable Flyway / Liquibase if present ── | |
| flyway: | |
| enabled: false | |
| liquibase: | |
| enabled: false | |
| spring: | |
| # ── In-memory H2 (replaces Neon PostgreSQL in CI) ── | |
| datasource: | |
| url: jdbc:h2:mem:nexusguard_test;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE | |
| username: sa | |
| password: | |
| driver-class-name: org.h2.Driver | |
| jpa: | |
| hibernate: | |
| ddl-auto: create-drop | |
| database-platform: org.hibernate.dialect.H2Dialect | |
| properties: | |
| hibernate: | |
| dialect: org.hibernate.dialect.H2Dialect | |
| # ── Stub JWT issuer URI (prevents network call to real auth server) ── | |
| security: | |
| oauth2: | |
| resourceserver: | |
| jwt: | |
| issuer-uri: http://localhost:9999/auth | |
| # ── Disable Flyway / Liquibase if present ── | |
| flyway: | |
| enabled: false | |
| liquibase: | |
| enabled: false | |
| # ── Disable Spring's script-based SQL init (schema.sql / data.sql) ── | |
| sql: | |
| init: | |
| mode: never |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/governance-api/src/test/resources/application.yml` around lines 7 -
34, Spring Boot is auto-running SQL scripts (including
services/governance-api/src/main/resources/schema.sql) against the H2 test
datasource which fails due to PostgreSQL-specific DDL; fix by adding the
property spring.sql.init.mode: never to the test application.yml (inside the
existing spring: block) so the SQL initializer is disabled for tests and the
PostgreSQL schema.sql is not executed during ApplicationContext startup.
… function or class (#20) Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/dashboard/policies/page.tsx (1)
123-128:⚠️ Potential issue | 🟡 MinorIcon-only buttons lack accessible names.
Both the edit and delete buttons contain only icon children with no
aria-label,title, or visible text. Screen readers will announce these as generic unlabeled buttons, blocking task completion for assistive-technology users.♿ Proposed fix
-<button className="p-2 border-2 border-ng-black hover:bg-ng-black hover:text-ng-white transition-colors"> +<button aria-label={`Edit ${policy.name}`} className="p-2 border-2 border-ng-black hover:bg-ng-black hover:text-ng-white transition-colors"> <Pencil className="w-4 h-4" strokeWidth={2} /> </button> -<button className="p-2 border-2 border-ng-black hover:bg-ng-danger hover:text-ng-white hover:border-ng-danger transition-colors"> +<button aria-label={`Delete ${policy.name}`} className="p-2 border-2 border-ng-black hover:bg-ng-danger hover:text-ng-white hover:border-ng-danger transition-colors"> <Trash2 className="w-4 h-4" strokeWidth={2} /> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/dashboard/policies/page.tsx` around lines 123 - 128, The icon-only edit and delete buttons that render <Pencil> and <Trash2> lack accessible names; update the <button> elements that wrap the Pencil and Trash2 icons in page.tsx to include explicit accessible labels (e.g., add aria-label="Edit policy" and aria-label="Delete policy") or add a title and/or visually-hidden text inside the button for screen readers; ensure the labels are descriptive and match the action/context so assistive tech will announce the buttons correctly.
🧹 Nitpick comments (2)
apps/web/src/app/dashboard/policies/page.tsx (2)
135-135:String.replacewith a string literal only replaces the first underscore.
policy.type.replace("_", " ")leaves subsequent underscores intact. The currentPolicyTypevalues each have exactly one underscore, so this is harmless today, but will silently misbehave if multi-underscore types are ever added.♻️ Proposed fix
-{policy.type.replace("_", " ")} +{policy.type.replaceAll("_", " ")}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/dashboard/policies/page.tsx` at line 135, The rendering uses policy.type.replace("_", " ") which only replaces the first underscore; update the transformation in apps/web/src/app/dashboard/policies/page.tsx to replace all underscores (e.g., use policy.type.replaceAll("_", " ") or policy.type.replace(/_/g, " ")) so multi-underscore PolicyType values are correctly displayed; locate the occurrence where policy.type is rendered and replace the single-replacement call accordingly.
83-85:typeparameter is unused —typeBadgeis effectively a constant.The function always returns
"ng-badge bg-ng-gray-100"regardless oftype. With the neweslint.config.mjsbeing introduced in this PR,@typescript-eslint/no-unused-varsmay flag the parameter.♻️ Proposed cleanup
-function typeBadge(type: PolicyType) { +function typeBadge(_type: PolicyType) { return "ng-badge bg-ng-gray-100"; }Or, if per-type styling is intended later, leave the signature but add a comment; if it's truly always the same class, drop the parameter entirely and update the two call-sites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/dashboard/policies/page.tsx` around lines 83 - 85, typeBadge currently ignores its PolicyType parameter and returns a constant string; remove the unused parameter to avoid linter errors by changing the function signature to typeBadge() and updating its call-sites (the two places that pass a PolicyType) to call typeBadge() with no arguments, or alternatively keep the signature and rename the parameter to _type or add a short comment to indicate it's intentionally unused if you expect per-type styling later; ensure references to typeBadge and the PolicyType parameter are consistent across both call-sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/app/dashboard/policies/page.tsx`:
- Around line 123-128: The icon-only edit and delete buttons that render
<Pencil> and <Trash2> lack accessible names; update the <button> elements that
wrap the Pencil and Trash2 icons in page.tsx to include explicit accessible
labels (e.g., add aria-label="Edit policy" and aria-label="Delete policy") or
add a title and/or visually-hidden text inside the button for screen readers;
ensure the labels are descriptive and match the action/context so assistive tech
will announce the buttons correctly.
---
Nitpick comments:
In `@apps/web/src/app/dashboard/policies/page.tsx`:
- Line 135: The rendering uses policy.type.replace("_", " ") which only replaces
the first underscore; update the transformation in
apps/web/src/app/dashboard/policies/page.tsx to replace all underscores (e.g.,
use policy.type.replaceAll("_", " ") or policy.type.replace(/_/g, " ")) so
multi-underscore PolicyType values are correctly displayed; locate the
occurrence where policy.type is rendered and replace the single-replacement call
accordingly.
- Around line 83-85: typeBadge currently ignores its PolicyType parameter and
returns a constant string; remove the unused parameter to avoid linter errors by
changing the function signature to typeBadge() and updating its call-sites (the
two places that pass a PolicyType) to call typeBadge() with no arguments, or
alternatively keep the signature and rename the parameter to _type or add a
short comment to indicate it's intentionally unused if you expect per-type
styling later; ensure references to typeBadge and the PolicyType parameter are
consistent across both call-sites.
- policies/page.tsx: remove unused Plus import; prefix unused typeBadge parameter with _ to satisfy no-unused-vars rule - test/application.yml: replace issuer-uri with jwk-set-uri so Spring Security skips OIDC discovery HTTP call during context refresh in CI
…r JwtDecoder - ci.yml: run 'npx next lint' directly from apps/web working-directory instead of via root npm script (which appends workspace path as arg) - package.json: update lint script to use -w shorthand - GovernanceApiApplicationTests: replace @import+@primary with @MockBean JwtDecoder @MockBean always overrides auto-configured beans before context refresh, preventing any JWK/OIDC network calls in CI
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/src/app/dashboard/policies/page.tsx (1)
83-85:_typeprefix is the right ESLint fix; consider implementing per-type badge styles.The underscore convention correctly suppresses the unused-parameter lint error. That said,
typeBadgealways returns the same CSS class regardless of the input, whileseverityBadgeright above it uses a properRecordmap. If type-specific styling is ever desired, the function will need to be revisited.♻️ Suggested per-type badge mapping (mirrors severityBadge pattern)
-function typeBadge(_type: PolicyType) { - return "ng-badge bg-ng-gray-100"; +function typeBadge(type: PolicyType) { + const map: Record<PolicyType, string> = { + CONTENT_FILTER: "ng-badge bg-ng-gray-100", + RATE_LIMIT: "ng-badge bg-ng-gray-100", + TOKEN_BUDGET: "ng-badge bg-ng-gray-100", + PII_REDACTION: "ng-badge bg-ng-gray-100", + MODEL_ALLOWLIST:"ng-badge bg-ng-gray-100", + }; + return map[type]; }This makes the exhaustive mapping explicit and type-safe, so adding a new
PolicyTypevariant will surface a compile-time error here rather than silently falling through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/dashboard/policies/page.tsx` around lines 83 - 85, The typeBadge function currently ignores its PolicyType input and returns a single class; change it to mirror the severityBadge pattern by creating a Record<PolicyType,string> (e.g., typeBadgeMap) that maps every PolicyType variant to a specific CSS class and have typeBadge(type: PolicyType) return typeBadgeMap[type]; ensure the mapping is exhaustive (so adding a new PolicyType triggers a compile error) and replace the unused-parameter underscore with a real parameter name to use for lookup.package.json (1)
11-14: Minor inconsistency:lintuses-wwhiledev/build/startuse--workspace=Both forms are equivalent, but mixing them in the same
scriptsblock reduces readability. Since the CI no longer invokes the rootlintscript directly (it runsnpx next lintfrom the workspace directory), there's no functional impact—just a style nit.✏️ Align to one convention
- "lint": "npm run lint -w apps/web", + "lint": "npm run lint --workspace=apps/web",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 11 - 14, The package.json scripts use mixed workspace flag styles; update the "lint" script to match the other scripts by replacing the short flag "-w" with the long form "--workspace=apps/web" (i.e., change the "lint" script value so it reads like the "dev"/"build"/"start" scripts), ensuring all workspace-invoking scripts use a consistent convention and keep the script name "lint" unchanged..github/workflows/ci.yml (1)
76-78: Considerchmod +x mvnwto preserve the Maven Wrapper's version pin.Switching to system
mvnfixes the permission-denied error but silently gives up the version pinning that.mvn/wrapper/maven-wrapper.propertiesprovides. Ubuntu-latest ships with Maven (currently 3.9.x), so tests will pass today, but a runner image bump could silently change the Maven version and surface compatibility issues.A minimal alternative that keeps the wrapper is:
- run: mvn test --no-transfer-progress + run: | + chmod +x mvnw + ./mvnw test --no-transfer-progressIf the intent is to permanently drop the wrapper in favour of the system Maven, that's a valid choice—just document it (e.g. remove
mvnw/.mvn/from the repo to avoid confusion).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 76 - 78, The workflow currently runs the system Maven ("mvn test") which bypasses the repository Maven Wrapper and its pinned version; restore the wrapper by making the wrapper executable and invoking it: add a step to run "chmod +x mvnw" then run "./mvnw test --no-transfer-progress" (refer to mvnw and .mvn/wrapper/maven-wrapper.properties) so the pinned Maven version is used; alternatively, if you intentionally want to use system Maven, remove mvnw/.mvn from the repo or document the decision to avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/governance-api/src/test/java/com/nexusguard/GovernanceApiApplicationTests.java`:
- Line 13: The Javadoc incorrectly documents the class-level form {`@code`
`@MockBean`(JwtDecoder.class)} while the test uses a field-level mock; update the
comment in GovernanceApiApplicationTests to reference the field-level form (e.g.
`@MockBean JwtDecoder jwtDecoder`) or the new annotation `@MockitoBean` used on
the field, so the Javadoc matches the actual declaration of the jwtDecoder field
and the migrated annotation.
- Around line 5-6: The test uses the removed `@MockBean`; replace its import and
usage with `@MockitoBean` from
org.springframework.test.context.bean.override.mockito: update the import
(remove org.springframework.boot.test.mock.mockito.MockBean and import
org.springframework.test.context.bean.override.mockito.MockitoBean) and change
any annotation instances (e.g., on the JwtDecoder field or other mocked beans in
GovernanceApiApplicationTests) from `@MockBean` to `@MockitoBean` so the test
compiles under Spring Boot 4.x.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 76-78: The workflow currently runs the system Maven ("mvn test")
which bypasses the repository Maven Wrapper and its pinned version; restore the
wrapper by making the wrapper executable and invoking it: add a step to run
"chmod +x mvnw" then run "./mvnw test --no-transfer-progress" (refer to mvnw and
.mvn/wrapper/maven-wrapper.properties) so the pinned Maven version is used;
alternatively, if you intentionally want to use system Maven, remove mvnw/.mvn
from the repo or document the decision to avoid confusion.
In `@apps/web/src/app/dashboard/policies/page.tsx`:
- Around line 83-85: The typeBadge function currently ignores its PolicyType
input and returns a single class; change it to mirror the severityBadge pattern
by creating a Record<PolicyType,string> (e.g., typeBadgeMap) that maps every
PolicyType variant to a specific CSS class and have typeBadge(type: PolicyType)
return typeBadgeMap[type]; ensure the mapping is exhaustive (so adding a new
PolicyType triggers a compile error) and replace the unused-parameter underscore
with a real parameter name to use for lookup.
In `@package.json`:
- Around line 11-14: The package.json scripts use mixed workspace flag styles;
update the "lint" script to match the other scripts by replacing the short flag
"-w" with the long form "--workspace=apps/web" (i.e., change the "lint" script
value so it reads like the "dev"/"build"/"start" scripts), ensuring all
workspace-invoking scripts use a consistent convention and keep the script name
"lint" unchanged.
| import org.springframework.boot.test.mock.mockito.MockBean; | ||
| import org.springframework.security.oauth2.jwt.JwtDecoder; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find services/governance-api -name 'pom.xml' -exec cat {} \;Repository: lab68dev/nexus-guard
Length of output: 4433
🏁 Script executed:
cat -n services/governance-api/src/test/java/com/nexusguard/GovernanceApiApplicationTests.javaRepository: lab68dev/nexus-guard
Length of output: 1114
🌐 Web query:
Spring Boot 4.0 @MockBean removed MockitoBean replacement
💡 Result:
In Spring Boot 4.0, Boot’s @MockBean / @SpyBean support has been removed; migrate to Spring Framework’s Mockito bean-override annotations:
- Replace:
org.springframework.boot.test.mock.mockito.MockBean→org.springframework.test.context.bean.override.mockito.MockitoBeanorg.springframework.boot.test.mock.mockito.SpyBean→org.springframework.test.context.bean.override.mockito.MockitoSpyBean
Spring Boot 4’s migration guide explicitly points to@MockitoBean/@MockitoSpyBeanas the replacements. [1]
Example (field-level, direct rename):
import org.springframework.test.context.bean.override.mockito.MockitoBean;
`@SpringBootTest`
class MyTest {
`@MockitoBean`
GreetingService greetingService;
}[1][2]
Important behavior difference: @MockitoBean / @MockitoSpyBean are supported on test classes/fields, but not in @Configuration / @TestConfiguration classes (a pattern that often worked with @MockBean). Instead, declare shared mocks at the type level (repeatable) and/or via a composed meta-annotation. [1][3]
🏁 Script executed:
find services/governance-api -name 'TestSecurityConfig.java' -o -name '*SecurityConfig*.java' | head -20Repository: lab68dev/nexus-guard
Length of output: 227
🏁 Script executed:
cat -n services/governance-api/src/test/java/com/nexusguard/config/TestSecurityConfig.javaRepository: lab68dev/nexus-guard
Length of output: 1659
🏁 Script executed:
# Check the actual imported package path and verify it's available in the test dependencies
rg "MockitoBean" services/governance-api/ --type java 2>/dev/null || echo "Not yet imported in codebase"Repository: lab68dev/nexus-guard
Length of output: 92
🌐 Web query:
Spring Boot 4.0 spring-boot-starter-test @MockitoBean available
💡 Result:
Yes. In Spring Boot 4.0, Boot’s @MockBean/@SpyBean support is gone; you should use Spring Framework’s @MockitoBean (and @MockitoSpyBean) instead. Spring Boot’s 4.0 Migration Guide shows @MockitoBean as the direct replacement in tests. [1]
Import it from Spring Framework (spring-test), which is brought in by spring-boot-starter-test:
import org.springframework.test.context.bean.override.mockito.MockitoBean;
`@SpringBootTest`
class MyTest {
`@MockitoBean` MyService myService;
}The annotation type is org.springframework.test.context.bean.override.mockito.MockitoBean. [2] (This is also the answer to “where did @MockBean go” in Boot 4.0.0.) [3]
Sources: [1] [2] [3]
Replace @MockBean with @MockitoBean from Spring Framework package
Spring Boot 4.0.2 removed @MockBean from org.springframework.boot.test.mock.mockito. The correct replacement is @MockitoBean from org.springframework.test.context.bean.override.mockito, which is available via spring-boot-starter-test.
Proposed fix
-import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.test.context.bean.override.mockito.MockitoBean;
import org.springframework.security.oauth2.jwt.JwtDecoder;
import org.springframework.test.context.ActiveProfiles;- `@MockBean`
+ `@MockitoBean`
JwtDecoder jwtDecoder;🧰 Tools
🪛 GitHub Actions: CI — Quality Gate
[error] 5-5: package org.springframework.boot.test.mock.mockito does not exist
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/governance-api/src/test/java/com/nexusguard/GovernanceApiApplicationTests.java`
around lines 5 - 6, The test uses the removed `@MockBean`; replace its import and
usage with `@MockitoBean` from
org.springframework.test.context.bean.override.mockito: update the import
(remove org.springframework.boot.test.mock.mockito.MockBean and import
org.springframework.test.context.bean.override.mockito.MockitoBean) and change
any annotation instances (e.g., on the JwtDecoder field or other mocked beans in
GovernanceApiApplicationTests) from `@MockBean` to `@MockitoBean` so the test
compiles under Spring Boot 4.x.
| * Smoke test — verifies the Spring application context loads successfully. | ||
| * | ||
| * <p> | ||
| * {@code @MockBean(JwtDecoder.class)} replaces the auto-configured |
There was a problem hiding this comment.
Javadoc references the type-level annotation form, but the code uses the field-level form
Line 13 documents {@code @MockBean(JwtDecoder.class)} (the class-level form), while the actual declaration is a field-level @MockBean JwtDecoder jwtDecoder. After migrating to @MockitoBean, update the Javadoc accordingly.
📝 Proposed fix
- * {`@code` `@MockBean`(JwtDecoder.class)} replaces the auto-configured
- * {`@link` JwtDecoder} (which would attempt to fetch the JWK set / OIDC
- * discovery document at startup) with a Mockito mock, preventing any
- * outbound network calls during CI.
+ * {`@code` `@MockitoBean`} on the {`@code` jwtDecoder} field replaces the
+ * auto-configured {`@link` JwtDecoder} (which would otherwise attempt to
+ * fetch the JWK set / OIDC discovery document at startup) with a Mockito
+ * mock, preventing outbound network calls during CI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/governance-api/src/test/java/com/nexusguard/GovernanceApiApplicationTests.java`
at line 13, The Javadoc incorrectly documents the class-level form {`@code`
`@MockBean`(JwtDecoder.class)} while the test uses a field-level mock; update the
comment in GovernanceApiApplicationTests to reference the field-level form (e.g.
`@MockBean JwtDecoder jwtDecoder`) or the new annotation `@MockitoBean` used on
the field, so the Javadoc matches the actual declaration of the jwtDecoder field
and the migrated annotation.
Bumps [eslint](https://github.com/eslint/eslint) from 10.0.0 to 10.0.1. - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v10.0.0...v10.0.1) --- updated-dependencies: - dependency-name: eslint dependency-version: 10.0.1 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [lucide-react](https://github.com/lucide-icons/lucide/tree/HEAD/packages/lucide-react) from 0.574.0 to 0.575.0. - [Release notes](https://github.com/lucide-icons/lucide/releases) - [Commits](https://github.com/lucide-icons/lucide/commits/0.575.0/packages/lucide-react) --- updated-dependencies: - dependency-name: lucide-react dependency-version: 0.575.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 25.2.3 to 25.3.0. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-version: 25.3.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps the tailwind-ecosystem group with 2 updates: [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) and [@tailwindcss/postcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-postcss). Updates `tailwindcss` from 4.1.18 to 4.2.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.0/packages/tailwindcss) Updates `@tailwindcss/postcss` from 4.1.18 to 4.2.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.0/packages/@tailwindcss-postcss) --- updated-dependencies: - dependency-name: tailwindcss dependency-version: 4.2.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: tailwind-ecosystem - dependency-name: "@tailwindcss/postcss" dependency-version: 4.2.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: tailwind-ecosystem ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Duong Phu Dong <dongduong840@gmail.com>
) Bumps [lucide-react](https://github.com/lucide-icons/lucide/tree/HEAD/packages/lucide-react) from 0.574.0 to 0.575.0. - [Release notes](https://github.com/lucide-icons/lucide/releases) - [Commits](https://github.com/lucide-icons/lucide/commits/0.575.0/packages/lucide-react) --- updated-dependencies: - dependency-name: lucide-react dependency-version: 0.575.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#21) Bumps [org.springframework.boot:spring-boot-starter-parent](https://github.com/spring-projects/spring-boot) from 4.0.2 to 4.0.3. - [Release notes](https://github.com/spring-projects/spring-boot/releases) - [Commits](spring-projects/spring-boot@v4.0.2...v4.0.3) --- updated-dependencies: - dependency-name: org.springframework.boot:spring-boot-starter-parent dependency-version: 4.0.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
Type of Change
Affected Services
apps/web— Next.js Frontendservices/governance-api— Spring Boot Backendpackages/shared-types— Shared TypeScript TypesTesting
npm run lintandnpm run buildlocally (for web changes)./mvnw testlocally (for API changes)Checklist
Summary by CodeRabbit
Chores
Tests
Dependencies