Conversation
There was a problem hiding this comment.
Pull request overview
Updates contributor/agent documentation to better match the repository’s actual local development and E2E workflows (including .env usage, expected runtime tooling, and Playwright Docker runner behavior).
Changes:
- Documented Node.js and npm expectations and clarified
npm run devvsnpm startbehavior around.env/environment variables. - Expanded the listed lint/test commands to match the
package.jsonscript set (including Playwright install/local/docker report flows). - Added notes on Playwright Docker runner argument forwarding and relevant E2E environment variables.
| - **Testing**: Jest 30 + React Testing Library (unit), Playwright 1.x (E2E) | ||
| - **Package Manager**: npm | ||
| - **Node Version**: 24+ required (`engines` field enforces `>=24.0`) | ||
| - **Node Version**: 24+ required (`engines` field enforces `>=24.0`); npm 11.x is expected (`^11.0.0`) |
| REACT_APP_META_BACKEND=undefined | ||
| META_YDB_BACKEND=undefined |
There was a problem hiding this comment.
Non-standard
.env =undefined pattern lacks inline explanation
REACT_APP_META_BACKEND=undefined and META_YDB_BACKEND=undefined set these variables to the literal string "undefined", not to an absent or empty value. This works because rsbuild.config.ts explicitly checks !== 'undefined' before enabling the proxy, but it is an unusual .env idiom that could mislead developers into thinking it is standard syntax for unsetting a variable. Adding a brief inline comment (e.g. # literal string "undefined" — disables this mode) would make the intent clear, especially since omitting the line or setting = would behave differently.
Prompt To Fix With AI
This is a comment left during a code review.
Path: AGENTS.md
Line: 219-220
Comment:
**Non-standard `.env` `=undefined` pattern lacks inline explanation**
`REACT_APP_META_BACKEND=undefined` and `META_YDB_BACKEND=undefined` set these variables to the *literal string* `"undefined"`, not to an absent or empty value. This works because `rsbuild.config.ts` explicitly checks `!== 'undefined'` before enabling the proxy, but it is an unusual `.env` idiom that could mislead developers into thinking it is standard syntax for unsetting a variable. Adding a brief inline comment (e.g. `# literal string "undefined" — disables this mode`) would make the intent clear, especially since omitting the line or setting `=` would behave differently.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Motivation
.envoverrides and clarify the defaultnpm run devbehavior.npmexpectation alongside the Node version.Description
AGENTS.mdto note that Node>=24.0is required and thatnpm11.x is expected (^11.0.0).npm startto the Quick Start section and clarified thatnpm run devuses local backend defaults whilenpm startpicks up.envoverrides.npm run lint:js,npm run lint:styles,npm run lint:other,npm run test:e2e:install,npm run test:e2e:docker:report, andnpm run test:e2e:localentries..envusage for single-cluster and multi-cluster development and addedREACT_APP_META_BACKEND/META_YDB_BACKENDexamples, plus documented Playwright/Docker E2E env vars (PLAYWRIGHT_YDB_IMAGE,PLAYWRIGHT_HTML_HOST/PLAYWRIGHT_HTML_PORT,PLAYWRIGHT_VIDEO) and that Docker E2E arguments are forwarded to Playwright (image tag is derived frompackage-lock.jsonwhen not overridden).Testing
npx prettier --check AGENTS.mdand it returned no formatting issues.npm run lint:other(Prettier check) and it passed.lint-stagedtasks ran during commit and appliedprettier --writewhere needed, with the commit completing successfully.git diff --checkproduced no warnings or whitespace errors.Codex Task
Greptile Summary
This PR updates
AGENTS.mdwith accurate developer-workflow documentation: it pins the npm version expectation (^11.0.0) alongside the existing Node>=24.0requirement, expands the lint and E2E test command tables, and documents thenpm startvsnpm run devdistinction and Docker/Playwright environment variables.lint:js,lint:styles,lint:other,test:e2e:install,test:e2e:docker:report,test:e2e:local) exist verbatim inpackage.jsonand the descriptions match their implementations.scripts/playwright-docker.sh: arguments are forwarded via\"$@\", the Playwright image tag is derived frompackage-lock.json, andPLAYWRIGHT_APP_BACKENDcontrols whether a local YDB container is started..envexample introducesREACT_APP_META_BACKEND=undefinedandMETA_YDB_BACKEND=undefinedentries, which are consistent with how the codebase uses the string\"undefined\"(checked inrsbuild.config.ts) but could use a brief inline comment explaining the non-standard pattern.Confidence Score: 4/5
Documentation-only change; all commands and env vars cross-check against the actual scripts, with one minor wording improvement possible in the .env example.
Every added npm script was verified to exist in package.json, the Docker runner claims (argument forwarding, image-tag derivation, YDB container lifecycle) match the playwright-docker.sh implementation, and the npm start vs npm run dev distinction accurately reflects the scripts. The only item worth attention is that the .env example sets two variables to the literal string "undefined" without explaining why — this is intentional given the rsbuild.config.ts check, but could confuse contributors unfamiliar with this codebase pattern.
No files require special attention; AGENTS.md is the only changed file and its content is factually correct.
Important Files Changed
.env=undefinedpattern lacks a clarifying commentFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Start dev session] --> B{Backend target?} B -->|Local default| C[npm run dev\nsets hardcoded backend env vars] B -->|Custom cluster via .env| D[npm start\nrsbuild dev — .env file controls env vars] B -->|Multi-cluster via .env| D C --> G[Dev server on port 3000] D --> G G --> H{Run E2E tests?} H -->|Local browsers| I[npm run test:e2e:install\nnpm run test:e2e:local] H -->|Docker runner| J[npm run test:e2e:docker\nor test:e2e:docker:report] J --> K{PLAYWRIGHT_APP_BACKEND set?} K -->|No| L[Script starts local-ydb container\nYDB image from PLAYWRIGHT_YDB_IMAGE] K -->|Yes| M[Uses external backend\nno local YDB container] L --> N[Playwright image tag from package-lock.json\nCLI args forwarded to playwright test] M --> NPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "docs: update agent workflows" | Re-trigger Greptile
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
😟 No changes in tests. 😕
Bundle Size: ✅
Current: 63.75 MB | Main: 63.75 MB
Diff: 0.00 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information