Merged
Conversation
📝 WalkthroughWalkthroughUpdated environment files and agent config defaults, added new Docker Compose services and container names, tightened agent-provisioning script parameter handling, and renamed Keycloak admin env vars with corresponding changes in Prisma seed and seed data placeholders. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 docstrings
🧪 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 |
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: Your Name <sahil.kamble@ayanworks.com> Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
KambleSahil3
force-pushed
the
fix/agent-startup-script
branch
from
339fb3a to
b8fde39
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose.yml (1)
177-177:⚠️ Potential issue | 🔴 CriticalContainer name mismatch in
agent-servicestartup command.The command references
platform-agent-provisioning-1but theagent-provisioningservice now has an explicitcontainer_name: agent-provisioning-service. This will cause theagent-serviceto wait indefinitely since no container with that name will exist.🐛 Proposed fix
- command: sh -c 'until (docker logs platform-agent-provisioning-1 | grep "Agent-Provisioning-Service Microservice is listening to NATS"); do sleep 1; done && node dist/apps/agent-service/main.js' + command: sh -c 'until (docker logs agent-provisioning-service | grep "Agent-Provisioning-Service Microservice is listening to NATS"); do sleep 1; done && node dist/apps/agent-service/main.js'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` at line 177, The agent-service startup command is watching logs from a non-existent container name; update the command in the agent-service service to reference the actual container name "agent-provisioning-service" (or use the service name) instead of "platform-agent-provisioning-1" so the until loop can find the log line; edit the command string that currently contains docker logs platform-agent-provisioning-1 | grep "Agent-Provisioning-Service Microservice is listening to NATS" to use "agent-provisioning-service" (or replace docker logs usage with docker-compose logs <service> if preferred) so the wait will succeed.
🧹 Nitpick comments (2)
apps/agent-provisioning/AFJ/scripts/start_agent.sh (1)
24-29: Consider validating additional critical parameters.The validation currently only checks
AGENCY,EXTERNAL_IP, andWALLET_NAME. Other parameters likeWALLET_PASSWORD,WEBHOOK_HOST,WALLET_STORAGE_HOST, andAGENT_API_KEYare also used in configuration generation and could cause issues if empty.💡 Suggested improvement
# Validate required parameters -if [ -z "$AGENCY" ] || [ -z "$EXTERNAL_IP" ] || [ -z "$WALLET_NAME" ]; then +if [ -z "$AGENCY" ] || [ -z "$EXTERNAL_IP" ] || [ -z "$WALLET_NAME" ] || [ -z "$WALLET_PASSWORD" ] || [ -z "$WEBHOOK_HOST" ]; then echo "ERROR: Missing required parameters" - echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME ..." + echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME WALLET_PASSWORD RANDOM_SEED WEBHOOK_HOST ..." exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh` around lines 24 - 29, Add checks in the start_agent.sh parameter validation to also ensure WALLETPASSWORD (WALLET_PASSWORD), WEBHOOK_HOST, WALLET_STORAGE_HOST and AGENT_API_KEY are non-empty before proceeding; update the existing if condition that currently tests AGENCY, EXTERNAL_IP and WALLET_NAME to include these additional environment variables (WALLET_PASSWORD, WEBHOOK_HOST, WALLET_STORAGE_HOST, AGENT_API_KEY) and surface a clear error/usage message and exit 1 if any are missing so the configuration generation later in the script does not run with empty critical values.docker-compose.yml (1)
23-38: Consider using environment variables for Postgres credentials.The Postgres service uses hardcoded default credentials (
postgres/postgres). While acceptable for local development, consider referencing environment variables from the.envfile for consistency with other services.💡 Suggested improvement
environment: - - POSTGRES_USER=postgres - - POSTGRES_PASSWORD=postgres - - POSTGRES_DB=credebl + - POSTGRES_USER=${POSTGRES_USER:-postgres} + - POSTGRES_PASSWORD=${POSTGRES_PASSWORD:-postgres} + - POSTGRES_DB=${POSTGRES_DB:-credebl}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 23 - 38, The Postgres service currently hardcodes credentials (service name "postgres" using POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_DB); update the docker-compose postgres environment entries to reference environment variables from the .env file instead of literal values, e.g., read POSTGRES_USER, POSTGRES_PASSWORD and POSTGRES_DB from the environment (and provide sensible defaults in .env or compose env_file) and update project docs or .env.example accordingly so local development still works and credentials remain configurable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent.env`:
- Around line 32-36: The four env vars ROOT_CA_VALIDITY_YEARS,
DCS_VALIDITY_YEARS, ROOT_CA_START_FROM_CURRENT_MONTH, and
DCS_START_FROM_CURRENT_MONTH are declared but unused; either remove them from
agent.env or read and apply them in the certificate logic: add parsing of these
env vars (with defaults) and pass them into the X509 certificate generation flow
(e.g., where CertificateManager/X509Service constructs cert validity — methods
like X509Service.generateCertificate, CertificateManager.createRootCA or
createDeviceCertificate / calculateValidityPeriod), using the *_VALIDITY_YEARS
to set NotAfter and *_START_FROM_CURRENT_MONTH to adjust NotBefore to the start
of the current month before computing expiry; ensure parsing handles
integers/booleans and falls back to existing defaults if unset.
- Around line 40-48: The new TRUST_SERVICE_* env vars are declared but not used;
either remove them or implement ClientAuth: add a TrustServiceClient that reads
TRUST_SERVICE_AUTH_TYPE, TRUST_SERVICE_TOKEN_URL, TRUST_SERVICE_CLIENT_ID and
TRUST_SERVICE_CLIENT_SECRET from env, requests a token from
TRUST_SERVICE_TOKEN_URL (using client_id/secret) when TRUST_SERVICE_AUTH_TYPE ==
"ClientAuth", caches/refreshes the token, and uses it as a Bearer auth header
when fetching the trust list from TRUST_LIST_URL; then update
TrustServiceRoleGuard to call TrustServiceClient to obtain the trust list (or
skip auth for NoAuth) and validate realm roles against the authenticated trust
list, handling errors and falling back clearly.
In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh`:
- Line 9: Remove the unused RANDOM_SEED assignment in start_agent.sh: delete the
line that sets RANDOM_SEED="$5" and any related dead code; update the script's
parameter handling and usage/help text (if any) to reflect one fewer positional
argument so callers aren't expected to pass a fifth parameter. Ensure no other
code references RANDOM_SEED before committing.
In `@docker-compose.yml`:
- Around line 37-38: The docker-compose uses the named volume platform-volume in
the postgres service but never declares it in the top-level volumes section; add
a declaration for platform-volume in the compose file's volumes block (the same
volumes section that should include other entries referenced around lines
251-253) so Docker Compose creates/uses a named persistent volume instead of an
anonymous one—ensure the symbol platform-volume is added to the volumes mapping
(optionally specifying driver or driver_opts if needed).
In
`@libs/prisma-service/prisma/data/credebl-master-table/credebl-master-table.json`:
- Line 16: Replace the instructional placeholder values in the
credebl-master-table.json sensitive fields with empty-string placeholders so
seed/decrypt logic fails fast and reads real secrets from .env; specifically
remove "##Please provide encrypted password using crypto-js##" from the
"password" property and any similar instructional string at the other sensitive
key on line 29 and set them to "" (empty string) so the seeding/decryption code
(which reads "password" and the other sensitive key) will not attempt to decrypt
an instructional text.
---
Outside diff comments:
In `@docker-compose.yml`:
- Line 177: The agent-service startup command is watching logs from a
non-existent container name; update the command in the agent-service service to
reference the actual container name "agent-provisioning-service" (or use the
service name) instead of "platform-agent-provisioning-1" so the until loop can
find the log line; edit the command string that currently contains docker logs
platform-agent-provisioning-1 | grep "Agent-Provisioning-Service Microservice is
listening to NATS" to use "agent-provisioning-service" (or replace docker logs
usage with docker-compose logs <service> if preferred) so the wait will succeed.
---
Nitpick comments:
In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh`:
- Around line 24-29: Add checks in the start_agent.sh parameter validation to
also ensure WALLETPASSWORD (WALLET_PASSWORD), WEBHOOK_HOST, WALLET_STORAGE_HOST
and AGENT_API_KEY are non-empty before proceeding; update the existing if
condition that currently tests AGENCY, EXTERNAL_IP and WALLET_NAME to include
these additional environment variables (WALLET_PASSWORD, WEBHOOK_HOST,
WALLET_STORAGE_HOST, AGENT_API_KEY) and surface a clear error/usage message and
exit 1 if any are missing so the configuration generation later in the script
does not run with empty critical values.
In `@docker-compose.yml`:
- Around line 23-38: The Postgres service currently hardcodes credentials
(service name "postgres" using POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_DB);
update the docker-compose postgres environment entries to reference environment
variables from the .env file instead of literal values, e.g., read
POSTGRES_USER, POSTGRES_PASSWORD and POSTGRES_DB from the environment (and
provide sensible defaults in .env or compose env_file) and update project docs
or .env.example accordingly so local development still works and credentials
remain configurable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01afb55b-071f-4118-94ec-1f11c923bfbc
📒 Files selected for processing (6)
.env.demoagent.envapps/agent-provisioning/AFJ/scripts/start_agent.shdocker-compose.ymllibs/prisma-service/prisma/data/credebl-master-table/credebl-master-table.jsonlibs/prisma-service/prisma/seed.ts
Comment on lines
+32
to
+36
| ROOT_CA_VALIDITY_YEARS=3 | ||
| DCS_VALIDITY_YEARS=3 | ||
|
|
||
| ROOT_CA_START_FROM_CURRENT_MONTH=true | ||
| DCS_START_FROM_CURRENT_MONTH=true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if x509 service reads these environment variables
echo "=== Searching for ROOT_CA_VALIDITY_YEARS usage ==="
rg -n "ROOT_CA_VALIDITY_YEARS" --type=ts
echo "=== Searching for DCS_VALIDITY_YEARS usage ==="
rg -n "DCS_VALIDITY_YEARS" --type=ts
echo "=== Searching for START_FROM_CURRENT_MONTH usage ==="
rg -n "START_FROM_CURRENT_MONTH" --type=tsRepository: credebl/platform
Length of output: 210
Remove or integrate unused certificate validity environment variables into the X509 service.
The environment variables ROOT_CA_VALIDITY_YEARS, DCS_VALIDITY_YEARS, ROOT_CA_START_FROM_CURRENT_MONTH, and DCS_START_FROM_CURRENT_MONTH are defined in agent.env but are not used anywhere in the codebase. Either remove them if they are not needed, or wire them into the X509 certificate service where certificate validity periods are calculated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent.env` around lines 32 - 36, The four env vars ROOT_CA_VALIDITY_YEARS,
DCS_VALIDITY_YEARS, ROOT_CA_START_FROM_CURRENT_MONTH, and
DCS_START_FROM_CURRENT_MONTH are declared but unused; either remove them from
agent.env or read and apply them in the certificate logic: add parsing of these
env vars (with defaults) and pass them into the X509 certificate generation flow
(e.g., where CertificateManager/X509Service constructs cert validity — methods
like X509Service.generateCertificate, CertificateManager.createRootCA or
createDeviceCertificate / calculateValidityPeriod), using the *_VALIDITY_YEARS
to set NotAfter and *_START_FROM_CURRENT_MONTH to adjust NotBefore to the start
of the current month before computing expiry; ensure parsing handles
integers/booleans and falls back to existing defaults if unset.
Comment on lines
40
to
48
| # Authentication type for trust-service calls. Supported: NoAuth | ClientAuth (defaults to NoAuth if not set) | ||
| TRUST_SERVICE_AUTH_TYPE= | ||
| # Full token endpoint URL for ClientAuth (e.g. http://host:5000/v1/orgs/{clientId}/token) | ||
| TRUST_SERVICE_TOKEN_URL= | ||
| # Client credentials used for trust-service authentication (ClientAuth only) | ||
| TRUST_SERVICE_CLIENT_ID= | ||
| TRUST_SERVICE_CLIENT_SECRET= | ||
| # Trust list URL — for NoAuth: GitHub/static JSON URL; for ClientAuth: trust-service base URL | ||
| TRUST_LIST_URL= |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if trust-service auth variables are consumed
echo "=== Searching for TRUST_SERVICE_AUTH_TYPE usage ==="
rg -n "TRUST_SERVICE_AUTH_TYPE" --type=ts
echo "=== Searching for TRUST_SERVICE_TOKEN_URL usage ==="
rg -n "TRUST_SERVICE_TOKEN_URL" --type=ts
echo "=== Searching for trust-service client auth implementation ==="
rg -n "ClientAuth" --type=tsRepository: credebl/platform
Length of output: 1253
🏁 Script executed:
#!/bin/bash
echo "=== Searching for TrustServiceRoleGuard implementation ==="
rg -n "TrustServiceRoleGuard" --type=ts -A 15
echo "=== Searching for trust-service related code ==="
rg -n "trust-service|trust_service|TrustService" --type=ts -i | head -30
echo "=== Searching for TRUST_SERVICE in any file type ==="
rg -n "TRUST_SERVICE" -A 2
echo "=== Searching for ClientAuth enum/type definitions ==="
rg -n "TRUST_SERVICE_AUTH_TYPE\|NoAuth.*ClientAuth" --type=tsRepository: credebl/platform
Length of output: 6946
🏁 Script executed:
#!/bin/bash
echo "=== Searching for ConfigService usage with TRUST_SERVICE ==="
rg -n "ConfigService|get\(" libs apps --type=ts -i | grep -i trust | head -20
echo "=== Checking if there's a trust-service client or service using these env vars ==="
find . -type f -name "*.ts" | xargs grep -l "TRUST_SERVICE\|trust.*service.*auth" 2>/dev/null | grep -v node_modules | head -20
echo "=== Checking agent.env file for where these are supposed to be used ==="
cat agent.env | grep -A 2 -B 2 "TRUST_SERVICE"
echo "=== Searching for process.env or environment variable access patterns ==="
rg -n "process\.env\.TRUST" --type=tsRepository: credebl/platform
Length of output: 889
Trust-service authentication variables are declared but not implemented.
The environment variables TRUST_SERVICE_AUTH_TYPE, TRUST_SERVICE_TOKEN_URL, TRUST_SERVICE_CLIENT_ID, and TRUST_SERVICE_CLIENT_SECRET are added to agent.env with comments describing a ClientAuth flow, but these variables are never consumed anywhere in the codebase. The TrustServiceRoleGuard only validates realm roles and does not implement the ClientAuth mechanism described in the comments. Either remove these unused variables or implement the authentication logic that the comments describe.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent.env` around lines 40 - 48, The new TRUST_SERVICE_* env vars are
declared but not used; either remove them or implement ClientAuth: add a
TrustServiceClient that reads TRUST_SERVICE_AUTH_TYPE, TRUST_SERVICE_TOKEN_URL,
TRUST_SERVICE_CLIENT_ID and TRUST_SERVICE_CLIENT_SECRET from env, requests a
token from TRUST_SERVICE_TOKEN_URL (using client_id/secret) when
TRUST_SERVICE_AUTH_TYPE == "ClientAuth", caches/refreshes the token, and uses it
as a Bearer auth header when fetching the trust list from TRUST_LIST_URL; then
update TrustServiceRoleGuard to call TrustServiceClient to obtain the trust list
(or skip auth for NoAuth) and validate realm roles against the authenticated
trust list, handling errors and falling back clearly.
| EXTERNAL_IP="$2" | ||
| WALLET_NAME="$3" | ||
| WALLET_PASSWORD="$4" | ||
| RANDOM_SEED="$5" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if RANDOM_SEED is used anywhere in the codebase
echo "=== Searching for RANDOM_SEED usage in shell scripts ==="
rg -n "RANDOM_SEED" --type=sh
echo "=== Searching for RANDOM_SEED in the agent provisioning directory ==="
rg -n "RANDOM_SEED" apps/agent-provisioning/Repository: credebl/platform
Length of output: 3354
🏁 Script executed:
cat -n apps/agent-provisioning/AFJ/scripts/start_agent.shRepository: credebl/platform
Length of output: 9472
Remove unused RANDOM_SEED variable assignment.
RANDOM_SEED is assigned from $5 on line 9 but is not used anywhere in this script or in the generated configuration files. Unlike other parameters that are passed to the script, this variable has no functional purpose and should be removed to improve code clarity.
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 9-9: RANDOM_SEED appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh` at line 9, Remove the
unused RANDOM_SEED assignment in start_agent.sh: delete the line that sets
RANDOM_SEED="$5" and any related dead code; update the script's parameter
handling and usage/help text (if any) to reflect one fewer positional argument
so callers aren't expected to pass a fifth parameter. Ensure no other code
references RANDOM_SEED before committing.
Comment on lines
+37
to
+38
| volumes: | ||
| - platform-volume:/var/lib/postgresql/data |
There was a problem hiding this comment.
Missing volume declaration for platform-volume.
The postgres service uses platform-volume for data persistence (line 38), but this volume is not declared in the volumes section at the end of the file. This will cause Docker Compose to create an anonymous volume instead of a named one, which may lead to data loss issues.
🔧 Proposed fix
volumes:
cache:
driver: local
+ platform-volume:
+ driver: localAlso applies to: 251-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 37 - 38, The docker-compose uses the named
volume platform-volume in the postgres service but never declares it in the
top-level volumes section; add a declaration for platform-volume in the compose
file's volumes block (the same volumes section that should include other entries
referenced around lines 251-253) so Docker Compose creates/uses a named
persistent volume instead of an anonymous one—ensure the symbol platform-volume
is added to the volumes mapping (optionally specifying driver or driver_opts if
needed).
| "email": "", | ||
| "username": "", | ||
| "password": "####Please provide encrypted password using crypto-js###", | ||
| "password": "##Please provide encrypted password using crypto-js##", |
There was a problem hiding this comment.
Use empty sensitive placeholders here; instructional strings can break seed/decrypt flow.
Line 16 and Line 29 now store instructional text as actual values. For Line 16 specifically, seed logic treats this as a real encrypted password and attempts decrypt, which can produce invalid credentials instead of failing fast.
🔧 Suggested change
- "password": "##Please provide encrypted password using crypto-js##",
+ "password": "",
@@
- "platformAdminKeycloakPassword":"##Please provide encrypted password using crypto-js##",
+ "platformAdminKeycloakPassword": "",Based on learnings: In this repository, sensitive seed fields in credebl-master-table.json are intentionally kept as empty strings and populated from .env at runtime.
Also applies to: 29-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@libs/prisma-service/prisma/data/credebl-master-table/credebl-master-table.json`
at line 16, Replace the instructional placeholder values in the
credebl-master-table.json sensitive fields with empty-string placeholders so
seed/decrypt logic fails fast and reads real secrets from .env; specifically
remove "##Please provide encrypted password using crypto-js##" from the
"password" property and any similar instructional string at the other sensitive
key on line 29 and set them to "" (empty string) so the seeding/decryption code
(which reads "password" and the other sensitive key) will not attempt to decrypt
an instructional text.
Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose.yml (1)
177-177:⚠️ Potential issue | 🔴 CriticalContainer name mismatch will cause startup failure.
The command references
platform-agent-provisioning-1but theagent-provisioningservice now hascontainer_name: agent-provisioning-service(line 143). This will cause theuntilloop to never find the expected log output, blockingagent-servicestartup indefinitely.🐛 Proposed fix
- command: sh -c 'until (docker logs platform-agent-provisioning-1 | grep "Agent-Provisioning-Service Microservice is listening to NATS"); do sleep 1; done && node dist/apps/agent-service/main.js' + command: sh -c 'until (docker logs agent-provisioning-service | grep "Agent-Provisioning-Service Microservice is listening to NATS"); do sleep 1; done && node dist/apps/agent-service/main.js'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` at line 177, The startup command's wait loop references the wrong container name ("platform-agent-provisioning-1") so it never sees the expected log and blocks agent-service; update the command (the sh -c until ... && node dist/apps/agent-service/main.js block) to use the actual container identifier used in the compose file (e.g., "agent-provisioning-service" or the service name "agent-provisioning"), or preferably switch to using docker-compose logs for the service name to detect the "Agent-Provisioning-Service Microservice is listening to NATS" message; ensure the string in the until grep matches the exact log text and keep the rest of the command intact.
🧹 Nitpick comments (1)
apps/agent-provisioning/AFJ/scripts/start_agent.sh (1)
24-29: Address static analysis findings for validation block.Per SonarCloud analysis:
- Use
[[instead of[for safer conditional tests (handles empty variables and special characters better)- Redirect error messages to stderr using
>&2♻️ Proposed fix
# Validate required parameters -if [ -z "$AGENCY" ] || [ -z "$EXTERNAL_IP" ] || [ -z "$WALLET_NAME" ]; then - echo "ERROR: Missing required parameters" +if [[ -z "$AGENCY" ]] || [[ -z "$EXTERNAL_IP" ]] || [[ -z "$WALLET_NAME" ]]; then + echo "ERROR: Missing required parameters" >&2 echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME ..." exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh` around lines 24 - 29, The validation block in start_agent.sh uses single-bracket tests and prints errors to stdout; update it to use bash [[ ... ]] conditional tests to safely handle empty variables/special chars for AGENCY, EXTERNAL_IP, and WALLET_NAME, and send error lines (the "ERROR: Missing required parameters" and the "Usage: $0 ..." message) to stderr (use >&2). Keep the same exit 1 behavior; change the conditional expression that currently checks [ -z "$AGENCY" ] || [ -z "$EXTERNAL_IP" ] || [ -z "$WALLET_NAME" ] to the [[ -z "$AGENCY" || -z "$EXTERNAL_IP" || -z "$WALLET_NAME" ]] form and redirect both echo calls to >&2.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 46-54: The docker-compose service schema-file-server references
ghcr.io/credebl/schema-file-server:latest but the CI workflow
(continuous-delivery.yml) does not build/publish that image; update the CI to
include schema-file-server in the build matrix (or add a separate job) so the
image is produced and pushed on release. Modify the matrix used by the
build/publish job in continuous-delivery.yml to add an entry for
schema-file-server (matching the service name), ensure the Docker build context
uses apps/schemas and the correct image name ghcr.io/credebl/schema-file-server,
and verify credentials/permissions for the registry are used in the same job.
Ensure any cache/args used by existing image builds (e.g., buildx, tags) are
applied consistently for schema-file-server so the service referenced in
docker-compose.yml is available after CI runs.
---
Outside diff comments:
In `@docker-compose.yml`:
- Line 177: The startup command's wait loop references the wrong container name
("platform-agent-provisioning-1") so it never sees the expected log and blocks
agent-service; update the command (the sh -c until ... && node
dist/apps/agent-service/main.js block) to use the actual container identifier
used in the compose file (e.g., "agent-provisioning-service" or the service name
"agent-provisioning"), or preferably switch to using docker-compose logs for the
service name to detect the "Agent-Provisioning-Service Microservice is listening
to NATS" message; ensure the string in the until grep matches the exact log text
and keep the rest of the command intact.
---
Nitpick comments:
In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh`:
- Around line 24-29: The validation block in start_agent.sh uses single-bracket
tests and prints errors to stdout; update it to use bash [[ ... ]] conditional
tests to safely handle empty variables/special chars for AGENCY, EXTERNAL_IP,
and WALLET_NAME, and send error lines (the "ERROR: Missing required parameters"
and the "Usage: $0 ..." message) to stderr (use >&2). Keep the same exit 1
behavior; change the conditional expression that currently checks [ -z "$AGENCY"
] || [ -z "$EXTERNAL_IP" ] || [ -z "$WALLET_NAME" ] to the [[ -z "$AGENCY" || -z
"$EXTERNAL_IP" || -z "$WALLET_NAME" ]] form and redirect both echo calls to >&2.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ff21d25-6c3b-4e7b-94da-bb37b3c1d22e
📒 Files selected for processing (6)
.env.demoagent.envapps/agent-provisioning/AFJ/scripts/start_agent.shdocker-compose.ymllibs/prisma-service/prisma/data/credebl-master-table/credebl-master-table.jsonlibs/prisma-service/prisma/seed.ts
✅ Files skipped from review due to trivial changes (1)
- libs/prisma-service/prisma/data/credebl-master-table/credebl-master-table.json
🚧 Files skipped from review as they are similar to previous changes (1)
- agent.env
Comment on lines
+46
to
+54
| schema-file-server: | ||
| container_name: schema-file-server | ||
| image: ghcr.io/credebl/schema-file-server:latest | ||
| ports: | ||
| - '4000:4000' | ||
| env_file: | ||
| - ./.env | ||
| volumes: | ||
| - $PWD/apps/schemas:/app/schemas |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if schema-file-server is built in any workflow
rg -l "schema-file-server" .github/workflows/Repository: credebl/platform
Length of output: 42
🏁 Script executed:
#!/bin/bash
# First, check the continuous-delivery.yml to see what services are actually built
cat .github/workflows/continuous-delivery.yml | head -100Repository: credebl/platform
Length of output: 1828
🏁 Script executed:
#!/bin/bash
# Search for schema-file-server references anywhere in the codebase
rg "schema-file-server" --type-list | head -5
rg "schema-file-server" -i -g '!.git' --max-count=20Repository: credebl/platform
Length of output: 637
🏁 Script executed:
#!/bin/bash
# Look for any separate build configuration or Dockerfile for schema-file-server
fd -i "dockerfile" . | grep -i schema
fd -i "schema.*file.*server" .Repository: credebl/platform
Length of output: 42
Build and publish the schema-file-server image or add it to the CI/CD workflow.
The schema-file-server service in docker-compose.yml references ghcr.io/credebl/schema-file-server:latest, but this service is not included in the CI/CD build matrix (.github/workflows/continuous-delivery.yml). The workflow currently builds 19 services but excludes schema-file-server, meaning this image is not automatically published during releases. Add schema-file-server to the matrix or provide a separate build/publish mechanism.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 46 - 54, The docker-compose service
schema-file-server references ghcr.io/credebl/schema-file-server:latest but the
CI workflow (continuous-delivery.yml) does not build/publish that image; update
the CI to include schema-file-server in the build matrix (or add a separate job)
so the image is produced and pushed on release. Modify the matrix used by the
build/publish job in continuous-delivery.yml to add an entry for
schema-file-server (matching the service name), ensure the Docker build context
uses apps/schemas and the correct image name ghcr.io/credebl/schema-file-server,
and verify credentials/permissions for the registry are used in the same job.
Ensure any cache/args used by existing image builds (e.g., buildx, tags) are
applied consistently for schema-file-server so the service referenced in
docker-compose.yml is available after CI runs.
Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
Signed-off-by: KambleSahil3 <sahil.kamble@ayanworks.com>
|
RinkalBhojani
approved these changes
Apr 15, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.env.demo (1)
60-62: NATS authentication configuration appears inconsistent.
NATS_USERandNATS_PASSWORDare set with placeholder values (xxxx), butNATS_AUTH_TYPE=noneandNOTIFICATION_NATS_AUTH_TYPE=noneare configured. This may confuse developers setting up the environment. Consider clarifying in the comments whether authentication is expected to be enabled, or remove the placeholder credentials if auth is disabled.Also applies to: 348-351
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.demo around lines 60 - 62, Placeholders NATS_USER and NATS_PASSWORD conflict with NATS_AUTH_TYPE=none and NOTIFICATION_NATS_AUTH_TYPE=none; either remove the credentials or update the comment to explain they are intentionally unused when auth type is "none". Edit the .env.demo entries for NATS_USER and NATS_PASSWORD (and the repeated block at lines ~348-351) to either delete the placeholder variables or add a clear comment stating "Only set when NATS_AUTH_TYPE or NOTIFICATION_NATS_AUTH_TYPE is not 'none' — leave empty otherwise", so developers aren't confused about expected configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.demo:
- Around line 169-171: The .env entries KEYCLOAK_MANAGEMENT_CLIENT_ID,
KEYCLOAK_MANAGEMENT_CLIENT_SECRET, and KEYCLOAK_REALM currently have inline
comments after the values which some dotenv/shell parsers will treat as part of
the value; remove those inline comments and place explanatory comments on their
own lines above the variables (or delete them) so the values remain pure (e.g.,
move "# Keycloak client created for platform management in keycloak." and "#
Keycloak realm name for the platform" to separate comment lines).
In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh`:
- Around line 24-29: The script's error messages currently print to stdout;
change the echo calls that report missing parameters in start_agent.sh to write
to stderr (redirect output for the two error/usage echo statements) so failures
can be distinguished programmatically; update the block that checks AGENCY,
EXTERNAL_IP, and WALLET_NAME to send its error/usage messages to stderr before
exiting (use stderr redirection for the echo lines that reference "$AGENCY",
"$EXTERNAL_IP", and "$WALLET_NAME").
---
Nitpick comments:
In @.env.demo:
- Around line 60-62: Placeholders NATS_USER and NATS_PASSWORD conflict with
NATS_AUTH_TYPE=none and NOTIFICATION_NATS_AUTH_TYPE=none; either remove the
credentials or update the comment to explain they are intentionally unused when
auth type is "none". Edit the .env.demo entries for NATS_USER and NATS_PASSWORD
(and the repeated block at lines ~348-351) to either delete the placeholder
variables or add a clear comment stating "Only set when NATS_AUTH_TYPE or
NOTIFICATION_NATS_AUTH_TYPE is not 'none' — leave empty otherwise", so
developers aren't confused about expected configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6149175-246f-40fe-8040-953c1f7ee9db
📒 Files selected for processing (3)
.env.demoapps/agent-provisioning/AFJ/scripts/start_agent.shdocker-compose.yml
Comment on lines
+169
to
+171
| KEYCLOAK_MANAGEMENT_CLIENT_ID=credeblClient # Keycloak client created for platform management in keycloak. | ||
| KEYCLOAK_MANAGEMENT_CLIENT_SECRET= | ||
| KEYCLOAK_REALM=credebl-platform # Keycloak realm name for the platform |
There was a problem hiding this comment.
Inline comments may cause parsing issues.
Some .env parsers (especially in shell contexts) don't support inline comments. The # character and text after it might be included as part of the value, causing unexpected behavior.
🔧 Proposed fix - move comments to separate lines
-KEYCLOAK_MANAGEMENT_CLIENT_ID=credeblClient # Keycloak client created for platform management in keycloak.
+# Keycloak client created for platform management in keycloak.
+KEYCLOAK_MANAGEMENT_CLIENT_ID=credeblClient
KEYCLOAK_MANAGEMENT_CLIENT_SECRET=
-KEYCLOAK_REALM=credebl-platform # Keycloak realm name for the platform
+# Keycloak realm name for the platform
+KEYCLOAK_REALM=credebl-platform📝 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.
Suggested change
| KEYCLOAK_MANAGEMENT_CLIENT_ID=credeblClient # Keycloak client created for platform management in keycloak. | |
| KEYCLOAK_MANAGEMENT_CLIENT_SECRET= | |
| KEYCLOAK_REALM=credebl-platform # Keycloak realm name for the platform | |
| # Keycloak client created for platform management in keycloak. | |
| KEYCLOAK_MANAGEMENT_CLIENT_ID=credeblClient | |
| KEYCLOAK_MANAGEMENT_CLIENT_SECRET= | |
| # Keycloak realm name for the platform | |
| KEYCLOAK_REALM=credebl-platform |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 169-169: [UnorderedKey] The KEYCLOAK_MANAGEMENT_CLIENT_ID key should go before the KEYCLOAK_MASTER_REALM key
(UnorderedKey)
[warning] 169-169: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 170-170: [UnorderedKey] The KEYCLOAK_MANAGEMENT_CLIENT_SECRET key should go before the KEYCLOAK_MASTER_REALM key
(UnorderedKey)
[warning] 171-171: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.demo around lines 169 - 171, The .env entries
KEYCLOAK_MANAGEMENT_CLIENT_ID, KEYCLOAK_MANAGEMENT_CLIENT_SECRET, and
KEYCLOAK_REALM currently have inline comments after the values which some
dotenv/shell parsers will treat as part of the value; remove those inline
comments and place explanatory comments on their own lines above the variables
(or delete them) so the values remain pure (e.g., move "# Keycloak client
created for platform management in keycloak." and "# Keycloak realm name for the
platform" to separate comment lines).
Comment on lines
+24
to
+29
| # Validate required parameters | ||
| if [[ -z "$AGENCY" ]] || [[ -z "$EXTERNAL_IP" ]] || [[ -z "$WALLET_NAME" ]]; then | ||
| echo "ERROR: Missing required parameters" | ||
| echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME ..." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Redirect error messages to stderr.
Error messages should be sent to stderr rather than stdout. This follows shell scripting best practices and allows proper error handling when the script output is captured.
🔧 Proposed fix
# Validate required parameters
if [[ -z "$AGENCY" ]] || [[ -z "$EXTERNAL_IP" ]] || [[ -z "$WALLET_NAME" ]]; then
- echo "ERROR: Missing required parameters"
- echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME ..."
+ echo "ERROR: Missing required parameters" >&2
+ echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME ..." >&2
exit 1
fi📝 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.
Suggested change
| # Validate required parameters | |
| if [[ -z "$AGENCY" ]] || [[ -z "$EXTERNAL_IP" ]] || [[ -z "$WALLET_NAME" ]]; then | |
| echo "ERROR: Missing required parameters" | |
| echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME ..." | |
| exit 1 | |
| fi | |
| # Validate required parameters | |
| if [[ -z "$AGENCY" ]] || [[ -z "$EXTERNAL_IP" ]] || [[ -z "$WALLET_NAME" ]]; then | |
| echo "ERROR: Missing required parameters" >&2 | |
| echo "Usage: $0 AGENCY EXTERNAL_IP WALLET_NAME ..." >&2 | |
| exit 1 | |
| fi |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 26-26: Redirect this error message to stderr (>&2).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/agent-provisioning/AFJ/scripts/start_agent.sh` around lines 24 - 29, The
script's error messages currently print to stdout; change the echo calls that
report missing parameters in start_agent.sh to write to stderr (redirect output
for the two error/usage echo statements) so failures can be distinguished
programmatically; update the block that checks AGENCY, EXTERNAL_IP, and
WALLET_NAME to send its error/usage messages to stderr before exiting (use
stderr redirection for the echo lines that reference "$AGENCY", "$EXTERNAL_IP",
and "$WALLET_NAME").
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What ?
Summary by CodeRabbit
Chores
Bug Fixes