-
Notifications
You must be signed in to change notification settings - Fork 19
docs: Update AGENTS.md #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Document repository layout and service playbooks - Capture tooling, scripts, and validation workflows - Outline critical rules and references for agents
Completed Working on "Auto PR Description"✅ Workflow completed successfully. |
Completed Working on "Code Review"✅ Workflow completed successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Findings: 0 BLOCKER, 0 CRITICAL, 7 MAJOR, 1 MINOR (all in AGENTS.md).
Key themes:
- Repository and service playbooks reference directories (
apps/*/kafka,nats, etc.) that do not exist, sending agents to the wrong paths. - Environment/workflow guidance (admin env vars, Prisma migrations, dev server ports) conflicts with the actual tooling, leading to failed setups.
- Guardrails misdirect operators on where credentials and seed customizations should live, risking broken auth flows.
Next steps:
- Align every documented path with the real
src/*structure for ecommerce and logistics services. - Update environment and workflow instructions (Vite var names, Prisma scripts, dev server port) so agents follow commands that succeed.
- Clarify that auth credentials are seeded (not .env) and that custom data goes through
scripts/customSeed.ts, then adjust the guardrail language accordingly.
| ├─ ecommerce-server/ # NestJS + PostgreSQL backend emitting Kafka events | ||
| │ ├─ src/ # Domain modules (see src/order/ for reference) | ||
| │ ├─ prisma/ # schema.prisma, migrations, Prisma scripts | ||
| │ ├─ kafka/ # messaging helpers for send pattern |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: The repository overview and ecommerce-server playbook describe a top-level apps/ecommerce-server/kafka/ directory, but the codebase only exposes src/kafka/. Pointing agents to a path that does not exist makes it impossible to locate the Kafka publisher templates referenced by the doc. Suggestion: Update the repo overview/ecommerce-server directory list to reference apps/ecommerce-server/src/kafka/ (and clarify that messaging helpers live under src/).
| ├─ logistic-server/ # NestJS + MySQL backend consuming Kafka/NATS | ||
| │ ├─ src/ | ||
| │ ├─ prisma/ | ||
| │ ├─ kafka/ & nats/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: The logistics-server summary advertises sibling apps/logistic-server/kafka/ and .../nats/ folders even though only src/kafka/ and src/nats/ exist. Following the guide sends agents hunting under directories that are not present. Suggestion: Point the repo-structure diagram and logistics-server playbook to apps/logistic-server/src/kafka/ and apps/logistic-server/src/nats/ (or otherwise state that the messaging code lives under src/).
| - **Prisma data layer:** Each backend supplies `prisma/` assets plus scripts for generating clients (`npm run prisma:generate`), saving migrations, initializing (`npm run db:init`), cleaning, and seeding. Running Prisma commands requires the corresponding database container/connection to be available. | ||
| - **Messaging modules:** `kafka/` directories wire Kafka topics; `nats/` is present in the logistics app for optional NATS consumption. Respect the established send/receive semantics from the README when extending topics. | ||
| - **Docker workflows:** Prefer the provided scripts (`npm run docker:dev`, `compose:up`, `compose:down`, `package:container`) instead of ad hoc commands. They encapsulate required broker and database containers per service. | ||
| - **Frontend tooling:** The admin app uses Vite. Scripts (`start`, `build`, `serve`, `type-check`, `lint`, `format`) expect `REACT_APP_SERVER_URL` and optional `PORT` (defaults to 3001) defined in `.env`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: Both the workspace setup section here and the ecommerce-admin playbook (line 74) tell agents to export REACT_APP_SERVER_URL, but the Vite-based admin only reads import.meta.env.VITE_REACT_APP_SERVER_URL (see apps/ecommerce-admin/.env and src/data-provider/graphqlDataProvider.ts). Using the documented name leaves the GraphQL client pointing at undefined, so the dashboard cannot reach the backend. Suggestion: Replace the REACT_APP_SERVER_URL guidance with VITE_REACT_APP_SERVER_URL (and call out the Vite prefix requirement) everywhere the variable is mentioned so agents configure the environment the app actually consumes.
| npm install | ||
| ``` | ||
| 2. **Bootstrap databases via Docker:** use `npm run docker:dev` (per backend) or `npm run compose:up` / `npm run compose:down` to orchestrate the DB + broker containers defined in the service. | ||
| 3. **Generate Prisma client & migrations:** `npm run prisma:generate`, then `npm run db:migrate-save`, `npm run db:migrate-up`, and `npm run db:init` to apply the baseline (`db:clean` resets when needed). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: Step 3 tells agents to run db:migrate-save, db:migrate-up, and then db:init, but each backend’s db:init script already runs migrate-save/migrate-up/seed. Following the doc causes the second db:migrate-save to fail because the migration name already exists, breaking the setup workflow. Suggestion: Either have the workflow call npm run db:init by itself or document the manual migrate commands without repeating db:init, so migrations run exactly once.
| ### 🧾 `apps/ecommerce-server/` | ||
| - **Purpose & stack:** Order management API (NestJS + GraphQL) backed by PostgreSQL and Kafka to emit product/order lifecycle events. | ||
| - **Key directories:** `src/order/` (baseline domain module), `prisma/` (see `schema.prisma`), `kafka/`, `scripts/`, Docker compose manifests. | ||
| - **Environment vars:** PostgreSQL DSN, Kafka broker URLs, JWT secret, HTTP port, and admin credentials stored in `.env`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: This section claims the ecommerce-server admin credentials live in .env, but apps/ecommerce-server/.env only defines DB/JWT/Kafka values while the default user/password are created in scripts/seed.ts (lines 28-33). Pointing operators to .env makes them edit a file that has no effect on the login they are trying to rotate. Suggestion: Document that the ecommerce-server admin credentials are seeded via scripts/seed.ts (and must be changed there or via Prisma) instead of referencing a non-existent .env entry.
| ### 🚚 `apps/logistic-server/` | ||
| - **Purpose & stack:** Warehouse/shipment backend (NestJS + GraphQL) using MySQL, Kafka consumers, and optional NATS connectors to react to ecommerce events. | ||
| - **Key directories:** Domain modules in `src/`, `prisma/` schema + migrations, `kafka/` and `nats/` adapters, Docker compose assets (`docker-compose.dev.yml`). | ||
| - **Environment vars:** MySQL DSN, Kafka/NATS endpoints, HTTP Basic or JWT secrets, HTTP port, service credentials. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: This playbook repeats that logistic-server credentials are configured in .env even though apps/logistic-server/.env mirrors the ecommerce file (only DB/JWT/Kafka/NATS values) and the admin account is created in scripts/seed.ts. The doc therefore sends agents to the wrong place when rotating credentials. Suggestion: Clarify that logistic-server credentials come from scripts/seed.ts (admin/admin by default) and must be changed in that seed process or via Prisma, not via .env variables.
| 2. **Bootstrap databases via Docker:** use `npm run docker:dev` (per backend) or `npm run compose:up` / `npm run compose:down` to orchestrate the DB + broker containers defined in the service. | ||
| 3. **Generate Prisma client & migrations:** `npm run prisma:generate`, then `npm run db:migrate-save`, `npm run db:migrate-up`, and `npm run db:init` to apply the baseline (`db:clean` resets when needed). | ||
| 4. **Seed data:** `npm run seed` after migrations (leverages `scripts/seed.ts`). | ||
| 5. **Start services:** `npm run start` (NestJS) or `npm run start` (Vite) with `.env` loaded; ecommerce-admin dev server defaults to http://localhost:3001 unless `PORT` overrides. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor]: Line 111 says the ecommerce-admin dev server defaults to 3001 and respects PORT, but the start script runs plain vite and vite.config.ts never reads PORT, so the UI actually serves on Vite’s default 5173. Following the doc causes health checks to watch the wrong port. Suggestion: Update the workflow notes to mention the real default (5173) or describe how to override the port (e.g., via vite --port).
| - Container build validation is handled through `npm run package:container` in each app to confirm Dockerfiles remain functional. | ||
|
|
||
| ## ⚠️ Critical Rules | ||
| - Never invent or seed new sample data outside the sanctioned Prisma seed scripts; extend `scripts/seed.ts` if adjustments are required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: The guardrail section instructs agents to edit scripts/seed.ts, but both backends wire their default admin accounts there and expect customization to live in scripts/customSeed.ts. Modifying seed.ts directly breaks the generated bootstrap logic and complicates future syncs. Suggestion: Direct agents to add data changes in scripts/customSeed.ts (or clearly explain the supported hook) so they extend the seed pipeline without rewriting the core script.
Summary
This PR adds the AGENTS guide that documents how this workspace is structured and how agents should interact with each service.
Changes
Why
Keeping AGENTS.md current ensures automation agents have an authoritative reference for the repo structure, tooling expectations, and operational guardrails.
Summary
This PR introduces a comprehensive AGENTS guide that documents the workspace structure across the ecommerce-server, logistic-server, and ecommerce-admin services. It ensures automation agents have a single reference for service playbooks, tooling expectations, messaging contracts, and operational guardrails.
Changes
AGENTS.mdoutlining the repository layout, common tooling requirements, and cross-service conventions.Commits
Testing
AGENTS.mdrenders correctly in GitHub (headings, code blocks, and lists).