feat(session): merge session plugin into monorepo (#5553) #5555
Conversation
Merge @eggjs/session plugin into the monorepo following the standard plugin structure. - Updated to version 5.0.0-beta.13 - Migrated from ESLint to oxlint - Converted tests from Mocha to Vitest - Updated dependencies to use workspace and catalog references - Added koa-session and @eggjs/redis to pnpm catalog - Updated minimum Node.js requirement to 22.18.0 - Configured tsdown for unbundled ESM builds - Added plugin documentation to CLAUDE.md 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant EggBoot as Egg Boot (App)
participant Config as SessionConfig (zod)
participant Middleware as Koa Session Middleware
participant Store as Session Store (cookie/redis/memory)
Dev->>EggBoot: start app
EggBoot->>Config: parse/validate app.config.session
Config-->>EggBoot: validated config
EggBoot->>Middleware: register 'session' middleware
EggBoot->>EggBoot: subscribe session events (missed/expired/invalid)
Client->>Middleware: HTTP request
Middleware->>Store: get(sessionId)
alt session found
Store-->>Middleware: session data
else session missing/expired
Store-->>Middleware: null
Middleware->>EggBoot: emit event
EggBoot-->>EggBoot: log (mask if logValue=false)
end
Middleware-->>Client: response (Set-Cookie / body)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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.
Pull Request Overview
This PR merges the session plugin into the monorepo, bringing session management functionality directly into the main codebase. The plugin has been migrated from external dependency to workspace package with modernized tooling and configuration.
- Migrated @eggjs/session plugin into the monorepo as a workspace package
- Updated build system from ESLint to oxlint and from Mocha to Vitest
- Modernized Node.js requirement to 22.18.0+ and added tsdown for unbundled ESM builds
Reviewed Changes
Copilot reviewed 59 out of 62 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pnpm-workspace.yaml | Updated catalog dependencies for session-related packages |
| plugins/session/ | Complete session plugin package with source, tests, and configuration |
| packages/egg/ | Updated to use workspace session dependency instead of catalog |
| CLAUDE.md | Added documentation for session plugin functionality |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| .expect({ foo: 'bar' }) | ||
| .expect(res => { | ||
| const cookies = res.headers['set-cookie']; | ||
| console.log(cookies); |
There was a problem hiding this comment.
Debug console.log statement should be removed from production test code. Use proper test assertions or debugging tools instead.
| console.log(cookies); | |
| assert(Array.isArray(cookies) && cookies.length > 0, 'Expected set-cookie header to be present'); |
| }); | ||
| afterAll(() => app.close()); | ||
|
|
||
| it('should work with maxage=ession', async () => { |
There was a problem hiding this comment.
Typo in test description: 'maxage=ession' should be 'maxAge=session'.
| it('should work with maxage=ession', async () => { | |
| it('should work with maxAge=session', async () => { |
Deploying egg with
|
| Latest commit: |
521a55a
|
| Status: | 🚫 Build failed. |
Summary of ChangesHello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying egg-v3 with
|
| Latest commit: |
521a55a
|
| Status: | 🚫 Build failed. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully merges the @eggjs/session plugin into the monorepo, updating its dependencies, build process, and tests. The changes are well-structured and follow the monorepo conventions. I've identified a few minor issues related to code style, maintainability, and leftover artifacts in tests, and provided suggestions for improvement. These include using a more generic type for better API design, ensuring consistency in code, removing a debug statement, and fixing a copy-paste error in a test fixture.
| | { | ||
| new (app: Application): SessionStore; | ||
| }; |
There was a problem hiding this comment.
The type Application used in the constructor signature new (app: Application): SessionStore; is defined locally in this file and is not exported. This can be confusing for users of this API and makes it hard to type session store classes correctly. It would be clearer to use the EggCore type from @eggjs/core, which is more generic and accurately reflects that the app instance is an EggCore compatible object.
| | { | |
| new (app: Application): SessionStore; | |
| }; | |
| | { | |
| new (app: EggCore): SessionStore; | |
| }; |
|
|
||
| export default { | ||
| session: SessionConfig.parse({ | ||
| maxAge: 24 * 3600 * 1000, // ms, one day |
There was a problem hiding this comment.
For consistency with the calculation in README.md (24 * 60 * 60 * 1000), it's better to write this as 24 * 60 * 60 * 1000. This also improves readability by explicitly showing hours, minutes, and seconds.
| maxAge: 24 * 3600 * 1000, // ms, one day | |
| maxAge: 24 * 60 * 60 * 1000, // 1 day in ms |
| .expect({ foo: 'bar' }) | ||
| .expect(res => { | ||
| const cookies = res.headers['set-cookie']; | ||
| console.log(cookies); |
There was a problem hiding this comment.
A console.log statement was left in the test. This should be removed before merging.
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "name": "httponly-false-session" | |||
There was a problem hiding this comment.
The package name appears to be a copy-paste error. It should be session-maxage-session to match the directory name and the test case it's intended for.
| "name": "httponly-false-session" | |
| "name": "session-maxage-session" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (9)
plugins/session/test/fixtures/cookie-session/package.json (1)
1-3: Set fixture package.json toprivateFixtures are never meant to be published. Adding
"private": truekeeps tooling from warning or accidentally trying to publish this manifest.plugins/session/test/fixtures/logValue-false-session/package.json (1)
1-3: Consider marking the fixture package.json as privateAdding
"private": trueis a low-cost guardrail that prevents accidental publishing or workspace hoisting of this test fixture.{ - "name": "logvalue-false-session" + "name": "logvalue-false-session", + "private": true }plugins/session/README.md (3)
8-8: Fix badge: repository path changedThe CodeRabbit PR badge still points to eggjs/session. Update to the monorepo or remove.
Apply:
- +
110-115: Clean up configuration section formatting and wording“logValue” description is inside a bash code block and phrasing is unclear.
Apply:
- - logValue - -```bash -Support not to print the session value when session event trigger log. Default to be true. -``` + - logValue: Do not print the session value when session events emit logs. Default: true.
27-32: Clarify default enablementText says enabled by default, yet sample shows enabling it. Add a note to avoid confusion.
Apply:
-exports.session = true; // enable by default +exports.session = true; // Enabled by default; set explicitly only if customizingplugins/session/test/fixtures/memory-session-generator/app.js (1)
13-15: Prefer deleting keys when destroying sessionsSetting the value to
undefinedkeeps the property around and can accumulate dead entries in this shared fixture store. Usingdeleteclears the slot altogether and avoids unnecessary growth over long-running test suites.- async destroy(key) { - sessions[key] = undefined; - }, + async destroy(key) { + delete sessions[key]; + },plugins/session/test/fixtures/samesite-none-session/config/config.default.js (1)
4-6: Align SameSite=None with secure cookiesBrowsers require Secure when SameSite=None. For realism (even in tests), set secure: true to avoid misleading behavior.
Apply:
exports.session = { sameSite: 'none', + secure: true, };plugins/session/src/types.ts (1)
4-15: Confirm augmentation exposure to consumersEnsure this file is included in the published types so module augmentation is applied (e.g., export types entry or include in "types" build output).
Example (package.json):
- "types": "./dist/index.d.ts", + "types": "./dist/index.d.ts", + "typesVersions": { + "*": { "*": ["dist/*.d.ts", "dist/**/*.d.ts"] } + }plugins/session/src/app.ts (1)
25-29: Avoid logging session values by defaultSession contents may include PII/secrets. Consider defaulting to logValue: false in config to be safer by default.
| "devDependencies": { | ||
| "@eggjs/mock": "workspace:*", | ||
| "@eggjs/redis": "catalog:", | ||
| "@eggjs/tsconfig": "catalog:", | ||
| "@types/node": "catalog:", | ||
| "tsdown": "catalog:", | ||
| "typescript": "catalog:", | ||
| "vitest": "catalog:" | ||
| }, |
There was a problem hiding this comment.
Declare the internal @eggjs/core dependency to fix typecheck failures.
src/app/extend/application.ts imports @eggjs/core, but this package.json doesn’t list it, so pnpm omits it from the workspace graph and TypeScript cannot resolve the module (see the failing pnpm -r run typecheck). Add it as a workspace dependency to unblock CI.
"devDependencies": {
+ "@eggjs/core": "workspace:*",
"@eggjs/mock": "workspace:*",
"@eggjs/redis": "catalog:",
"@eggjs/tsconfig": "catalog:",
"@types/node": "catalog:",
"tsdown": "catalog:",
"typescript": "catalog:",
"vitest": "catalog:"
},📝 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.
| "devDependencies": { | |
| "@eggjs/mock": "workspace:*", | |
| "@eggjs/redis": "catalog:", | |
| "@eggjs/tsconfig": "catalog:", | |
| "@types/node": "catalog:", | |
| "tsdown": "catalog:", | |
| "typescript": "catalog:", | |
| "vitest": "catalog:" | |
| }, | |
| "devDependencies": { | |
| "@eggjs/core": "workspace:*", | |
| "@eggjs/mock": "workspace:*", | |
| "@eggjs/redis": "catalog:", | |
| "@eggjs/tsconfig": "catalog:", | |
| "@types/node": "catalog:", | |
| "tsdown": "catalog:", | |
| "typescript": "catalog:", | |
| "vitest": "catalog:" | |
| }, |
🤖 Prompt for AI Agents
In plugins/session/package.json around lines 57 to 65, the file is missing the
internal @eggjs/core workspace dependency used by src/app/extend/application.ts;
add "@eggjs/core": "workspace:*" to the devDependencies block so pnpm includes
it in the workspace graph and TypeScript can resolve the module.
| "scripts": { | ||
| "build": "tsdown", | ||
| "typecheck": "tsc --noEmit", | ||
| "lint": "oxlint --type-aware", | ||
| "lint:fix": "npm run lint -- --fix", | ||
| "test": "npm run lint:fix && vitest", | ||
| "prepublishOnly": "npm run build" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add the required clean script (and its tool) per plugin standards.
Our plugin guidelines demand a clean script (rimraf dist) alongside the existing build/test commands. Please add the script and list rimraf in devDependencies so contributors can reset build artifacts locally.
"devDependencies": {
"@eggjs/core": "workspace:*",
"@eggjs/mock": "workspace:*",
"@eggjs/redis": "catalog:",
"@eggjs/tsconfig": "catalog:",
"@types/node": "catalog:",
+ "rimraf": "catalog:",
"tsdown": "catalog:",
"typescript": "catalog:",
"vitest": "catalog:"
},
"scripts": {
+ "clean": "rimraf dist",
"build": "tsdown",
"typecheck": "tsc --noEmit",
"lint": "oxlint --type-aware",📝 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.
| "scripts": { | |
| "build": "tsdown", | |
| "typecheck": "tsc --noEmit", | |
| "lint": "oxlint --type-aware", | |
| "lint:fix": "npm run lint -- --fix", | |
| "test": "npm run lint:fix && vitest", | |
| "prepublishOnly": "npm run build" | |
| "devDependencies": { | |
| "@eggjs/core": "workspace:*", | |
| "@eggjs/mock": "workspace:*", | |
| "@eggjs/redis": "catalog:", | |
| "@eggjs/tsconfig": "catalog:", | |
| "@types/node": "catalog:", | |
| "rimraf": "catalog:", | |
| "tsdown": "catalog:", | |
| "typescript": "catalog:", | |
| "vitest": "catalog:" | |
| }, | |
| "scripts": { | |
| "clean": "rimraf dist", | |
| "build": "tsdown", | |
| "typecheck": "tsc --noEmit", | |
| "lint": "oxlint --type-aware", | |
| "lint:fix": "npm run lint -- --fix", | |
| "test": "npm run lint:fix && vitest", | |
| "prepublishOnly": "npm run build" | |
| }, |
🤖 Prompt for AI Agents
In plugins/session/package.json around lines 66 to 72, add a "clean" npm script
that runs "rimraf dist" and add "rimraf" to devDependencies so contributors can
remove build artifacts; update the "scripts" section to include "clean": "rimraf
dist" and add an entry in devDependencies (e.g., "rimraf": "^X.Y.Z") ensuring
package.json remains valid and npm install will provide the tool.
| 1. Dependent [@eggjs/redis](https://github.com/eggjs/egg/tree/next/plugins/redis) | ||
|
|
||
| ```bash | ||
| npm i --save @eggjs/redis | ||
| ``` | ||
|
|
||
| 2. Import `@eggjs/redis` as a plugin and set the configuration | ||
|
|
||
| ```js | ||
| // config/plugin.js | ||
| exports.redis = { | ||
| enable: true, | ||
| package: '@eggjs/redis', | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
Verify monorepo link correctness
The Redis plugin link targets next/plugins/redis. Confirm path stays valid post-merge.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh api repos/eggjs/egg/contents/plugins/redis --jq '.path // "missing"'Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Determine default branch
default_branch=$(gh api repos/eggjs/egg --jq '.default_branch')
echo "default_branch=${default_branch}"
# Check plugin path on default branch
gh api repos/eggjs/egg/contents/plugins/redis -F ref="${default_branch}" --jq '.path // "missing"'
# Check plugin path on next branch
gh api repos/eggjs/egg/contents/plugins/redis -F ref=next --jq '.path // "missing"'Length of output: 150
Fix broken Redis plugin link in plugins/session/README.md
The link to @eggjs/redis at https://github.com/eggjs/egg/tree/next/plugins/redis returns 404—update it to the correct repository URL (e.g. https://github.com/eggjs/redis).
🤖 Prompt for AI Agents
In plugins/session/README.md around lines 40 to 54, the README links to a
non-existent URL for the Redis plugin; update the broken link to the correct
repository URL (replace https://github.com/eggjs/egg/tree/next/plugins/redis
with https://github.com/eggjs/redis), verify the markdown link syntax remains
valid, and run a quick scan of the file to ensure no other references to the old
URL remain.
| Please open an issue [here](https://github.com/eggjs/egg/issues). | ||
|
|
There was a problem hiding this comment.
Address markdownlint MD059: descriptive link text
Replace “here” with descriptive text.
Apply:
-Please open an issue [here](https://github.com/eggjs/egg/issues).
+Please open an issue on the [egg repository issue tracker](https://github.com/eggjs/egg/issues).📝 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.
| Please open an issue [here](https://github.com/eggjs/egg/issues). | |
| Please open an issue on the [egg repository issue tracker](https://github.com/eggjs/egg/issues). |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
120-120: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 Prompt for AI Agents
In plugins/session/README.md around lines 120 to 121, the link text “here” is
non-descriptive and triggers markdownlint MD059; replace it with descriptive
text (e.g., “open an issue on the Egg.js GitHub”) so the link clearly conveys
its destination, updating the sentence to read something like “Please open an
issue on the Egg.js GitHub” while preserving the existing URL.
plugins/session/src/app.ts
Outdated
| @@ -0,0 +1,32 @@ | |||
| import type { ILifecycleBoot, EggCore } from '@eggjs/core'; | |||
There was a problem hiding this comment.
Fix implicit any and tighten typing for boot logic
- Type the app field.
- Add explicit return type.
- Type event payloads to eliminate implicit any errors.
Apply:
-import type { ILifecycleBoot, EggCore } from '@eggjs/core';
+import type { ILifecycleBoot, EggCore, Context } from '@eggjs/core';
@@
export default class AppBoot implements ILifecycleBoot {
- private readonly app;
+ private readonly app: EggCore;
@@
- configWillLoad() {
+ configWillLoad(): void {
@@
- app.on('session:missed', ({ ctx, key }) => {
+ app.on('session:missed', ({ ctx, key }: { ctx: Context; key: string }) => {
ctx.coreLogger.warn('[session][missed] key(%s)', key);
});
- app.on('session:expired', ({ ctx, key, value }) => {
+ app.on(
+ 'session:expired',
+ ({ ctx, key, value }: { ctx: Context; key: string; value: unknown }) => {
ctx.coreLogger.warn('[session][expired] key(%s) value(%j)', key, app.config.session.logValue ? value : '');
- });
- app.on('session:invalid', ({ ctx, key, value }) => {
+ });
+ app.on(
+ 'session:invalid',
+ ({ ctx, key, value }: { ctx: Context; key: string; value: unknown }) => {
ctx.coreLogger.warn('[session][invalid] key(%s) value(%j)', key, app.config.session.logValue ? value : '');
- });
+ });Also applies to: 5-5, 11-11, 22-30
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: pnpm -r run typecheck failed. TypeScript compilation errors detected across multiple files: Cannot find module '@eggjs/core' (src/app.ts, src/app/extend/application.ts, src/types.ts), implicit any in binding elements (src/app.ts: ctx, key, value), missing type declarations for modules '@eggjs/core' and '@eggjs/supertest', and multiple property/type errors in application.ts and test files. Exit code: 2.
🤖 Prompt for AI Agents
In plugins/session/src/app.ts lines 1, 5, 11 and 22-30, the boot module uses
implicitly typed app, return value and event payloads; add explicit typings:
import the proper Egg/EggApp/Application and ILifecycleBoot types at the top
(line 1), annotate the local app field with the Application/EggCore concrete
type on line 5, give the exported boot function an explicit return type
(ILifecycleBoot or Promise<ILifecycleBoot> as appropriate) on line 11, and
define typed interfaces for each event payload used in lines 22-30 then use
those interfaces in the app.on/app.once listener signatures so the callback
parameters are strongly typed rather than implicit any.
| { | ||
| "name": "memory-session-generator" | ||
| } |
There was a problem hiding this comment.
Add the required Node.js engines constraint
Per the coding guidelines, every package.json under plugins/** must declare the minimum Node.js runtime as >=22.18.0. This fixture package omits the engines.node field, so it currently allows older runtimes that the monorepo no longer supports. Please add the constraint to keep the fixtures aligned with the enforced baseline.
{
"name": "memory-session-generator"
+ , "engines": {
+ "node": ">=22.18.0"
+ }
}As per coding guidelines
📝 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.
| { | |
| "name": "memory-session-generator" | |
| } | |
| { | |
| "name": "memory-session-generator", | |
| "engines": { | |
| "node": ">=22.18.0" | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: pnpm -r run typecheck failed. TypeScript compilation errors detected across multiple files: Cannot find module '@eggjs/core' (src/app.ts, src/app/extend/application.ts, src/types.ts), implicit any in binding elements (src/app.ts: ctx, key, value), missing type declarations for modules '@eggjs/core' and '@eggjs/supertest', and multiple property/type errors in application.ts and test files. Exit code: 2.
🤖 Prompt for AI Agents
In plugins/session/test/fixtures/memory-session-generator/package.json around
lines 1 to 3, the package.json is missing the required Node.js engines
constraint; add an "engines" field with "node": ">=22.18.0" to enforce the
minimum runtime (e.g., add "engines": { "node": ">=22.18.0" } at top-level) so
the fixture matches the monorepo coding guidelines.
| async get(key) { | ||
| return sessions[key]; | ||
| }, | ||
|
|
||
| async set(key, value) { | ||
| sessions[key] = value; | ||
| }, | ||
|
|
||
| async destroy(key) { | ||
| sessions[key] = undefined; | ||
| }, |
There was a problem hiding this comment.
Normalize store API and semantics (null on miss, accept maxAge, delete on destroy)
Current get returns undefined on miss and destroy sets undefined, which diverges from the Redis fixture (returns null) and common expectations. Also set should accept maxAge for API parity.
Apply:
async get(key) {
- return sessions[key];
+ return sessions[key] ?? null;
},
- async set(key, value) {
- sessions[key] = value;
+ async set(key, value, _maxAge) {
+ sessions[key] = value;
},
async destroy(key) {
- sessions[key] = undefined;
+ delete sessions[key];
},Optional: clear residual state between test boots to avoid cross-test bleed.
async willReady() {
+ for (const k of Object.keys(sessions)) delete sessions[k];
this.app.sessionStore = {📝 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.
| async get(key) { | |
| return sessions[key]; | |
| }, | |
| async set(key, value) { | |
| sessions[key] = value; | |
| }, | |
| async destroy(key) { | |
| sessions[key] = undefined; | |
| }, | |
| async get(key) { | |
| return sessions[key] ?? null; | |
| }, | |
| async set(key, value, _maxAge) { | |
| sessions[key] = value; | |
| }, | |
| async destroy(key) { | |
| delete sessions[key]; | |
| }, |
| async get(key) { | |
| return sessions[key]; | |
| }, | |
| async set(key, value) { | |
| sessions[key] = value; | |
| }, | |
| async destroy(key) { | |
| sessions[key] = undefined; | |
| }, | |
| async willReady() { | |
| for (const k of Object.keys(sessions)) delete sessions[k]; | |
| this.app.sessionStore = { | |
| // … | |
| }; |
🤖 Prompt for AI Agents
In plugins/session/test/fixtures/memory-session/app.js around lines 10–20,
normalize the store API: change get to return null when a key is missing (not
undefined); change set to accept a second optional maxAge parameter and store
the value (and optionally ignore or implement TTL semantics) so the signature
matches other fixtures; change destroy to delete the key from the sessions
object (use delete sessions[key] so it is removed, not set to undefined); and
optionally add or expose a clear/reset helper to wipe the sessions object
between test boots to avoid cross-test state bleed.
| async get(key) { | ||
| const res = await this.app.redis.get(key); | ||
| if (!res) return null; | ||
| return JSON.parse(res); | ||
| } | ||
|
|
||
| async set(key, value, maxAge) { | ||
| value = JSON.stringify(value); | ||
| await this.app.redis.set(key, value, 'PX', maxAge); | ||
| } |
There was a problem hiding this comment.
Guard TTL usage and harden JSON parsing
- set: using 'PX' with undefined/0 maxAge will error; guard it.
- get: JSON.parse can throw on bad data; handle gracefully.
Apply:
async get(key) {
const res = await this.app.redis.get(key);
- if (!res) return null;
- return JSON.parse(res);
+ if (!res) return null;
+ try {
+ return JSON.parse(res);
+ } catch {
+ return null;
+ }
}
async set(key, value, maxAge) {
- value = JSON.stringify(value);
- await this.app.redis.set(key, value, 'PX', maxAge);
+ const str = JSON.stringify(value);
+ if (typeof maxAge === 'number' && maxAge > 0) {
+ await this.app.redis.set(key, str, 'PX', maxAge);
+ } else {
+ await this.app.redis.set(key, str);
+ }
}📝 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.
| async get(key) { | |
| const res = await this.app.redis.get(key); | |
| if (!res) return null; | |
| return JSON.parse(res); | |
| } | |
| async set(key, value, maxAge) { | |
| value = JSON.stringify(value); | |
| await this.app.redis.set(key, value, 'PX', maxAge); | |
| } | |
| async get(key) { | |
| const res = await this.app.redis.get(key); | |
| if (!res) return null; | |
| try { | |
| return JSON.parse(res); | |
| } catch { | |
| return null; | |
| } | |
| } | |
| async set(key, value, maxAge) { | |
| const str = JSON.stringify(value); | |
| if (typeof maxAge === 'number' && maxAge > 0) { | |
| await this.app.redis.set(key, str, 'PX', maxAge); | |
| } else { | |
| await this.app.redis.set(key, str); | |
| } | |
| } |
🤖 Prompt for AI Agents
In plugins/session/test/fixtures/redis-session/app.js around lines 7 to 16, the
set method currently calls this.app.redis.set with 'PX' even when maxAge is
undefined/0 which can error, and the get method uses JSON.parse without handling
malformed data; update set to only pass the 'PX' TTL when maxAge is a positive
number (otherwise call redis.set without TTL or use a safe default), and update
get to wrap JSON.parse in a try/catch that returns null (and optionally removes
the bad key or logs the parse error) instead of letting the parse exception
bubble.
| { | ||
| "name": "samesite-none-session" | ||
| } |
There was a problem hiding this comment.
Enforce the Node 22.18.0 minimum in this fixture package.
All manifests under plugins/** are required to declare the repository’s Node floor via engines.node. Please add the field here to keep the fixture compliant (adding "private": true is also acceptable). As per coding guidelines.
{
"name": "samesite-none-session"
+ "private": true,
+ "engines": {
+ "node": ">=22.18.0"
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: pnpm -r run typecheck failed. TypeScript compilation errors detected across multiple files: Cannot find module '@eggjs/core' (src/app.ts, src/app/extend/application.ts, src/types.ts), implicit any in binding elements (src/app.ts: ctx, key, value), missing type declarations for modules '@eggjs/core' and '@eggjs/supertest', and multiple property/type errors in application.ts and test files. Exit code: 2.
🤖 Prompt for AI Agents
In plugins/session/test/fixtures/samesite-none-session/package.json lines 1-3,
the fixture package.json is missing the repository Node floor; add an "engines"
entry with "node": ">=22.18.0" (or alternatively add "private": true) to comply
with the plugins/** manifest requirement—update the JSON manifest to include
either "engines": { "node": ">=22.18.0" } or "private": true so the fixture
declares the Node minimum.
| koa-onerror: ^5.0.1 | ||
| koa-override: ^4.0.0 | ||
| koa-range: ^0.3.0 | ||
| koa-session: ^7.0.2 |
There was a problem hiding this comment.
🧩 Analysis chain
Missing catalog alias usage
Per our pnpm catalog guideline, every dependency we add to the catalog must be consumed via the "catalog:" alias in package.json files. Please confirm that every reference to koa-session now uses "catalog:koa-session" rather than a semver string (for example in plugins/session/package.json). If any direct version strings remain, change them to the catalog form so that the workspace stays consistent.
Run this to audit the repo and surface any direct koa-session specifiers:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=json '"koa-session":' --glob '!pnpm-lock.yaml'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "🔍 Searching for direct ‘koa-session’ version specifiers in package.json files"
rg -n '"koa-session":' -g '**/package.json'
echo "🔍 Searching for ‘catalog:koa-session’ references in package.json files"
rg -n '"catalog:koa-session"' -g '**/package.json'Length of output: 213
Use full catalog alias for koa-session
In plugins/session/package.json (line 51), change
"koa-session": "catalog:"to
"koa-session": "catalog:koa-session"so it correctly references the catalog entry.
🤖 Prompt for AI Agents
In plugins/session/package.json around line 51 (and note pnpm-workspace.yaml
line 127 references the package), the dependency alias for koa-session is
incomplete ("catalog:"); update the dependency value to the full catalog alias
"catalog:koa-session" so the package correctly resolves the catalog entry;
ensure the package.json string is updated exactly and run a quick install/check
to verify the alias resolves.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #5555 +/- ##
==========================================
+ Coverage 78.70% 78.78% +0.07%
==========================================
Files 170 173 +3
Lines 5142 5165 +23
Branches 786 786
==========================================
+ Hits 4047 4069 +22
- Misses 1024 1025 +1
Partials 71 71 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
plugins/session/test/app/middleware/session.test.ts (1)
12-12: Align suite title with filename.Nit: the suite string references .js; this file is .ts.
Apply:
-describe('test/app/middlewares/session.test.js', () => { +describe('test/app/middlewares/session.test.ts', () => {plugins/session/src/app.ts (1)
3-3: Prefer .js extension in ESM import specifiers.With ESM/NodeNext builds, import TS sources using the emitted .js extension to avoid runtime resolution issues. Confirm tsdown rewrites extensions if keeping .ts here.
Apply:
-import { SessionConfig } from './config/config.default.ts'; +import { SessionConfig } from './config/config.default.js';plugins/session/src/app/extend/application.ts (2)
38-39: Fix log message grammar.“will be overwrite” → “will be overwritten”.
Apply:
- this.coreLogger.warn('[@eggjs/session] sessionStore already exists and will be overwrite'); + this.coreLogger.warn('[@eggjs/session] sessionStore already exists and will be overwritten');
9-13: Broaden constructor type for custom stores (optional).If you want to allow constructing stores with the base Egg app type, consider accepting a more generic type than Application to improve compatibility.
Apply:
- | { - new (app: Application): SessionStore; - }; + | { + new (app: Application): SessionStore; + };If you later adopt EggCore in this repo, swap Application for EggCore accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
plugins/session/package.json(1 hunks)plugins/session/src/app.ts(1 hunks)plugins/session/src/app/extend/application.ts(1 hunks)plugins/session/src/config/config.default.ts(1 hunks)plugins/session/src/index.ts(1 hunks)plugins/session/test/app/middleware/session.test.ts(1 hunks)pnpm-workspace.yaml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/session/package.json
- plugins/session/src/index.ts
- plugins/session/src/config/config.default.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
plugins/session/test/app/middleware/session.test.tsplugins/session/src/app.tsplugins/session/src/app/extend/application.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suites
Files:
plugins/session/test/app/middleware/session.test.ts
{packages,plugins,tools,examples}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Enable TypeScript strict mode and follow TS best practices: explicit return types for public APIs, prefer interfaces over type aliases for object shapes, use readonly where appropriate, avoid any and use unknown when truly unknown
Files:
plugins/session/test/app/middleware/session.test.tsplugins/session/src/app.tsplugins/session/src/app/extend/application.ts
{packages,plugins}/**/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Vitest test files must follow the naming pattern test/**/*.test.ts
Files:
plugins/session/test/app/middleware/session.test.ts
{packages,plugins,tools}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should import test functions from vitest when using Vitest: import { describe, it } from 'vitest'
Files:
plugins/session/test/app/middleware/session.test.tsplugins/session/src/app.tsplugins/session/src/app/extend/application.ts
plugins/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
New plugins should use TypeScript sources in src and publish unbundled ESM output via tsdown
Files:
plugins/session/src/app.tsplugins/session/src/app/extend/application.ts
pnpm-workspace.yaml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Define shared dependency versions in pnpm-workspace.yaml catalog
Files:
pnpm-workspace.yaml
🧠 Learnings (8)
📚 Learning: 2025-09-27T09:43:43.746Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to packages/cookies/test/**/*.test.ts : cookies package tests use Mocha with test/**/*.test.ts pattern
Applied to files:
plugins/session/test/app/middleware/session.test.ts
📚 Learning: 2025-09-27T09:43:43.746Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin package tests use Mocha with test/**/*.test.ts pattern
Applied to files:
plugins/session/test/app/middleware/session.test.tspnpm-workspace.yaml
📚 Learning: 2025-09-27T09:43:43.746Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to {packages,plugins,tools}/**/*.ts : Test files should import test functions from vitest when using Vitest: import { describe, it } from 'vitest'
Applied to files:
plugins/session/test/app/middleware/session.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: For features affecting HTTP or process orchestration, add regression tests that exercise both the CommonJS and TypeScript example apps
Applied to files:
plugins/session/test/app/middleware/session.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to pnpm-workspace.yaml : Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-27T09:43:43.746Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to pnpm-workspace.yaml : Define shared dependency versions in pnpm-workspace.yaml catalog
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-27T09:43:43.746Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to {packages,plugins,tools,examples,site}/**/package.json : Use "catalog:" specifiers for external dependencies defined in the pnpm catalog
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-27T09:43:43.746Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to plugins/**/package.json : All plugins must include egg in peerDependencies as "egg": "workspace:*"
Applied to files:
pnpm-workspace.yaml
🧬 Code graph analysis (3)
plugins/session/test/app/middleware/session.test.ts (2)
packages/egg/test/utils.ts (2)
MockApplication(24-24)mm(23-23)packages/supertest/src/test.ts (1)
expect(77-132)
plugins/session/src/app.ts (1)
plugins/session/src/config/config.default.ts (2)
SessionConfig(4-6)SessionConfig(8-8)
plugins/session/src/app/extend/application.ts (1)
plugins/session/src/config/config.default.ts (2)
SessionConfig(4-6)SessionConfig(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: Test (ubuntu-latest, 22, packages)
- GitHub Check: Test (windows-latest, 24, plugins)
- GitHub Check: Test (windows-latest, 22, tools)
- GitHub Check: Test (windows-latest, 24, packages)
- GitHub Check: Test (windows-latest, 22, packages)
- GitHub Check: Test (ubuntu-latest, 22, plugins)
- GitHub Check: Test (ubuntu-latest, 24, plugins)
- GitHub Check: Test (windows-latest, 24, tools)
- GitHub Check: Test (macos-latest, 24, packages)
- GitHub Check: Test (windows-latest, 22, plugins)
- GitHub Check: Test (macos-latest, 22, plugins)
- GitHub Check: Test (ubuntu-latest, 24, packages)
- GitHub Check: Test egg-bin (macos-latest, 22)
- GitHub Check: Test egg-bin (macos-latest, 24)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
🔇 Additional comments (4)
plugins/session/test/app/middleware/session.test.ts (3)
205-205: Fix test description typo.Use “maxAge=session”.
Apply:
- it('should work with maxage=ession', async () => { + it('should work with maxAge=session', async () => {
4-4: Fix broken @eggjs/supertest import and agent construction (use supertest).The package '@eggjs/supertest' doesn’t exist in the workspace; this breaks typecheck/CI. Use the published/workspace 'supertest' and its Agent type, and construct the agent via supertest.agent(app.callback()).
Apply:
-import { request, TestAgent } from '@eggjs/supertest'; +import supertest, { type SuperAgentTest } from 'supertest'; @@ - let agent: TestAgent; + let agent: SuperAgentTest; @@ - agent = new TestAgent(app.callback()); + agent = supertest.agent(app.callback()); @@ - agent = new TestAgent(app.callback()); + agent = supertest.agent(app.callback()); @@ - agent = new TestAgent(app.callback()); + agent = supertest.agent(app.callback()); @@ - agent = new TestAgent(app.callback()); + agent = supertest.agent(app.callback()); @@ - agent = new TestAgent(app.callback()); + agent = supertest.agent(app.callback()); @@ - await request(app.callback()).get('/get').set('cookie', cookie).expect(200).expect({}); + await supertest(app.callback()).get('/get').set('cookie', cookie).expect(200).expect({}); @@ - await request(app.callback()).get('/get').set('cookie', cookie).expect(200).expect({}); + await supertest(app.callback()).get('/get').set('cookie', cookie).expect(200).expect({}); @@ - await request(app.callback()).get('/get').set('cookie', cookie).expect(200).expect({}); + await supertest(app.callback()).get('/get').set('cookie', cookie).expect(200).expect({});#!/bin/bash # Verify no remaining bad imports/usages rg -n -C2 "@eggjs/supertest|new\s+TestAgent|[^a-zA-Z]request\s*\(" --type=tsAlso applies to: 14-14, 23-24, 82-83, 109-110, 136-137, 201-202, 247-248, 155-156, 173-174, 341-342
123-126: Replace console.log with concrete CHIPS assertions.Logging won’t fail the test if attributes regress. Assert Partitioned (and optionally Priority=High) on Set-Cookie.
Apply:
- const cookies = res.headers['set-cookie']; - console.log(cookies); + const cookies = res.headers['set-cookie'] ?? []; + expect(cookies.length).toBeGreaterThan(0); + const cookie = cookies.join('|').toLowerCase(); + expect(cookie).toContain('partitioned'); + // optionally enforce cookie priority if set by implementation + // expect(cookie).toContain('priority=high');pnpm-workspace.yaml (1)
8-201: Ensure packages use the catalog alias for koa-session.Per workspace catalog rules, dependencies must reference “catalog:koa-session” in package.json. Verify no packages use a bare version or an incomplete “catalog:” value.
As per coding guidelines
#!/bin/bash # Find any non-catalog or malformed catalog references to koa-session rg -n --glob '!pnpm-lock.yaml' -g '**/package.json' '"koa-session":\s*"(?!catalog:koa-session)[^"]+"' # Show correct catalog references rg -n -g '**/package.json' '"koa-session":\s*"catalog:koa-session"'
| @@ -0,0 +1,33 @@ | |||
| import type { ILifecycleBoot, Application } from 'egg'; | |||
There was a problem hiding this comment.
Add explicit typings and event payload types (fixes implicit any, TS strict).
Annotate the app field, method return type, and listener params to satisfy strict mode and avoid CI type errors.
Apply:
-import type { ILifecycleBoot, Application } from 'egg';
+import type { ILifecycleBoot, Application, Context } from 'egg';
@@
export default class AppBoot implements ILifecycleBoot {
- private readonly app;
+ private readonly app: Application;
@@
- configWillLoad() {
+ configWillLoad(): void {
@@
- app.on('session:missed', ({ ctx, key }) => {
+ app.on('session:missed', ({ ctx, key }: { ctx: Context; key: string }) => {
ctx.coreLogger.warn('[session][missed] key(%s)', key);
});
- app.on('session:expired', ({ ctx, key, value }) => {
+ app.on(
+ 'session:expired',
+ ({ ctx, key, value }: { ctx: Context; key: string; value: unknown }) => {
ctx.coreLogger.warn('[session][expired] key(%s) value(%j)', key, app.config.session.logValue ? value : '');
- });
- app.on('session:invalid', ({ ctx, key, value }) => {
+ });
+ app.on(
+ 'session:invalid',
+ ({ ctx, key, value }: { ctx: Context; key: string; value: unknown }) => {
ctx.coreLogger.warn('[session][invalid] key(%s) value(%j)', key, app.config.session.logValue ? value : '');
- });
+ });Also applies to: 6-6, 12-12, 23-31
🤖 Prompt for AI Agents
In plugins/session/src/app.ts at lines 1, 6, 12 and 23-31, the code uses
implicit any types causing TS strict failures; explicitly type the exported app
variable (annotate as Application), annotate ILifecycleBoot methods with correct
return types (e.g., Promise<void> or void per implementation), and add explicit
types for lifecycle listener parameters and event payloads (use appropriate
types from egg or custom payload interfaces). Update imports if needed to bring
in the Application and any payload interfaces, and ensure each function
signature and field has an explicit type to satisfy strict mode.
| it('should get empty session and do not set cookie when session not populated', async () => { | ||
| await agent | ||
| .get('/get') | ||
| .expect(200) | ||
| .expect({}) | ||
| .expect(res => { | ||
| expect( | ||
| !res | ||
| .get('Set-Cookie')! | ||
| .join('') | ||
| .match(/EGG_SESS/) | ||
| ).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Fix assertion when cookie is intentionally absent (avoids TypeError).
res.get('Set-Cookie') can be undefined; calling .join() on it will throw. Assert undefined explicitly.
Apply:
- .expect(res => {
- expect(
- !res
- .get('Set-Cookie')!
- .join('')
- .match(/EGG_SESS/)
- ).toBe(true);
- });
+ .expect(res => {
+ const cookies = res.get('Set-Cookie');
+ expect(cookies).toBeUndefined();
+ });📝 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.
| it('should get empty session and do not set cookie when session not populated', async () => { | |
| await agent | |
| .get('/get') | |
| .expect(200) | |
| .expect({}) | |
| .expect(res => { | |
| expect( | |
| !res | |
| .get('Set-Cookie')! | |
| .join('') | |
| .match(/EGG_SESS/) | |
| ).toBe(true); | |
| }); | |
| }); | |
| it('should get empty session and do not set cookie when session not populated', async () => { | |
| await agent | |
| .get('/get') | |
| .expect(200) | |
| .expect({}) | |
| .expect(res => { | |
| const cookies = res.get('Set-Cookie'); | |
| expect(cookies).toBeUndefined(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In plugins/session/test/app/middleware/session.test.ts around lines 265 to 278,
the test assumes res.get('Set-Cookie') is always defined and calls .join(),
which will throw when the header is intentionally absent; change the assertion
to explicitly assert that res.get('Set-Cookie') is undefined (or falsy) rather
than joining and matching, e.g., retrieve the header once, assert it is
undefined/null or assert !header, or if you need to keep type narrow, check
Array.isArray(header) before joining and matching.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/egg/test/cluster1/app_worker.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
packages/**/test/**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/test/**/*.test.ts: Name test files as test/**/*.test.ts and run them with Vitest
Use import { describe, it } from 'vitest' in tests
Use Node.js built-in assert module for test assertions
Files:
packages/egg/test/cluster1/app_worker.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
packages/egg/test/cluster1/app_worker.test.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suites
Files:
packages/egg/test/cluster1/app_worker.test.ts
{packages,plugins,tools,examples}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Enable TypeScript strict mode and follow TS best practices: explicit return types for public APIs, prefer interfaces over type aliases for object shapes, use readonly where appropriate, avoid any and use unknown when truly unknown
Files:
packages/egg/test/cluster1/app_worker.test.ts
{packages,plugins}/**/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Vitest test files must follow the naming pattern test/**/*.test.ts
Files:
packages/egg/test/cluster1/app_worker.test.ts
{packages,plugins,tools}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should import test functions from vitest when using Vitest: import { describe, it } from 'vitest'
Files:
packages/egg/test/cluster1/app_worker.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to plugins/**/package.json : All plugins must include egg in peerDependencies as "egg": "workspace:*"
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin package tests use Mocha with test/**/*.test.ts pattern
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to plugins/**/package.json : Plugin package.json should include an eggPlugin configuration field
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Built-in framework configurations must live under packages/egg/src/config/ with plugin.ts and config.default.ts present
📚 Learning: 2025-09-27T09:43:43.746Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-27T09:43:43.746Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin package tests use Mocha with test/**/*.test.ts pattern
Applied to files:
packages/egg/test/cluster1/app_worker.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Test (windows-latest, 24, plugins)
- GitHub Check: Test (macos-latest, 24, plugins)
- GitHub Check: Test (windows-latest, 22, plugins)
- GitHub Check: Test (macos-latest, 22, plugins)
- GitHub Check: Test egg-bin (macos-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (macos-latest, 24)
- GitHub Check: Cloudflare Pages: egg-v3
| // FIXME: unsable | ||
| it.skip('should start cluster success and app worker emit `server` event', async () => { | ||
| await app.httpRequest().get('/').expect('true'); | ||
| }); |
There was a problem hiding this comment.
Don't silently skip this regression test
Line 28 used to verify that clustered apps emit the server event; skipping it removes coverage for a core boot path and can let regressions ship unnoticed. If the case is flaky, please either fix the underlying issue or gate it with a documented it.skipIf(...) (linking a tracking issue / root cause). Simply skipping it with a vague FIXME (and a typo) isn’t an acceptable long-term solution.
🤖 Prompt for AI Agents
packages/egg/test/cluster1/app_worker.test.ts lines 27-30: the test is currently
silently skipped with a vague FIXME and typo ("unsable"), which removes coverage
for verifying clustered apps emit the `server` event; either restore the test by
fixing its flakiness or explicitly gate it with a documented conditional skip.
To fix: investigate and remove the root cause of flakiness (ensure deterministic
cluster startup, wait for the proper event or use stable synchronization),
re-enable the test (remove .skip) once reliable; if it cannot be immediately
stabilized, replace it with a clear conditional skip such as
it.skipIf(condition, 'reason and tracking-issue-url') or add a comment linking a
tracking issue and include retries/timeouts so the intent and cause are
documented and the test won't be silently ignored.
Merge @eggjs/view plugin from https://github.com/eggjs/view into monorepo at plugins/view/. Updated to version 4.0.0-beta.15 with standardized build configuration and dependencies. - Migrate from ESLint to oxlint for linting - Standardize with tsdown for unbundled builds - Update tests to use Vitest - Convert dependencies to workspace:* and catalog: - Add comprehensive export paths in package.json 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 participant
Merge @eggjs/session plugin into the monorepo following the standard plugin structure.
🤖 Generated with Claude Code
Summary by CodeRabbit