Repository navigation
#10670: Add AGENTS.md for AI coding assistant guidance - #956
Merged
Merged
Conversation
Adds a root AGENTS.md hub plus per-module guides for signup-service, signup-ui, signup-with-plugins, and deploy, covering repository overview, tech stack, build/test commands, configuration, and contribution notes for AI agents and contributors. api-test already has its own CLAUDE.md, which the root guide links to instead of duplicating. Addresses mosip/mosip-config#10670 Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdded repository-wide and module-specific ChangesRepository guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 49-54: Update the Kubernetes secrets guidance in the Configuration
section of AGENTS.md to remove `helm --set` as a method for sensitive secret
storage. Direct contributors to use Kubernetes Secret resources or
external-secret references for credentials, while limiting `helm --set` to
non-sensitive values and preserving the reference to deploy/AGENTS.md.
- Line 90: Update the guidance around the command-ordering rule to separate
Maven commands from Java commands: document Maven `-D` properties in Maven
syntax, and document Java `-D` properties before `-jar`. Remove the invalid
combined `mvn ... -Dfoo=bar -jar ...` example while preserving the intended
ordering for each command type.
In `@deploy/AGENTS.md`:
- Around line 24-31: Update the resource-copy guidance in “How secrets actually
flow” to describe `deploy/copy_cm_func.sh` accurately: copying an existing
Secret or ConfigMap fetches its YAML, rewrites the namespace and name, and pipes
it to `kubectl create -f -`. Keep `--from-literal` guidance only for creating
new resources from literal values.
In `@signup-ui/AGENTS.md`:
- Line 69: Update the development guidance in AGENTS.md to remove the
recommendation to use Chrome’s --disable-web-security flag as the normal CORS
solution. Direct developers to use a development proxy or configure backend CORS
instead; if the flag is retained only as an emergency workaround, require a
disposable browser profile with no normal browsing.
- Around line 63-69: Update the documented REACT_APP_API_BASE_URL default to use
the signup-service port, http://localhost:8089/v1/signup, and align any related
mock configuration with that endpoint; alternatively, document and start a
localhost:8088 gateway routing /v1/signup to the service.
🪄 Autofix
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b43628a6-de4d-4d21-b30f-cd71fd9cafaf
📒 Files selected for processing (5)
AGENTS.mddeploy/AGENTS.mdsignup-service/AGENTS.mdsignup-ui/AGENTS.mdsignup-with-plugins/AGENTS.md
- Restrict helm --set to non-sensitive values; kubectl create secret is the documented path for actual credentials. - Separate Maven -D property ordering from plain java -D/-jar ordering instead of blending them into one invalid example. - deploy/AGENTS.md: distinguish copy_cm_func.sh's YAML-pipe copy flow from --from-literal, which prereq.sh uses to create new secrets (captcha, keystore), not to copy existing ones. - signup-ui/AGENTS.md: state the actual signup-service port (8089) since the documented .env.example default (8088) will not reach a local backend as-is, and stop recommending --disable-web-security as a normal CORS workaround. Addresses review comments on mosip#956 Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com>
Author
|
Addressed the CodeRabbit findings in the latest commit. Separate note: |
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 (2)
AGENTS.md (1)
96-96: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse one secure credential-injection rule across both guides.
Both locations permit readers to interpret Helm
--setas a credential path. Requirekubectl create secretor an external-secret mechanism for credentials. Reserve--setfor non-sensitive chart values.
AGENTS.md#L96-L96: replace the combined--set/kubectl create secretwording.deploy/AGENTS.md#L50-L50: remove--setfrom the credential guidance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` at line 96, Update the credential-injection guidance in AGENTS.md (lines 96-96) to require kubectl create secret or an external-secret mechanism, reserving Helm --set for non-sensitive chart values; update deploy/AGENTS.md (lines 50-50) to remove --set from credential guidance and apply the same secure rule.deploy/AGENTS.md (1)
49-49: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not use
|| truefor namespace creation.Use
kubectl applyor handle only theAlreadyExistserror. Unconditional error suppression can hide authentication, kubeconfig, RBAC, and API-server failures and cause misleading later errors.Proposed fix
- Keep new install scripts idempotent where the existing ones are (`kubectl create ns $NS || true` pattern). + Keep new install scripts idempotent without suppressing unrelated `kubectl` failures. Use `kubectl apply` or handle only the `AlreadyExists` case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/AGENTS.md` at line 49, Update the namespace-creation guidance in the install-script instructions to prohibit the `kubectl create ns $NS || true` pattern; require `kubectl apply` or explicit handling limited to the `AlreadyExists` condition so authentication, kubeconfig, RBAC, and API-server errors remain visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 90: Revise the Maven portion of the ordering rule to describe placing -D
system properties before goals as the preferred style rather than a mandatory
requirement, while retaining the separate plain java rule that properties must
precede -jar.
---
Outside diff comments:
In `@AGENTS.md`:
- Line 96: Update the credential-injection guidance in AGENTS.md (lines 96-96)
to require kubectl create secret or an external-secret mechanism, reserving Helm
--set for non-sensitive chart values; update deploy/AGENTS.md (lines 50-50) to
remove --set from credential guidance and apply the same secure rule.
In `@deploy/AGENTS.md`:
- Line 49: Update the namespace-creation guidance in the install-script
instructions to prohibit the `kubectl create ns $NS || true` pattern; require
`kubectl apply` or explicit handling limited to the `AlreadyExists` condition so
authentication, kubeconfig, RBAC, and API-server errors remain visible.
🪄 Autofix
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 442c6069-2278-47f1-9f18-b6f460eedf23
📒 Files selected for processing (3)
AGENTS.mddeploy/AGENTS.mdsignup-ui/AGENTS.md
Maven's CLI parser accepts -D properties in either position relative to the goal, unlike a plain java -jar command where the JVM stops parsing options at -jar. The repo's own build commands (this file included) consistently write -D after the goal (mvn clean install -Dgpg.skip=true), so the previous "-D before goal" rule contradicted 14 existing tracked examples. Keep the java -jar requirement, drop the incorrect Maven requirement. Addresses review comment on mosip#956 Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com>
chetankh239
added a commit
to chetankh239/esignet-plugins
that referenced
this pull request
Aug 12, 2026
Maven's CLI parser accepts -D properties in either position relative to the goal, unlike a plain java -jar command where the JVM stops parsing options at -jar. The previous rule incorrectly applied the java -jar requirement to Maven too. Standardize the examples on -D after the goal (mvn clean install -Dfoo=bar), matching common Maven convention, and correct the stated rule. Self-correction: found while fixing the same mistake in mosip/esignet-signup#956 Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com>
anushasunkada
approved these changes
Aug 25, 2026
Prafulrakhade
pushed a commit
to mosip/esignet-plugins
that referenced
this pull request
Aug 26, 2026
* #10670: Add AGENTS.md for AI coding assistant guidance Addresses mosip/mosip-config#10670 — provides repository overview, tech stack, build/test commands, configuration notes, and contribution guidelines for AI agents and contributors working across the mock-plugin, mosip-identity-plugin, and sunbird-rc-plugin modules. Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com> * #10670: Fix Maven flag ordering in AGENTS.md -Dtest must come before the test goal for it to take effect. Addresses review comment on #215 Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com> * #10670: Correct the Maven -D flag ordering rule Maven's CLI parser accepts -D properties in either position relative to the goal, unlike a plain java -jar command where the JVM stops parsing options at -jar. The previous rule incorrectly applied the java -jar requirement to Maven too. Standardize the examples on -D after the goal (mvn clean install -Dfoo=bar), matching common Maven convention, and correct the stated rule. Self-correction: found while fixing the same mistake in mosip/esignet-signup#956 Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com> * #10670: Add per-module AGENTS.md guides for all three plugins Adds mock-plugin/AGENTS.md, mosip-identity-plugin/AGENTS.md, and sunbird-rc-plugin/AGENTS.md, documenting each module's dual esignet-integration-api/signup-integration-api package layout (where applicable), implemented interfaces, and key classes. Notably flags that mosip-identity-plugin (the production plugin) still ships a MockIdentityVerifierPluginImpl for signup identity verification. Root AGENTS.md now links to each. Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com> * #10670: Address review feedback on AGENTS.md build commands - Add -Dgpg.skip=true to every mvn clean install example (root and per-module AGENTS.md): each module's maven-gpg-plugin binds sign to the verify phase, which install runs through, so local builds fail without a signing key unless skipped. - sunbird-rc-plugin/AGENTS.md: remove the "don't drive-by rename the misspelled test file" Do-not item, per maintainer request. Addresses review comments on #215 Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com> * Slim down AGENTS.md tree for agent-speed, not README duplication Consolidate facts restated 2-3x across sections ("target develop", "no root pom.xml", mock-plugin not-for-production warning, -D CLI ordering rationale, provided-scope dependency rule) down to one statement each, and tighten prose. 477 -> 388 lines across root + 3 module guides. Every command, -Dgpg.skip=true flag, and security warning kept, just tighter. Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com> --------- Signed-off-by: Chetan Kumar Hirematha <chetankumar.h.239@gmail.com>
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.
Addresses mosip/mosip-config#10670 — adds a root AGENTS.md hub plus per-module guides for signup-service, signup-ui, signup-with-plugins, and deploy.
Summary by CodeRabbit