Merge branch 'master' into copilot/implement-email-sending-functionality

.claude/commands/kiro/spec-cleanup.md

@@ -0,0 +1,277 @@
+---
+description: Organize and clean up specification documents after implementation completion
+allowed-tools: Bash, Glob, Grep, Read, Write, Edit, MultiEdit, Update
+argument-hint: <feature-name>
+---
+
+# Specification Cleanup
+
+<background_information>
+- **Mission**: Organize specification documents after implementation completion, removing implementation details while preserving essential context for future refactoring
+- **Success Criteria**:
+  - Implementation details (testing procedures, deployment checklists) removed
+  - Design decisions and constraints preserved in research.md and design.md
+  - Requirements simplified (Acceptance Criteria condensed to summaries)
+  - Unimplemented features removed or documented
+  - Documents remain valuable for future refactoring work
+</background_information>
+
+<instructions>
+## Core Task
+Clean up and organize specification documents for feature **$1** after implementation is complete.
+
+## Organizing Principle
+
+**"Can we read essential context from these spec documents when refactoring this feature months later?"**
+
+- **Keep**: "Why" (design decisions, architectural constraints, limitations, trade-offs)
+- **Remove**: "How" (testing procedures, deployment steps, detailed implementation examples)
+
+## Execution Steps
+
+### Step 1: Load Context
+
+**Discover all spec files**:
+- Use Glob to find all files in `.kiro/specs/$1/` directory
+- Categorize files:
+  - **Core files** (must preserve): `spec.json`, `requirements.md`, `design.md`, `tasks.md`, `research.md`
+  - **Other files** (evaluate case-by-case): validation reports, notes, prototypes, migration guides, etc.
+
+**Read all discovered files**:
+- Read all core files first
+- Read other files to understand their content and value
+
+**Verify implementation status**:
+- Check that tasks are marked complete `[x]` in tasks.md
+- If implementation incomplete, warn user and ask to confirm cleanup
+
+### Step 2: Analyze Current State
+
+**Identify cleanup opportunities**:
+
+1. **Other files** (non-core files like validation-report.md, notes.md, etc.):
+   - Read each file to understand its content and purpose
+   - Identify valuable information that should be preserved:
+     * Implementation discoveries and lessons learned
+     * Critical constraints or design decisions
+     * Historical context for future refactoring
+   - Determine salvage strategy:
+     * Migrate valuable content to research.md or design.md
+     * Keep file if it contains essential reference information
+     * Delete if content is redundant or no longer relevant
+   - **Case-by-case evaluation required** - never assume files should be deleted
+
+2. **research.md**:
+   - Should contain production discoveries and implementation lessons learned
+   - Check if implementation revealed new constraints or patterns to document
+   - Identify content from other files that should be migrated here
+
+3. **requirements.md**:
+   - Identify verbose Acceptance Criteria that can be condensed to summaries
+   - Find unimplemented requirements (compare with tasks.md)
+   - Detect duplicate or redundant content
+
+4. **design.md**:
+   - Identify implementation-specific sections that can be removed:
+     * Detailed Testing Strategy (test procedures)
+     * Security Considerations (if covered in implementation)
+     * Error Handling code examples (if implemented)
+     * Migration Strategy (after migration complete)
+     * Deployment Checklist (after deployment)
+   - Identify sections to preserve:
+     * Architecture diagrams (essential for understanding)
+     * Component interfaces (API contracts)
+     * Design decisions and rationale
+     * Critical implementation constraints
+     * Known limitations
+   - Check if content from other files should be migrated here
+
+### Step 3: Interactive Confirmation
+
+**Present cleanup plan to user**:
+
+For each file and section identified in Step 2, ask:
+- "Should I delete/simplify/keep/salvage this section?"
+- Provide recommendations based on organizing principle
+- Show brief preview of content to aid decision
+
+**Example questions for other files**:
+- "validation-report.md found. Contains {brief summary}. Options:"
+  - "A: Migrate valuable content to research.md, then delete"
+  - "B: Keep as historical reference"
+  - "C: Delete (content no longer needed)"
+- "notes.md found. Contains {brief summary}. Salvage to research.md before deleting? [Y/n]"
+
+**Example questions for core files**:
+- "research.md: Add 'Session N: Production Discoveries' section to document implementation lessons? [Y/n]"
+- "requirements.md: Simplify Acceptance Criteria from detailed bullet points to summary paragraphs? [Y/n]"
+- "requirements.md: Remove unimplemented requirements (e.g., Req 4.4 field masking not implemented)? [Y/n]"
+- "design.md: Delete 'Testing Strategy' section (lines X-Y)? [Y/n]"
+- "design.md: Delete 'Security Considerations' section (lines X-Y)? [Y/n]"
+- "design.md: Keep Architecture diagrams (essential for refactoring)? [Y/n]"
+
+**Batch similar decisions**:
+- Group related sections (e.g., all "delete implementation details" decisions)
+- Allow user to approve categories rather than individual items
+- Present file-by-file salvage decisions for other files
+
+### Step 4: Execute Cleanup
+
+**For each approved action**:
+
+1. **Salvage and cleanup other files** (if approved):
+   - For each non-core file (validation-report.md, notes.md, etc.):
+     * Extract valuable information (implementation lessons, constraints, decisions)
+     * Migrate content to appropriate core file:
+       - Technical discoveries → research.md
+       - Design constraints → design.md
+       - Requirement clarifications → requirements.md
+     * Delete file after salvage (if approved)
+   - Document salvaged content with source reference (e.g., "From validation-report.md:")
+
+2. **Update research.md** (if new discoveries or salvaged content):
+   - Add new section "Session N: Production Implementation Discoveries" (if needed)
+   - Document critical technical constraints discovered during implementation
+   - Include code examples for critical patterns (e.g., falsy checks, credential preservation)
+   - Integrate salvaged content from other files
+   - Cross-reference requirements.md and design.md where relevant
+
+3. **Simplify requirements.md** (if approved):
+   - Transform detailed Acceptance Criteria into summary paragraphs
+   - Remove unimplemented requirements entirely
+   - Preserve requirement objectives and summaries
+   - Example transformation:
+     ```
+     Before: "1. System shall X... 2. System shall Y... [7 criteria]"
+     After: "**Summary**: System provides X and Y. Configuration includes..."
+     ```
+
+4. **Clean up design.md** (if approved):
+   - Delete approved sections (Testing Strategy, Security Considerations, etc.)
+   - Add "Critical Implementation Constraints" section if implementation revealed new constraints
+   - Integrate salvaged content from other files (if relevant)
+   - Preserve architecture diagrams and component interfaces
+   - Keep design decisions and rationale sections
+
+5. **Update spec.json metadata**:
+   - Set `phase: "implementation-complete"` (if not already set)
+   - Add `cleanup_completed: true` flag
+   - Update `updated_at` timestamp
+
+### Step 5: Generate Cleanup Summary
+
+**Provide summary report**:
+- List of files modified/deleted
+- Sections removed and lines saved
+- Critical information preserved
+- Recommendations for future refactoring
+
+**Format**:
+```markdown
+## Cleanup Summary for {feature-name}
+
+### Files Modified
+- ✅ validation-report.md: Salvaged to research.md, then deleted (730 lines removed)
+- ✅ notes.md: Salvaged to design.md, then deleted (120 lines removed)
+- ✅ research.md: Added Session 2 discoveries + salvaged content (180 lines added)
+- ✅ requirements.md: Simplified 6 requirements (350 lines → 180 lines)
+- ✅ design.md: Removed 4 sections, added constraints + salvaged content (250 lines removed, 100 added)
+
+### Information Salvaged
+- Implementation discoveries from validation-report.md → research.md
+- Design notes from notes.md → design.md
+- Historical context preserved with source attribution
+
+### Information Preserved
+- Architecture diagrams and component interfaces
+- Design decisions and rationale
+- Critical implementation constraints
+- Known limitations and trade-offs
+
+### Next Steps
+- Spec documents ready for future refactoring reference
+- Consider creating knowledge base entry if pattern is reusable
+```
+
+## Critical Constraints
+
+- **User approval required**: Never delete content without explicit confirmation
+- **Language consistency**: Use language specified in spec.json for all updates
+- **Preserve history**: Don't delete discovery rationale or design decisions
+- **Balance brevity with completeness**: Remove redundancy but keep essential context
+- **Interactive workflow**: Pause for user input rather than making assumptions
+
+## Tool Guidance
+
+- **Glob**: Discover all files in `.kiro/specs/{feature}/` directory
+- **Read**: Load all discovered files for analysis
+- **Grep**: Search for patterns (e.g., unimplemented requirements, completed tasks)
+- **Edit/Write**: Update files based on approved changes, salvage content
+- **Bash**: Delete files after salvage (if approved)
+- **MultiEdit**: For batch edits across multiple sections
+
+## Output Description
+
+Provide cleanup plan and execution report in the language specified in spec.json.
+
+**Report Structure**:
+1. **Current State Analysis**: What needs cleanup and why
+2. **Cleanup Plan**: Proposed changes with recommendations
+3. **Confirmation Prompts**: Interactive questions for user approval
+4. **Execution Summary**: What was changed and why
+5. **Preserved Context**: What critical information remains for future refactoring
+
+**Format**: Clear, scannable format with sections and bullet points
+
+## Safety & Fallback
+
+### Error Scenarios
+
+**Implementation Incomplete**:
+- **Condition**: Less than 90% of tasks marked `[x]` in tasks.md
+- **Action**: Warn user: "Implementation appears incomplete (X/Y tasks done). Continue cleanup? [y/N]"
+- **Recommendation**: Wait until implementation complete before cleanup
+
+**Spec Not Found**:
+- **Message**: "No spec found for `$1`. Check available specs in `.kiro/specs/`"
+- **Action**: List available spec directories
+
+**Missing Critical Files**:
+- **Condition**: requirements.md or design.md missing
+- **Action**: Skip cleanup for missing files, proceed with available files
+- **Warning**: "requirements.md missing - cannot simplify requirements"
+
+### Dry Run Mode (Future Enhancement)
+
+**If `-n` or `--dry-run` flag provided**:
+- Show cleanup plan without executing changes
+- Allow user to review before committing to cleanup
+
+### Backup Recommendation
+
+**Before cleanup**:
+- Recommend user create git commit or backup
+- Warning: "This will modify spec files. Commit current state first? [Y/n]"
+
+### Undo Support
+
+**If cleanup goes wrong**:
+- Use git to restore previous state: `git checkout HEAD -- .kiro/specs/{feature}/`
+- Remind user to commit before cleanup for easy rollback
+
+## Example Usage
+
+```bash
+# Basic cleanup after implementation
+/kiro:spec-cleanup oauth2-email-support
+
+# With conversation context about implementation discoveries
+# Command will prompt for Session N discoveries to document
+/kiro:spec-cleanup user-authentication
+```
+
+## Related Commands
+
+- `/kiro:spec-impl {feature}` - Implement tasks (run before cleanup)
+- `/kiro:validate-impl {feature}` - Validate implementation (run before cleanup)
+- `/kiro:spec-status {feature}` - Check implementation status

.claude/commands/learn.md

@@ -25,7 +25,18 @@ Focus on four key areas:
 
 ## Skill File Structure
 
-Extracted patterns are saved in `.claude/skills/learned/{topic-name}/SKILL.md` with:
+Extracted patterns are saved in **appropriate skill directories** based on the scope of the pattern:
+
+**Workspace-specific patterns**:
+- `apps/{workspace}/.claude/skills/learned/{topic-name}/SKILL.md`
+- `packages/{package}/.claude/skills/learned/{topic-name}/SKILL.md`
+- Examples: patterns specific to a single app or package
+
+**Global patterns** (monorepo-wide):
+- `.claude/skills/learned/{topic-name}/SKILL.md`
+- Examples: patterns applicable across all workspaces
+
+### File Template
 
 ```yaml
 ---
@@ -49,12 +60,19 @@ description: Brief description (auto-invoked when working on related code)
 ## GROWI-Specific Examples
 
 Topics commonly learned in GROWI development:
-- `virtualized-tree-patterns` — @headless-tree + @tanstack/react-virtual optimizations
+
+**Apps/app-specific** (`apps/app/.claude/skills/learned/`):
+- `page-save-origin-semantics` — Origin-based conflict detection for collaborative editing
 - `socket-jotai-integration` — Real-time state synchronization patterns
 - `api-v3-error-handling` — RESTful API error response patterns
-- `jotai-atom-composition` — Derived atoms and state composition
 - `mongodb-query-optimization` — Mongoose indexing and aggregation patterns
 
+**Global monorepo patterns** (`.claude/skills/learned/`):
+- `virtualized-tree-patterns` — @headless-tree + @tanstack/react-virtual optimizations (if used across apps)
+- `jotai-atom-composition` — Derived atoms and state composition (if shared pattern)
+- `turborepo-cache-invalidation` — Build cache debugging techniques
+- `pnpm-workspace-dependencies` — Workspace dependency resolution issues
+
 ## Quality Guidelines
 
 **Extract:**
@@ -74,7 +92,25 @@ Topics commonly learned in GROWI development:
 1. User triggers `/learn` after solving a complex problem
 2. Review the session to identify valuable patterns
 3. Draft skill file(s) with clear structure
-4. Save to `.claude/skills/learned/{topic-name}/SKILL.md`
+4. **Autonomously determine the appropriate directory**:
+   - Analyze the pattern's scope (which files/modules were involved)
+   - If pattern is specific to a workspace in `apps/*` or `packages/*`:
+     - Save to `{workspace}/.claude/skills/learned/{topic-name}/SKILL.md`
+   - If pattern is applicable across multiple workspaces:
+     - Save to `.claude/skills/learned/{topic-name}/SKILL.md`
 5. Skills automatically apply in future sessions when working on related code
 
+### Directory Selection Logic
+
+**Workspace-specific** (save to `{workspace}/.claude/skills/learned/`):
+- Pattern involves workspace-specific concepts, models, or APIs
+- References files primarily in one `apps/*` or `packages/*` directory
+- Example: Page save logic in `apps/app` → `apps/app/.claude/skills/learned/`
+
+**Global** (save to `.claude/skills/learned/`):
+- Pattern applies across multiple workspaces
+- Involves monorepo-wide tools (Turborepo, pnpm, Biome, Vitest)
+- Shared coding patterns or architectural principles
+- Example: Turborepo caching pitfall → `.claude/skills/learned/`
+
 Learned skills are automatically invoked based on their description when working on related code.

.claude/skills/learned/.gitkeep

@@ -1,27 +0,0 @@
-# Learned Skills Directory
-
-This directory contains Skills learned from development sessions using the `/learn` command.
-
-Each learned skill is automatically applied when working on related code based on its description.
-
-## Structure
-
-```
-learned/
-├── {topic-name-1}/
-│   └── SKILL.md
-├── {topic-name-2}/
-│   └── SKILL.md
-└── {topic-name-3}/
-    └── SKILL.md
-```
-
-## How Skills Are Created
-
-Use the `/learn` command after completing a feature or solving a complex problem:
-
-```
-/learn
-```
-
-Claude will extract reusable patterns and save them as Skills in this directory.

CLAUDE.md

@@ -33,6 +33,7 @@ Kiro-style Spec Driven Development implementation on AI-DLC (AI Development Life
   - `/kiro:spec-tasks {feature} [-y]`
 - Phase 2 (Implementation): `/kiro:spec-impl {feature} [tasks]`
   - `/kiro:validate-impl {feature}` (optional: after implementation)
+  - `/kiro:spec-cleanup {feature}` (optional: organize specs post-implementation)
 - Progress check: `/kiro:spec-status {feature}` (use anytime)
 
 ## Development Rules

apps/app/.claude/skills/learned/page-save-origin-semantics/SKILL.md

@@ -0,0 +1,302 @@
+---
+name: page-save-origin-semantics
+description: Auto-invoked when modifying origin-based conflict detection, revision validation logic, or isUpdatable() method. Explains the two-stage origin check mechanism for conflict detection and its separation from diff detection.
+---
+
+# Page Save Origin Semantics
+
+## Problem
+
+When modifying page save logic, it's easy to accidentally break the carefully designed origin-based conflict detection system. The system uses a two-stage check mechanism (frontend + backend) to determine when revision validation should be enforced vs. bypassed for collaborative editing (Yjs).
+
+**Key Insight**: **Conflict detection (revision check)** and **diff detection (hasDiffToPrev)** serve different purposes and require separate logic.
+
+## Solution
+
+Understanding the two-stage origin check mechanism:
+
+### Stage 1: Frontend Determines revisionId Requirement
+
+```typescript
+// apps/app/src/client/components/PageEditor/PageEditor.tsx:158
+const isRevisionIdRequiredForPageUpdate = currentPage?.revision?.origin === undefined;
+
+// lines 308-310
+const revisionId = isRevisionIdRequiredForPageUpdate
+  ? currentRevisionId
+  : undefined;
+```
+
+**Logic**: Check the **latest revision's origin** on the page:
+- If `origin === undefined` (legacy/API save) → Send `revisionId`
+- If `origin === "editor"` or `"view"` → Do NOT send `revisionId`
+
+### Stage 2: Backend Determines Conflict Check Behavior
+
+```javascript
+// apps/app/src/server/models/obsolete-page.js:167-172
+const ignoreLatestRevision =
+  origin === Origin.Editor &&
+  (latestRevisionOrigin === Origin.Editor || latestRevisionOrigin === Origin.View);
+
+if (ignoreLatestRevision) {
+  return true;  // Bypass revision check
+}
+
+// Otherwise, enforce strict revision matching
+if (revision != previousRevision) {
+  return false;  // Reject save
+}
+return true;
+```
+
+**Logic**: Check **current request's origin** AND **latest revision's origin**:
+- If `origin === "editor"` AND latest is `"editor"` or `"view"` → Bypass revision check
+- Otherwise → Enforce strict revision ID matching
+
+## Origin Values
+
+Three types of page update methods (called "origin"):
+
+- **`Origin.Editor = "editor"`** - Save from editor mode (collaborative editing via Yjs)
+- **`Origin.View = "view"`** - Save from view mode
+  - Examples: HandsontableModal, DrawioModal editing
+- **`undefined`** - API-based saves or legacy pages
+
+## Origin Strength (強弱)
+
+**Basic Rule**: Page updates require the previous revision ID in the request. If the latest revision doesn't match, the server rejects the request.
+
+**Exception - Editor origin is stronger than View origin**:
+- **UX Goal**: Avoid `Posted param "revisionId" is outdated` errors when multiple members are using the Editor and View changes interrupt them
+- **Special Case**: When the latest revision's origin is View, Editor origin requests can update WITHOUT requiring revision ID
+
+### Origin Strength Matrix
+
+|        | Latest Revision: Editor | Latest Revision: View | Latest Revision: API |
+| ------ | ----------------------- | --------------------- | -------------------- |
+| **Request: Editor** | ⭕️ Bypass revision check | ⭕️ Bypass revision check | ❌ Strict check |
+| **Request: View**   | ❌ Strict check | ❌ Strict check | ❌ Strict check |
+| **Request: API**    | ❌ Strict check | ❌ Strict check | ❌ Strict check |
+
+**Reading the table**:
+- ⭕️ = Revision check bypassed (revisionId not required)
+- ❌ = Strict revision check required (revisionId must match)
+
+## Behavior by Scenario
+
+| Latest Revision Origin | Request Origin | revisionId Sent? | Revision Check | Use Case |
+|------------------------|----------------|------------------|----------------|----------|
+| `editor` or `view` | `editor` | ❌ No | ✅ Bypassed | Normal Editor use (most common) |
+| `undefined` | `editor` | ✅ Yes | ✅ Enforced | Legacy page in Editor |
+| `undefined` | `undefined` (API) | ✅ Yes (required) | ✅ Enforced | API save |
+
+## Example: Server-Side Logic Respecting Origin Semantics
+
+When adding server-side functionality that needs previous revision data:
+
+```typescript
+// ✅ CORRECT: Separate concerns - conflict detection vs. diff detection
+let previousRevision: IRevisionHasId | null = null;
+
+// Priority 1: Use provided revisionId (for conflict detection)
+if (sanitizeRevisionId != null) {
+  previousRevision = await Revision.findById(sanitizeRevisionId);
+}
+
+// Priority 2: Fallback to currentPage.revision (for other purposes like diff detection)
+if (previousRevision == null && currentPage.revision != null) {
+  previousRevision = await Revision.findById(currentPage.revision);
+}
+
+const previousBody = previousRevision?.body ?? null;
+
+// Continue with existing conflict detection logic (unchanged)
+if (currentPage != null && !(await currentPage.isUpdatable(sanitizeRevisionId, origin))) {
+  // ... return conflict error
+}
+
+// Use previousBody for diff detection or other purposes
+updatedPage = await crowi.pageService.updatePage(
+  currentPage,
+  body,
+  previousBody,  // ← Available regardless of conflict detection logic
+  req.user,
+  options,
+);
+```
+
+```typescript
+// ❌ WRONG: Forcing frontend to always send revisionId
+const revisionId = currentRevisionId;  // Always send, regardless of origin
+// This breaks Yjs collaborative editing semantics!
+```
+
+```typescript
+// ❌ WRONG: Changing backend conflict detection logic
+// Don't modify isUpdatable() unless you fully understand the implications
+// for collaborative editing
+```
+
+## When to Apply
+
+**Always consider this pattern when**:
+- Modifying page save/update API handlers
+- Adding functionality that needs previous revision data
+- Working on conflict detection or revision validation logic
+- Implementing features that interact with page history
+- Debugging save operation issues
+
+**Key Principles**:
+1. **Do NOT modify frontend revisionId logic** unless explicitly required for conflict detection
+2. **Do NOT modify isUpdatable() logic** unless fixing conflict detection bugs
+3. **Separate concerns**: Conflict detection ≠ Other revision-based features (diff detection, history, etc.)
+4. **Server-side fallback**: If you need previous revision data when revisionId is not provided, fetch from `currentPage.revision`
+
+## Detailed Scenario Analysis
+
+### Scenario A: Normal Editor Mode (Most Common Case)
+
+**Latest revision has `origin=editor`**:
+
+1. **Frontend Logic**:
+   - `isRevisionIdRequiredForPageUpdate = false` (latest revision origin is not undefined)
+   - Does NOT send `revisionId` in request
+   - Sends `origin: Origin.Editor`
+
+2. **API Layer**:
+   ```typescript
+   previousRevision = await Revision.findById(undefined);  // → null
+   ```
+   Result: No previousRevision fetched via revisionId
+
+3. **Backend Conflict Check** (`isUpdatable`):
+   ```javascript
+   ignoreLatestRevision =
+     (Origin.Editor === Origin.Editor) &&
+     (latestRevisionOrigin === Origin.Editor || latestRevisionOrigin === Origin.View)
+   // → true (latest revision is editor)
+   return true;  // Bypass revision check
+   ```
+   Result: ✅ Save succeeds without revision validation
+
+4. **Impact on Other Features**:
+   - If you need previousRevision data (e.g., for diff detection), it won't be available unless you implement server-side fallback
+   - This is where `currentPage.revision` fallback becomes necessary
+
+### Scenario B: Legacy Page in Editor Mode
+
+**Latest revision has `origin=undefined`**:
+
+1. **Frontend Logic**:
+   - `isRevisionIdRequiredForPageUpdate = true` (latest revision origin is undefined)
+   - Sends `revisionId` in request
+   - Sends `origin: Origin.Editor`
+
+2. **API Layer**:
+   ```typescript
+   previousRevision = await Revision.findById(sanitizeRevisionId);  // → revision object
+   ```
+   Result: previousRevision fetched successfully
+
+3. **Backend Conflict Check** (`isUpdatable`):
+   ```javascript
+   ignoreLatestRevision =
+     (Origin.Editor === Origin.Editor) &&
+     (latestRevisionOrigin === undefined)
+   // → false (latest revision is undefined, not editor/view)
+
+   // Strict revision check
+   if (revision != sanitizeRevisionId) {
+     return false;  // Reject if mismatch
+   }
+   return true;
+   ```
+   Result: ✅ Save succeeds only if revisionId matches
+
+4. **Impact on Other Features**:
+   - previousRevision data is available
+   - All revision-based features work correctly
+
+### Scenario C: API-Based Save
+
+**Request has `origin=undefined` or omitted**:
+
+1. **Frontend**: Not applicable (API client)
+
+2. **API Layer**:
+   - API client MUST send `revisionId` in request
+   - `previousRevision = await Revision.findById(sanitizeRevisionId)`
+
+3. **Backend Conflict Check** (`isUpdatable`):
+   ```javascript
+   ignoreLatestRevision =
+     (undefined === Origin.Editor) && ...
+   // → false
+
+   // Strict revision check
+   if (revision != sanitizeRevisionId) {
+     return false;
+   }
+   return true;
+   ```
+   Result: Strict validation enforced
+
+## Root Cause: Why This Separation Matters
+
+**Historical Context**: At some point, the frontend stopped sending `previousRevision` (revisionId) for certain scenarios to support Yjs collaborative editing. This broke features that relied on previousRevision data being available.
+
+**The Core Issue**:
+- **Conflict detection** needs to know "Is this save conflicting with another user's changes?" (Answered by revision check)
+- **Diff detection** needs to know "Did the content actually change?" (Answered by comparing body)
+- **Current implementation conflates these**: When conflict detection is bypassed, previousRevision is not fetched, breaking diff detection
+
+**The Solution Pattern**:
+```typescript
+// Separate the two concerns:
+
+// 1. Fetch previousRevision for data purposes (diff detection, history, etc.)
+let previousRevision: IRevisionHasId | null = null;
+if (sanitizeRevisionId != null) {
+  previousRevision = await Revision.findById(sanitizeRevisionId);
+} else if (currentPage.revision != null) {
+  previousRevision = await Revision.findById(currentPage.revision);  // Fallback
+}
+
+// 2. Use previousRevision data for your feature
+const previousBody = previousRevision?.body ?? null;
+
+// 3. Conflict detection happens independently via isUpdatable()
+if (currentPage != null && !(await currentPage.isUpdatable(sanitizeRevisionId, origin))) {
+  // Return conflict error
+}
+```
+
+## Reference
+
+**Official Documentation**:
+- https://dev.growi.org/651a6f4a008fee2f99187431#origin-%E3%81%AE%E5%BC%B7%E5%BC%B1
+
+**Related Files**:
+- Frontend: `apps/app/src/client/components/PageEditor/PageEditor.tsx` (lines 158, 240, 308-310)
+- Backend: `apps/app/src/server/models/obsolete-page.js` (lines 159-182, isUpdatable method)
+- API: `apps/app/src/server/routes/apiv3/page/update-page.ts` (lines 260-282, conflict check)
+- Interface: `packages/core/src/interfaces/revision.ts` (lines 6-11, Origin definition)
+
+## Common Pitfalls
+
+1. **Assuming revisionId is always available**: It's not! Editor mode with recent editor/view saves omits it by design.
+2. **Conflating conflict detection with other features**: They serve different purposes and need separate logic.
+3. **Breaking Yjs collaborative editing**: Forcing revisionId to always be sent breaks the bypass mechanism.
+4. **Ignoring origin values**: The system behavior changes significantly based on origin combinations.
+
+## Lessons Learned
+
+This pattern was identified during the "improve-unchanged-revision" feature implementation, where the initial assumption was that frontend should always send `revisionId` for diff detection. Deep analysis revealed:
+
+- The frontend logic is correct for conflict detection and should NOT be changed
+- Server-side fallback is the correct approach to get previous revision data
+- Two-stage checking is intentional and critical for Yjs collaborative editing
+- Conflict detection and diff detection must be separated
+
+**Key Takeaway**: Always understand the existing architectural patterns before proposing changes. What appears to be a "fix" might actually break carefully designed functionality.

apps/app/CLAUDE.md

@@ -1 +1,19 @@
 @AGENTS.md
+
+# apps/app Specific Knowledge
+
+## Critical Architectural Patterns
+
+### Page Save Origin Semantics
+
+**IMPORTANT**: When working on page save, update, or revision operations, always consult the **page-save-origin-semantics** skill for understanding the two-stage origin check mechanism.
+
+**Key Concept**: Origin-based conflict detection uses a two-stage check (frontend + backend) to determine when revision validation should be enforced vs. bypassed for Yjs collaborative editing.
+
+**Critical Rule**: **Conflict detection (revision check)** and **other revision-based features (diff detection, history, etc.)** serve different purposes and require separate logic. Do NOT conflate them.
+
+**Documentation**:
+- Skill (auto-invoked): `.claude/skills/learned/page-save-origin-semantics/SKILL.md`
+
+**Common Pitfall**: Assuming `revisionId` is always available or forcing frontend to always send it will break Yjs collaborative editing.
+

apps/app/src/client/components/PageHistory/PageRevisionTable.tsx

@@ -44,8 +44,8 @@ export const PageRevisionTable = (
 
   const { data, size, error, setSize, isValidating } = swrInifiniteResponse;
 
-  const revisions = data && data[0].revisions;
-  const oldestRevision = revisions && revisions[revisions.length - 1];
+  const revisions = data?.[0].revisions;
+  const oldestRevision = revisions?.[revisions.length - 1];
 
   // First load
   const isLoadingInitialData = !data && !error;
@@ -166,34 +166,30 @@ export const PageRevisionTable = (
           </div>
         </td>
         <td className="col-1">
-          {(hasDiff || revisionId === sourceRevision?._id) && (
-            <div className="form-check form-check-inline me-0">
-              <input
-                type="radio"
-                className="form-check-input"
-                id={`compareSource-${revisionId}`}
-                name="compareSource"
-                value={revisionId}
-                checked={revisionId === sourceRevision?._id}
-                onChange={() => setSourceRevision(revision)}
-              />
-            </div>
-          )}
+          <div className="form-check form-check-inline me-0">
+            <input
+              type="radio"
+              className="form-check-input"
+              id={`compareSource-${revisionId}`}
+              name="compareSource"
+              value={revisionId}
+              checked={revisionId === sourceRevision?._id}
+              onChange={() => setSourceRevision(revision)}
+            />
+          </div>
         </td>
         <td className="col-2">
-          {(hasDiff || revisionId === targetRevision?._id) && (
-            <div className="form-check form-check-inline me-0">
-              <input
-                type="radio"
-                className="form-check-input"
-                id={`compareTarget-${revisionId}`}
-                name="compareTarget"
-                value={revisionId}
-                checked={revisionId === targetRevision?._id}
-                onChange={() => setTargetRevision(revision)}
-              />
-            </div>
-          )}
+          <div className="form-check form-check-inline me-0">
+            <input
+              type="radio"
+              className="form-check-input"
+              id={`compareTarget-${revisionId}`}
+              name="compareTarget"
+              value={revisionId}
+              checked={revisionId === targetRevision?._id}
+              onChange={() => setTargetRevision(revision)}
+            />
+          </div>
         </td>
       </tr>
     );

apps/app/src/client/components/PageHistory/Revision.tsx

@@ -45,13 +45,18 @@ export const Revision = (props: RevisionProps): JSX.Element => {
     return (
       <div
         className={`${styles['revision-history-main']} ${styles['revision-history-main-nodiff']}
-        revision-history-main revision-history-main-nodiff my-1 d-flex align-items-center`}
+        revision-history-main revision-history-main-nodiff my-1 flex-grow-1 d-flex`}
       >
-        <div className="picture-container">{pic}</div>
-        <div className="ms-3">
-          <span className="text-muted small">
-            <UserDate dateTime={revision.createdAt} /> {t('No diff')}
-          </span>
+        <div className="d-flex align-items-center">
+          <div className="picture-container">{pic}</div>
+          <div className="ms-2">
+            <span className="text-muted small">
+              <UserDate dateTime={revision.createdAt} />
+            </span>
+          </div>
+        </div>
+        <div className="flex-grow-1 text-center">
+          <span className="text-muted small">{t('No diff')}</span>
         </div>
       </div>
     );
@@ -75,7 +80,7 @@ export const Revision = (props: RevisionProps): JSX.Element => {
         <div className="ms-2">
           <div className="revision-history-author mb-1">
             <strong>
-              <Username user={author}></Username>
+              <Username user={author} />
             </strong>
             {isLatestRevision && (
               <span className="badge bg-info ms-2">{t('Latest')}</span>

apps/app/src/server/routes/apiv3/page/update-page.ts

@@ -161,11 +161,11 @@ export const updatePageHandlersFactory = (crowi: Crowi): RequestHandler[] => {
           'update',
           option,
         );
-        results.forEach((result) => {
+        for (const result of results) {
           if (result.status === 'rejected') {
             logger.error('Create user notification failed', result.reason);
           }
-        });
+        }
       } catch (err) {
         logger.error('Create user notification failed', err);
       }
@@ -298,7 +298,36 @@ export const updatePageHandlersFactory = (crowi: Crowi): RequestHandler[] => {
           options.grant = grant;
           options.userRelatedGrantUserGroupIds = userRelatedGrantUserGroupIds;
         }
-        previousRevision = await Revision.findById(sanitizeRevisionId);
+
+        // Priority 1: Use provided revisionId (for conflict detection)
+        previousRevision = null;
+        if (sanitizeRevisionId != null) {
+          try {
+            previousRevision = await Revision.findById(sanitizeRevisionId);
+          } catch (error) {
+            logger.error('Failed to fetch previousRevision by revisionId', {
+              revisionId: sanitizeRevisionId,
+              pageId: currentPage._id,
+              error,
+            });
+          }
+        }
+
+        // Priority 2: Fallback to currentPage.revision (for diff detection)
+        if (previousRevision == null && currentPage.revision != null) {
+          try {
+            previousRevision = await Revision.findById(currentPage.revision);
+          } catch (error) {
+            logger.error(
+              'Failed to fetch previousRevision by currentPage.revision',
+              {
+                pageId: currentPage._id,
+                revisionId: currentPage.revision,
+                error,
+              },
+            );
+          }
+        }
 
         // There are cases where "revisionId" is not required for revision updates
         // See: https://dev.growi.org/651a6f4a008fee2f99187431#origin-%E3%81%AE%E5%BC%B7%E5%BC%B1