Merge branch 'master' into imprv/unchanged-revision

.claude/skills/learned/essential-test-design/SKILL.md

@@ -0,0 +1,122 @@
+---
+name: essential-test-design
+description: Write tests that verify observable behavior (contract), not implementation details. Auto-invoked when writing or reviewing tests.
+---
+
+## Problem
+
+Tests that are tightly coupled to implementation details cause two failures:
+
+1. **False positives** — Tests pass even when behavior is broken (e.g., delay shortened but test still passes because it only checks `setTimeout` was called)
+2. **False negatives** — Tests fail even when behavior is correct (e.g., implementation switches from `setTimeout` to a `delay()` utility, spy breaks)
+
+Both undermine the purpose of testing: detecting regressions in behavior.
+
+## Principle: Test the Contract, Not the Mechanism
+
+A test is "essential" when it:
+- **Fails if the behavior degrades** (catches real bugs)
+- **Passes if the behavior is preserved** (survives refactoring)
+- **Does not depend on how the behavior is implemented** (implementation-agnostic)
+
+Ask: "What does the caller of this function experience?" — test that.
+
+## Anti-Patterns and Corrections
+
+### Anti-Pattern 1: Implementation Spy
+
+```typescript
+// BAD: Tests implementation, not behavior
+// Breaks if implementation changes from setTimeout to any other delay mechanism
+const spy = vi.spyOn(global, 'setTimeout');
+await exponentialBackoff(1);
+expect(spy).toHaveBeenCalledWith(expect.any(Function), 1000);
+```
+
+### Anti-Pattern 2: Arrange That Serves the Assert
+
+```typescript
+// BAD: The "arrange" is set up only to make the "assert" trivially pass
+// This is a self-fulfilling prophecy, not a meaningful test
+vi.advanceTimersByTime(1000);
+await promise;
+// No assertion — "it didn't throw" is not a valuable test
+```
+
+### Correct: Behavior Boundary Test
+
+```typescript
+// GOOD: Tests the observable contract
+// "Does not resolve before the expected delay, resolves at the expected delay"
+let resolved = false;
+mailService.exponentialBackoff(1).then(() => { resolved = true });
+
+await vi.advanceTimersByTimeAsync(999);
+expect(resolved).toBe(false);  // Catches: delay too short
+
+await vi.advanceTimersByTimeAsync(1);
+expect(resolved).toBe(true);   // Catches: delay too long or hangs
+```
+
+## Decision Framework
+
+When writing a test, ask these questions in order:
+
+1. **What is the contract?** — What does the caller expect to experience?
+   - e.g., "Wait for N ms before resolving"
+2. **What breakage should this test catch?** — Define the regression scenario
+   - e.g., "Someone changes the delay from 1000ms to 500ms"
+3. **Would this test still pass if I refactored the internals?** — If no, you're testing implementation
+   - e.g., Switching from `setTimeout` to `Bun.sleep()` shouldn't break the test
+4. **Would this test fail if the behavior degraded?** — If no, the test has no value
+   - e.g., If delay is halved, `expect(resolved).toBe(false)` at 999ms would catch it
+
+## Common Scenarios
+
+### Async Delay / Throttle / Debounce
+
+Use fake timers + boundary assertions (as shown above).
+
+### Data Transformation
+
+Assert on output shape/values, not on which internal helper was called.
+
+```typescript
+// BAD
+const spy = vi.spyOn(utils, 'formatDate');
+transform(input);
+expect(spy).toHaveBeenCalled();
+
+// GOOD
+const result = transform(input);
+expect(result.date).toBe('2026-01-01');
+```
+
+### Side Effects (API calls, DB writes)
+
+Mocking the boundary (API/DB) is acceptable — that IS the observable behavior.
+
+```typescript
+// OK: The contract IS "sends an email via mailer"
+expect(mockMailer.sendMail).toHaveBeenCalledWith(
+  expect.objectContaining({ to: 'user@example.com' })
+);
+```
+
+### Retry Logic
+
+Test the number of attempts and the final outcome, not the internal flow.
+
+```typescript
+// GOOD: Contract = "retries N times, then fails with specific error"
+mockMailer.sendMail.mockRejectedValue(new Error('fail'));
+await expect(sendWithRetry(config, 3)).rejects.toThrow('failed after 3 attempts');
+expect(mockMailer.sendMail).toHaveBeenCalledTimes(3);
+```
+
+## When to Apply
+
+- Writing new test cases for any function or method
+- Reviewing existing tests for flakiness or brittleness
+- Refactoring tests after fixing flaky CI failures
+- Code review of test pull requests

.claude/skills/testing-patterns-with-vitest/SKILL.md → .claude/skills/learned/essential-test-patterns/SKILL.md

@@ -1,7 +1,6 @@
 ---
-name: testing-patterns-with-vitest
-description: GROWI testing patterns with Vitest, React Testing Library, and vitest-mock-extended. Auto-invoked for all GROWI development work.
-user-invocable: false
+name: essential-test-patterns
+description: GROWI testing patterns with Vitest, React Testing Library, and vitest-mock-extended.
 ---
 
 # GROWI Testing Patterns
@@ -415,12 +414,43 @@ Before committing tests, ensure:
 
 ## Running Tests
 
+### From Monorepo Root (Recommended)
+
 ```bash
-# Run all tests for a package
+# Run all tests for a specific package
 turbo run test --filter @growi/app
+turbo run test --filter @growi/core
+
+# Or with Turborepo caching
+pnpm run test --filter @growi/app
+```
+
+### From Package Directory
+
+```bash
+# Run all tests
+pnpm vitest run
 
-# Run specific test file
-cd {package_dir} && pnpm vitest run src/components/Button/Button.spec.tsx
+# Run specific test file (use partial file name)
+pnpm vitest run yjs.integ
+pnpm vitest run helper.spec
+pnpm vitest run Button.spec
+
+# Run tests matching a pattern
+pnpm vitest run PageService
+```
+
+**File pattern tips**:
+- Use **partial file name** - Vitest automatically finds matching files
+- No need for `src/` prefix or full path
+- No need for `--project` flag - Vitest auto-detects based on file extension
+- Works across all packages (apps/app, packages/core, etc.)
+
+### Running Multiple Times (Flaky Test Detection)
+
+```bash
+# Repeat test execution to verify stability
+pnpm vitest run yjs.integ --repeat=10
 ```
 
 ## Summary: GROWI Testing Philosophy

.kiro/steering/tdd.md

@@ -1,6 +1,6 @@
 # Test-Driven Development
 
-See: `.claude/commands/tdd.md` and `.claude/skills/testing-patterns-with-vitest/SKILL.md` (auto-loaded by Claude Code)
+See: `.claude/commands/tdd.md`, `.claude/skills/learned/essential-test-patterns/SKILL.md` and `.claude/skills/learned/essential-test-design/SKILL.md`
 
 ## cc-sdd Specific Notes
 

AGENTS.md

@@ -24,7 +24,6 @@ Technical information is available in **Claude Code Skills** (`.claude/skills/`)
 |-------|-------------|
 | **monorepo-overview** | Monorepo structure, workspace organization, Changeset versioning |
 | **tech-stack** | Technology stack, pnpm/Turborepo, TypeScript, Biome |
-| **testing-patterns-with-vitest** | Vitest, React Testing Library, vitest-mock-extended |
 
 **Rules** (always applied):
 

apps/app/.claude/skills/app-specific-patterns/SKILL.md

@@ -6,7 +6,7 @@ user-invocable: false
 
 # App Specific Patterns (apps/app)
 
-For general testing patterns, see the global `testing-patterns-with-vitest` skill.
+For general testing patterns, see the global `learned/essential-test-patterns` and `learned/essential-test-design` skills.
 
 ## Next.js Pages Router
 

apps/app/AGENTS.md

@@ -29,6 +29,10 @@ pnpm run test                   # Run tests
 
 # Build
 pnpm run build                  # Build for production
+
+# Run Specific Tests
+pnpm vitest run yjs.integ       # Use partial file name
+pnpm vitest run helper.spec     # Vitest auto-finds matching files
 ```
 
 ### Key Directories
@@ -150,7 +154,7 @@ When working in this directory, these skills are automatically loaded:
 - **app-commands** - apps/app specific commands (migrations, OpenAPI, etc.)
 - **app-specific-patterns** - Jotai/SWR/Next.js patterns, testing
 
-Plus all global skills (monorepo-overview, tech-stack, testing-patterns-with-vitest).
+Plus all global skills (monorepo-overview, tech-stack).
 
 ---
 

apps/app/src/server/service/yjs/yjs.integ.ts

@@ -1,5 +1,5 @@
 import { YDocStatus } from '@growi/core/dist/consts';
-import { Types } from 'mongoose';
+import mongoose, { Types } from 'mongoose';
 import type { Server } from 'socket.io';
 import { mock } from 'vitest-mock-extended';
 
@@ -33,6 +33,10 @@ describe('YjsService', () => {
 
       // initialize
       initializeYjsService(ioMock);
+
+      // Wait for index creation to complete to avoid race condition
+      const collection = mongoose.connection.collection('yjs-writings');
+      await collection.listIndexes().toArray();
     });
 
     afterAll(async () => {
@@ -42,7 +46,15 @@ describe('YjsService', () => {
       // flush yjs-writings
       const yjsService = getYjsService();
       const privateMdb = getPrivateMdbInstance(yjsService);
-      await privateMdb.flushDB();
+      try {
+        await privateMdb.flushDB();
+      } catch (error) {
+        // Ignore IndexBuildAborted error (code: 276) which can occur in CI
+        // when cleanup happens while index creation is still in progress
+        if (error.code !== 276) {
+          throw error;
+        }
+      }
     });
 
     it('returns ISOLATED when neither revisions nor YDocs exists', async () => {