SKILL.md 12 KB


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

// 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

// 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:

// ✅ 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,
);
// ❌ WRONG: Forcing frontend to always send revisionId
const revisionId = currentRevisionId;  // Always send, regardless of origin
// This breaks Yjs collaborative editing semantics!
// ❌ 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:

    previousRevision = await Revision.findById(undefined);  // → null
    

    Result: No previousRevision fetched via revisionId

  3. Backend Conflict Check (isUpdatable):

    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:

    previousRevision = await Revision.findById(sanitizeRevisionId);  // → revision object
    

    Result: previousRevision fetched successfully

  3. Backend Conflict Check (isUpdatable):

    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

  1. 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):

    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:

// 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:

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.