From a69b25c9dccd5b7d584e5442e8b8a528b3b2bfa8 Mon Sep 17 00:00:00 2001 From: Hunter Read <21973361+hunter-read@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:15:28 -0700 Subject: [PATCH] Fix: preserve GM secrets when a player edits a shared wiki note --- .github/ISSUE_TEMPLATE/bug_report.yml | 2 +- .github/ISSUE_TEMPLATE/feature_request.yml | 2 +- .github/ISSUE_TEMPLATE/question.yml | 2 +- backend/indexer.py | 175 +++++++++++++----- backend/routers/campaigns/_helpers.py | 89 ++++++++- backend/routers/campaigns/wiki.py | 17 +- .../tests/test_campaign_schedule_helpers.py | 98 ++++++++++ backend/tests/test_campaign_wiki.py | 137 +++++++++++++- docs/api.md | 4 +- .../src/components/campaigns/WikiMarkdown.jsx | 6 +- 10 files changed, 459 insertions(+), 73 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 517586c..ce8b1db 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -1,6 +1,6 @@ name: Bug Report description: Something isn't working the way it should -labels: ["bug"] +labels: ["bug", "needs triage"] body: - type: markdown attributes: diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml index 62a2b18..f81d9f3 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -1,6 +1,6 @@ name: Feature Request description: Suggest an idea or improvement for Grimoire -labels: ["enhancement"] +labels: ["enhancement", "needs triage"] body: - type: markdown attributes: diff --git a/.github/ISSUE_TEMPLATE/question.yml b/.github/ISSUE_TEMPLATE/question.yml index 46d1423..f9b55fb 100644 --- a/.github/ISSUE_TEMPLATE/question.yml +++ b/.github/ISSUE_TEMPLATE/question.yml @@ -1,6 +1,6 @@ name: Question / Help description: Not sure how something works, or need help getting set up? -labels: ["question"] +labels: ["question", "needs triage"] body: - type: markdown attributes: diff --git a/backend/indexer.py b/backend/indexer.py index 7a92f8f..0d6c4f6 100644 --- a/backend/indexer.py +++ b/backend/indexer.py @@ -26,7 +26,7 @@ logger = logging.getLogger("grimoire.indexer") -_FITZ_TIMEOUT = 30 # seconds — files that can't be opened in 30s are unreadable +_FITZ_TIMEOUT = 30 # seconds — files that can't be opened in 30s are unreadable _DB_TIMEOUT = 30 # seconds — max time to wait for a DB operation before treating it as hung # Wall-clock budget for extracting text from a single PDF in the isolated @@ -110,6 +110,7 @@ def _open(): raise exc[0] return result[0] + CATEGORY_MAP = { "core": ["core", "rulebook", "rules", "phb", "dmg", "mm", "basic"], "supplement": ["supplement", "expansion", "sourcebook", "guide", "companion"], @@ -124,17 +125,20 @@ def _open(): # Normalized folder names (after slugify) that are treated as the system-agnostic # collection. Books placed in any of these folders use their immediate subfolder # name as the category label instead of going through the normal CATEGORY_MAP. -_SYSTEM_AGNOSTIC_SLUGS = frozenset({ - "system-agnostic", - "generic", - "any", -}) +_SYSTEM_AGNOSTIC_SLUGS = frozenset( + { + "system-agnostic", + "generic", + "any", + } +) def is_system_agnostic_folder(folder_name: str) -> bool: """Return True if this top-level books folder should be treated as system-agnostic.""" return slugify(folder_name) in _SYSTEM_AGNOSTIC_SLUGS + IMAGE_EXTS = {".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff", ".svg"} PDF_EXTS = {".pdf"} DOC_EXTS = {".pdf", ".epub", ".djvu"} @@ -146,10 +150,18 @@ def is_system_agnostic_folder(folder_name: str) -> bool: # generate_thumbnail. Multi-suffix names (.tar.gz/.tar.bz2) are matched by # archive_ext() rather than Path.suffix. ARCHIVE_EXTS = { - ".zip", ".cbz", - ".rar", ".cbr", - ".7z", ".cb7", - ".tar", ".cbt", ".tar.gz", ".tgz", ".tar.bz2", ".tbz2", + ".zip", + ".cbz", + ".rar", + ".cbr", + ".7z", + ".cb7", + ".tar", + ".cbt", + ".tar.gz", + ".tgz", + ".tar.bz2", + ".tbz2", } # Comic-book archives whose first image is used as a cover thumbnail. _COMIC_ARCHIVE_EXTS = {".cbz", ".cbr", ".cb7", ".cbt"} @@ -427,7 +439,9 @@ def _generate_thumbnail_task(filepath: str, output_path: str, size: tuple, resul exc[0] = e -def generate_thumbnail(filepath: str, output_path: str, size: tuple = (300, 400), should_stop=None) -> bool: +def generate_thumbnail( + filepath: str, output_path: str, size: tuple = (300, 400), should_stop=None +) -> bool: """Generate a thumbnail from the first page of a PDF or from an image. Runs in a daemon thread with a timeout so a corrupt or pathologically large @@ -523,9 +537,7 @@ def extract_text_isolated( fd, result_path = tempfile.mkstemp(prefix="grimoire_extract_", suffix=".pkl") os.close(fd) - proc = _MP_CONTEXT.Process( - target=pdf_worker.main, args=(filepath, result_path, text_only) - ) + proc = _MP_CONTEXT.Process(target=pdf_worker.main, args=(filepath, result_path, text_only)) try: proc.start() poll_interval = 0.5 @@ -549,7 +561,8 @@ def extract_text_isolated( if os.path.getsize(result_path) == 0: code = proc.exitcode reason = ( - f"killed by signal {-code}" if code is not None and code < 0 + f"killed by signal {-code}" + if code is not None and code < 0 else f"exited with code {code}" ) logger.error(f"Text extraction worker crashed ({reason}) for {filepath}") @@ -568,7 +581,9 @@ def extract_text_isolated( logger.debug("Failed to remove temp result file %s: %s", result_path, e) -def ocr_page_isolated(filepath: str, page_index: int, should_stop=None, dpi: int | None = None) -> str: +def ocr_page_isolated( + filepath: str, page_index: int, should_stop=None, dpi: int | None = None +) -> str: """OCR a single page in a spawned child, bounded by ``_OCR_PAGE_TIMEOUT``. Returns the recognised text ("" on timeout, crash, cancel, or empty result — @@ -651,7 +666,9 @@ def ocr_book(book: Book, session: Session, should_stop=None, on_page=None) -> st start = book.ocr_pages_done or 0 dpi = book.ocr_dpi # per-book override; None => global OCR_DPI default _where = f" (from page {start + 1})" if start else "" - logger.info(f"Reading text from '{book.title or book.filename}' — {page_count} page(s){_where}…") + logger.info( + f"Reading text from '{book.title or book.filename}' — {page_count} page(s){_where}…" + ) logger.debug( f"OCR: '{book.filename}' — {page_count} page(s), resuming at page {start + 1}" + (f" (dpi={dpi})" if dpi else "") @@ -955,9 +972,7 @@ def resolve_scope(library_path: str, scope_path: str) -> tuple[str, Path]: head, _, rest = cleaned.partition("/") section = head.lower() if section not in ("books", "maps", "tokens", "audio"): - raise ValueError( - f"scope must start with books/, maps/, tokens/, or audio/: {scope_path!r}" - ) + raise ValueError(f"scope must start with books/, maps/, tokens/, or audio/: {scope_path!r}") # Build the target without resolving symlinks so the walked paths match the # filepaths stored by an unscoped scan (which uses library_path verbatim). @@ -1007,8 +1022,15 @@ def _apply_opf_to_book(book: Book, opf_meta: dict, mode: str) -> bool: return changed -def scan_library(library_path: str, data_path: str, session: Session, on_progress=None, - should_stop=None, scope_path: str | None = None, metadata_mode: str = "new"): +def scan_library( + library_path: str, + data_path: str, + session: Session, + on_progress=None, + should_stop=None, + scope_path: str | None = None, + metadata_mode: str = "new", +): """Scan the library directory and register all files in the database. on_progress(scanned_books, total_books, scanned_maps, total_maps, scanned_tokens, @@ -1061,19 +1083,23 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres total_books = ( _count_eligible_files(books_walk_dir, DOC_EXTS | IMAGE_EXTS | ARCHIVE_EXTS) - if scan_books and books_walk_dir.exists() else 0 + if scan_books and books_walk_dir.exists() + else 0 ) total_maps = ( _count_eligible_files(maps_walk_dir, MAP_IMAGE_EXTS) - if scan_maps and maps_walk_dir.exists() else 0 + if scan_maps and maps_walk_dir.exists() + else 0 ) total_tokens = ( _count_eligible_files(tokens_walk_dir, IMAGE_EXTS) - if scan_tokens and tokens_walk_dir.exists() else 0 + if scan_tokens and tokens_walk_dir.exists() + else 0 ) total_audio = ( _count_eligible_files(audio_walk_dir, AUDIO_EXTS) - if scan_audio and audio_walk_dir.exists() else 0 + if scan_audio and audio_walk_dir.exists() + else 0 ) scanned_books = scanned_maps = scanned_tokens = scanned_audio = 0 @@ -1103,7 +1129,8 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres try: system = _run_with_timeout( lambda slug=system_slug: session.query(GameSystem).filter_by(slug=slug).first(), - _DB_TIMEOUT, f"query system '{system_slug}'" + _DB_TIMEOUT, + f"query system '{system_slug}'", ) except TimeoutError as e: logger.error(f"DB hang: {e} — skipping system '{system_name}'") @@ -1127,7 +1154,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres stats["errors"] += 1 continue stats["new_systems"] += 1 - logger.info(f"Found a new game system: {system_name}" + (" (mature)" if is_nsfw else "")) + logger.info( + f"Found a new game system: {system_name}" + (" (mature)" if is_nsfw else "") + ) elif is_nsfw and not system.is_explicit: system.is_explicit = True if is_agnostic and not system.is_system_agnostic: @@ -1135,7 +1164,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres # When scoped to a path deeper than the system dir, walk only that # subtree; otherwise walk the whole system. - walk_root = scope_dir if (scope_section == "books" and len(scope_parts) > 1) else system_dir + walk_root = ( + scope_dir if (scope_section == "books" and len(scope_parts) > 1) else system_dir + ) for root, dirs, files in os.walk(walk_root): dirs[:] = [d for d in dirs if not d.startswith(".")] @@ -1188,7 +1219,8 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres try: existing = _run_with_timeout( lambda fp=filepath: session.query(Book).filter_by(filepath=fp).first(), - _DB_TIMEOUT, f"query book '{filepath}'" + _DB_TIMEOUT, + f"query book '{filepath}'", ) except TimeoutError as e: logger.error(f"DB hang: {e} — skipping '{filename}'") @@ -1200,21 +1232,33 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres if metadata_mode in ("missing", "replace"): opf_meta = _find_opf_meta(root, filename) if _apply_opf_to_book(existing, opf_meta, metadata_mode): - logger.debug(f"Refreshing metadata for '{filename}' (mode={metadata_mode})") + logger.debug( + f"Refreshing metadata for '{filename}' (mode={metadata_mode})" + ) try: - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit metadata refresh '{filepath}'") + _run_with_timeout( + session.commit, + _DB_TIMEOUT, + f"commit metadata refresh '{filepath}'", + ) stats["updated_books"] += 1 except (TimeoutError, IntegrityError) as e: - logger.error(f"DB hang refreshing metadata for '{filename}': {e}") + logger.error( + f"DB hang refreshing metadata for '{filename}': {e}" + ) session.rollback() if existing.scan_failed: logger.debug(f"Already registered, skipping: {filename}") continue # Archives are opaque: only comic-book variants get a # cover thumbnail, and none carry a page count. - thumbnailable = ext in IMAGE_EXTS or ext == ".pdf" or arc_ext in _COMIC_ARCHIVE_EXTS + thumbnailable = ( + ext in IMAGE_EXTS or ext == ".pdf" or arc_ext in _COMIC_ARCHIVE_EXTS + ) needs_thumbnail = thumbnailable and not existing.has_thumbnail - needs_page_count = ext == ".pdf" and existing.page_count == 0 and not existing.index_error + needs_page_count = ( + ext == ".pdf" and existing.page_count == 0 and not existing.index_error + ) if ext in IMAGE_EXTS and existing.page_count == 0: existing.page_count = 1 if not needs_thumbnail and not needs_page_count: @@ -1223,7 +1267,11 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres logger.debug(f"Resuming incomplete scan for: {filename}") book = existing else: - category = agnostic_category(relative_path) if is_agnostic else guess_category(relative_path) + category = ( + agnostic_category(relative_path) + if is_agnostic + else guess_category(relative_path) + ) title = Path(filename).stem.replace("_", " ").replace("-", " ").strip() try: @@ -1246,8 +1294,10 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres category=category, file_size=file_size, mime_type=( - "application/pdf" if ext == ".pdf" - else archive_mime(arc_ext) if arc_ext + "application/pdf" + if ext == ".pdf" + else archive_mime(arc_ext) + if arc_ext else f"image/{ext[1:]}" ), authors=opf_meta.get("authors"), @@ -1263,7 +1313,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres session.add(book) logger.debug(f"DB: committing new book '{filename}'") try: - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit book '{filepath}'") + _run_with_timeout( + session.commit, _DB_TIMEOUT, f"commit book '{filepath}'" + ) stats["new_books"] += 1 logger.info(f"Added book: {title} ({category}) in {system_name}") except TimeoutError as e: @@ -1296,7 +1348,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres # clears it below so the file is resumed normally next time. book.scan_failed = True try: - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit scan_failed '{filepath}'") + _run_with_timeout( + session.commit, _DB_TIMEOUT, f"commit scan_failed '{filepath}'" + ) except (TimeoutError, IntegrityError) as e: logger.error(f"DB hang writing scan_failed for '{filename}': {e}") session.rollback() @@ -1307,7 +1361,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres # Cancelled — clear the flag so the file is resumed next scan. book.scan_failed = False try: - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit thumbnail '{filepath}'") + _run_with_timeout( + session.commit, _DB_TIMEOUT, f"commit thumbnail '{filepath}'" + ) except (TimeoutError, IntegrityError) as e: logger.error(f"DB hang saving thumbnail for '{filename}': {e}") session.rollback() @@ -1316,7 +1372,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres if not book.scan_failed: book.scan_failed = True try: - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit scan_failed '{filepath}'") + _run_with_timeout( + session.commit, _DB_TIMEOUT, f"commit scan_failed '{filepath}'" + ) except (TimeoutError, IntegrityError) as e: logger.error(f"DB hang writing scan_failed for '{filename}': {e}") session.rollback() @@ -1327,7 +1385,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres doc.close() logger.debug(f"Page count: {book.page_count} pages in '{filename}'") book.scan_failed = False - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit page_count '{filepath}'") + _run_with_timeout( + session.commit, _DB_TIMEOUT, f"commit page_count '{filepath}'" + ) except Exception as e: if should_stop and should_stop(): # Cancelled — clear the flag so the file is resumed next scan. @@ -1337,7 +1397,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres book.index_error = str(e)[:500] stats["errors"] += 1 try: - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit scan_failed '{filepath}'") + _run_with_timeout( + session.commit, _DB_TIMEOUT, f"commit scan_failed '{filepath}'" + ) except (TimeoutError, IntegrityError) as e2: logger.error(f"DB hang saving index_error for '{filename}': {e2}") session.rollback() @@ -1378,8 +1440,11 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres logger.debug(f"DB: querying existing map '{filepath}'") try: existing = _run_with_timeout( - lambda fp=filepath: session.query(GenericMap).filter_by(filepath=fp).first(), - _DB_TIMEOUT, f"query map '{filepath}'" + lambda fp=filepath: ( + session.query(GenericMap).filter_by(filepath=fp).first() + ), + _DB_TIMEOUT, + f"query map '{filepath}'", ) except TimeoutError as e: logger.error(f"DB hang: {e} — skipping '{filename}'") @@ -1464,7 +1529,8 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres try: existing = _run_with_timeout( lambda fp=filepath: session.query(Token).filter_by(filepath=fp).first(), - _DB_TIMEOUT, f"query token '{filepath}'" + _DB_TIMEOUT, + f"query token '{filepath}'", ) except TimeoutError as e: logger.error(f"DB hang: {e} — skipping '{filename}'") @@ -1495,7 +1561,9 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres f"{slugify(title)}_{hashlib.md5(filepath.encode()).hexdigest()[:8]}.webp", ) logger.debug(f"Generating thumbnail: {filepath}") - if generate_thumbnail(filepath, thumb_path, size=(200, 200), should_stop=should_stop): + if generate_thumbnail( + filepath, thumb_path, size=(200, 200), should_stop=should_stop + ): token.has_thumbnail = True session.add(token) @@ -1549,7 +1617,8 @@ def scan_library(library_path: str, data_path: str, session: Session, on_progres try: existing = _run_with_timeout( lambda fp=filepath: session.query(Audio).filter_by(filepath=fp).first(), - _DB_TIMEOUT, f"query audio '{filepath}'" + _DB_TIMEOUT, + f"query audio '{filepath}'", ) except TimeoutError as e: logger.error(f"DB hang: {e} — skipping '{filename}'") @@ -1702,7 +1771,9 @@ def _within_scope(path: Path, scope_dir: Path | None) -> bool: return False -def _apply_tags_from_library(library_path: str, session: Session, scope_dir: Path | None = None) -> None: +def _apply_tags_from_library( + library_path: str, session: Session, scope_dir: Path | None = None +) -> None: """Apply tags declared in tags.json files throughout the library tree. When `scope_dir` is given, only tags.json files within that subtree are applied. @@ -1876,7 +1947,9 @@ def index_book_text(book: Book, data_path: str, session: Session, should_stop=No book.index_failed = False logger.debug(f"DB: committing image-only indexed for '{book.filename}'") try: - _run_with_timeout(session.commit, _DB_TIMEOUT, f"commit image-only indexed '{book.filepath}'") + _run_with_timeout( + session.commit, _DB_TIMEOUT, f"commit image-only indexed '{book.filepath}'" + ) except TimeoutError as e: logger.error(f"DB hang: {e} — rolling back image-only indexed for '{book.filename}'") session.rollback() diff --git a/backend/routers/campaigns/_helpers.py b/backend/routers/campaigns/_helpers.py index 9f0aca3..591b682 100644 --- a/backend/routers/campaigns/_helpers.py +++ b/backend/routers/campaigns/_helpers.py @@ -44,12 +44,97 @@ def delete_guest_user(db, user_id: str) -> None: def strip_gm_secrets(body: str) -> str: """Remove every ||...|| GM-only span (markers and enclosed text) from a body. - Used before sending a wiki page to a non-owner so the GM's hidden notes never - leave the server. An unterminated trailing `||` (no closing pair) is left as-is. + This is what a non-owner receives: the hidden text is gone without a trace — + no placeholder, no marker — so a player never learns a secret exists or where + it sits, whether in the rendered page, the raw editor body, or a search + snippet. An unterminated trailing ``||`` (no closing pair) is left as-is. + + A non-owner edits this stripped body; on save ``merge_gm_secrets`` re-weaves + the stored secrets back in by matching the surrounding text. See + [[merge_gm_secrets]]. """ return _GM_SECRET_RE.sub("", body or "") +def _secret_positions(stored: str): + """Return [(secret_text, offset_in_stripped)] for each ||...|| in ``stored``. + + ``offset_in_stripped`` is where the secret sat within + ``strip_gm_secrets(stored)`` — the boundary in the visible text the player last + saw. It is the secret's start minus the length of all secrets before it (those + chars aren't in the stripped text). Adjacent secrets legitimately share one + boundary; both are re-inserted there in document order. + """ + out = [] + removed = 0 # total chars of earlier secrets, absent from the stripped text + for m in _GM_SECRET_RE.finditer(stored): + out.append((m.group(0), m.start() - removed)) + removed += m.end() - m.start() + return out + + +def merge_gm_secrets(stored_body: str, new_body: str) -> str: + """Re-weave the stored body's ||...|| GM secrets into a non-owner's saved edit. + + A non-owner never receives the secrets: they edit ``strip_gm_secrets(stored)``, + a body with no marker or trace of the hidden text. So ``new_body`` has none, + and storing it verbatim would delete every secret. This restores them by + position: + + * ``old_visible`` — the exact stripped text the player was shown — is aligned + against ``new_body`` (a character diff). Each secret sat at a known boundary + in ``old_visible``; that boundary is mapped forward through the diff to the + corresponding spot in ``new_body``, and the secret is re-inserted there. + * This holds the secret in place when the player edits the text above and/or + below it — the secret does not drift past later paragraphs. + * If the text on *both* sides of a secret's boundary was rewritten (the anchor + is gone), the secret is appended at the end so it is preserved, never lost. + + ``new_body`` is returned unchanged when the stored body had no secrets. + """ + from difflib import SequenceMatcher + + stored = stored_body or "" + positioned = _secret_positions(stored) + if not positioned: + return new_body + + new = new_body if new_body is not None else "" + old_visible = strip_gm_secrets(stored) + + # Map each offset in old_visible to an offset in new via the diff's matching + # blocks. A boundary that falls inside an unchanged run maps exactly; one that + # falls in a replaced/deleted region has no stable image → treat as lost. + sm = SequenceMatcher(None, old_visible, new, autojunk=False) + blocks = sm.get_matching_blocks() # includes the terminating (len, len, 0) + + def map_offset(old_off): + for b in blocks: + if b.a <= old_off <= b.a + b.size: + return b.b + (old_off - b.a) + return None + + # Insert secrets from last to first so earlier insertions don't shift the + # offsets of later ones. Ties (adjacent secrets at one boundary) are broken by + # document index so their original order is preserved after insertion. + indexed = [(off, i, secret) for i, (secret, off) in enumerate(positioned)] + result = new + orphans = [] + for old_off, i, secret in sorted(indexed, reverse=True): + pos = map_offset(old_off) + if pos is None: + orphans.append((i, secret)) + else: + result = result[:pos] + secret + result[pos:] + + if orphans: + # Restore document order for the appended, position-lost secrets. + orphans.sort() + sep = "" if not result or result.endswith("\n") else "\n" + result = result + sep + "\n".join(s for _, s in orphans) + return result + + def is_gm_or_admin(user: CurrentUser) -> bool: return user.role in ("admin", "gm") diff --git a/backend/routers/campaigns/wiki.py b/backend/routers/campaigns/wiki.py index 14ca260..9b05acf 100644 --- a/backend/routers/campaigns/wiki.py +++ b/backend/routers/campaigns/wiki.py @@ -20,6 +20,7 @@ can_view, extract_snippet, get_campaign_or_404, + merge_gm_secrets, strip_gm_secrets, ) from ._schemas import WikiPageCreate, WikiPageUpdate, WikiReorder @@ -252,9 +253,12 @@ def get_page( "campaign_id": campaign_id, "title": page.title, "slug": page.slug, - # ||...|| spans are GM-only: strip them for everyone but the owner. - # (Personal campaigns only ever have the owner as a viewer, so their - # bodies are never stripped.) + # ||...|| spans are GM-only. The owner gets the raw body; everyone else + # gets it fully stripped — no secret text and no marker, so a player + # never learns a secret exists or where. A later save re-weaves the + # stored secrets back by position (merge_gm_secrets). (Personal + # campaigns only ever have the owner as a viewer, so nothing is + # stripped.) "body": page.body if is_owner else strip_gm_secrets(page.body), "visibility": page.visibility, "page_type": page.page_type, @@ -355,7 +359,12 @@ def update_page( page.title = new_title page.slug = _ensure_unique_slug(db, campaign_id, slugify(new_title), exclude_id=page.id) if data.body is not None: - page.body = data.body + # A non-owner never received the ||...|| GM secrets (they're stripped + # on read), so their submitted body has none — storing it verbatim + # would delete the GM's hidden notes. Re-inject the stored secrets so + # they outlive a player's edit. The owner submits the full body, secrets + # and all, so nothing is merged for them. + page.body = data.body if is_owner else merge_gm_secrets(page.body, data.body) if data.visibility is not None: if data.visibility not in ("gm", "group", "members"): raise HTTPException(400, "Invalid visibility") diff --git a/backend/tests/test_campaign_schedule_helpers.py b/backend/tests/test_campaign_schedule_helpers.py index 13ce54d..e50f166 100644 --- a/backend/tests/test_campaign_schedule_helpers.py +++ b/backend/tests/test_campaign_schedule_helpers.py @@ -8,6 +8,7 @@ from backend.routers.campaigns._helpers import ( compute_next_sessions, + merge_gm_secrets, nth_weekday_of_month, extract_snippet, strip_gm_secrets, @@ -130,3 +131,100 @@ def test_leaves_unterminated_marker(self): def test_none_body_returns_empty(self): assert strip_gm_secrets(None) == "" + + +class TestMergeGmSecrets: + """merge_gm_secrets re-weaves stored ||secrets|| into a body the player edited. + + A non-owner edits ``strip_gm_secrets(stored)`` — a fully clean body with no + secret text and no marker of any kind. So every ``new`` here is built by + editing that stripped view, mirroring what the player actually sends back. + """ + + def _seen(self, stored): + """The clean body the player was shown (what they edit).""" + return strip_gm_secrets(stored) + + def test_no_stored_secrets_returns_new_body_verbatim(self): + assert merge_gm_secrets("plain stored", "player edit") == "player edit" + + def test_unedited_clean_body_restores_secret_in_place(self): + stored = "Para A. ||secret|| Para B." + # Player saved without touching anything. + assert merge_gm_secrets(stored, self._seen(stored)) == stored + + def test_edit_above_secret_keeps_it_in_place(self): + stored = "Para A. ||secret|| Para B." + new = self._seen(stored).replace("Para A.", "Para A edited.") + assert merge_gm_secrets(stored, new) == "Para A edited. ||secret|| Para B." + + def test_edit_below_secret_keeps_it_in_place(self): + stored = "Para A. ||secret|| Para B." + new = self._seen(stored).replace("Para B.", "Para B edited.") + assert merge_gm_secrets(stored, new) == "Para A. ||secret|| Para B edited." + + def test_edit_both_sides_keeps_secret_between_them(self): + # The reporter's 3-paragraph case: the secret must NOT drift below the + # second block when the text above AND below it is edited. + stored = "Para A.\n\n||secret||\n\nPara B." + new = self._seen(stored).replace("Para A.", "A2.").replace("Para B.", "B2.") + assert merge_gm_secrets(stored, new) == "A2.\n\n||secret||\n\nB2." + + def test_secret_does_not_move_below_following_paragraph(self): + stored = "First paragraph here. ||twist|| Second paragraph here." + # Player rewrote the first paragraph but left the second intact. + new = self._seen(stored).replace( + "First paragraph here.", "A different first paragraph." + ) + merged = merge_gm_secrets(stored, new) + assert merged.index("||twist||") < merged.index("Second paragraph") + + def test_rewriting_both_sides_appends_secret_so_it_survives(self): + # When the anchor text on both sides of the secret is gone, the position + # can't be recovered — the secret is appended rather than lost. + stored = "Original A. ||the twist|| Original B." + new = "Completely different content with nothing in common." + merged = merge_gm_secrets(stored, new) + assert merged.startswith("Completely different content") + assert "||the twist||" in merged + + def test_multiple_secrets_all_kept_in_order(self): + stored = "aaaa ||one|| bbbb ||two|| cccc" + assert merge_gm_secrets(stored, self._seen(stored)) == stored + + def test_multiple_secrets_survive_edits_around_each(self): + stored = "A ||one|| B ||two|| C" + new = self._seen(stored).replace("A ", "A! ").replace(" C", " C?") + merged = merge_gm_secrets(stored, new) + assert "||one||" in merged and "||two||" in merged + assert merged.index("one") < merged.index("two") + + def test_adjacent_secrets_kept_in_document_order(self): + stored = "x ||one||||two|| y" + assert merge_gm_secrets(stored, self._seen(stored)) == stored + + def test_multiline_secret_restored_intact(self): + stored = "Intro.\n||line one\nline two||\nOutro." + assert merge_gm_secrets(stored, self._seen(stored)) == stored + + def test_secret_at_start_of_body(self): + stored = "||opening|| then visible." + new = self._seen(stored).replace("visible", "visible text") + assert merge_gm_secrets(stored, new) == "||opening|| then visible text." + + def test_secret_at_end_of_body(self): + stored = "Visible lead-in. ||closing||" + new = self._seen(stored).replace("Visible", "The visible") + assert merge_gm_secrets(stored, new) == "The visible lead-in. ||closing||" + + def test_none_new_body_still_preserves_secret(self): + merged = merge_gm_secrets("x ||secret||", None) + assert "||secret||" in merged + + def test_no_marker_or_secret_text_ever_shown_to_player(self): + # The player's copy leaks nothing: not the text, not a placeholder token. + stored = "Visible ||the duke is a doppelganger|| more" + seen = self._seen(stored) + assert "doppelganger" not in seen + assert "||" not in seen + assert "⟦" not in seen and "GM·" not in seen diff --git a/backend/tests/test_campaign_wiki.py b/backend/tests/test_campaign_wiki.py index 7f69617..e4948bb 100644 --- a/backend/tests/test_campaign_wiki.py +++ b/backend/tests/test_campaign_wiki.py @@ -168,26 +168,41 @@ def test_member_cannot_edit_others_page(self, client, gm_headers, player_headers class TestWikiGmSecrets: BODY = "Public intro. ||The duke is a doppelganger|| The rest is shared." - STRIPPED = "Public intro. The rest is shared." + + def _get(self, client, cid, pid, headers): + return client.get(f"/api/campaigns/{cid}/wiki/{pid}", headers=headers).json() + + def _patch(self, client, cid, pid, body, headers): + return client.patch( + f"/api/campaigns/{cid}/wiki/{pid}", json={"body": body}, headers=headers + ) def test_owner_sees_secret_spans(self, client, gm_headers, player_headers, campaign_with_member): cid = campaign_with_member page = _create(client, gm_headers, cid, title="Lore", body=self.BODY, visibility="group").json() - got = client.get(f"/api/campaigns/{cid}/wiki/{page['id']}", headers=gm_headers).json() + got = self._get(client, cid, page["id"], gm_headers) assert got["body"] == self.BODY - def test_player_gets_secret_stripped(self, client, gm_headers, player_headers, campaign_with_member): + def test_player_gets_secret_fully_stripped_no_trace( + self, client, gm_headers, player_headers, campaign_with_member + ): cid = campaign_with_member page = _create(client, gm_headers, cid, title="Lore", body=self.BODY, visibility="group").json() - got = client.get(f"/api/campaigns/{cid}/wiki/{page['id']}", headers=player_headers).json() - assert got["body"] == self.STRIPPED + got = self._get(client, cid, page["id"], player_headers) + # The player's body leaks nothing: no hidden text, no pipe markers, and no + # placeholder token hinting a secret exists or where. assert "doppelganger" not in got["body"] + assert "||" not in got["body"] + assert "⟦" not in got["body"] and "GM·" not in got["body"] + assert got["body"] == "Public intro. The rest is shared." - def test_multiline_secret_stripped(self, client, gm_headers, player_headers, campaign_with_member): + def test_multiline_secret_fully_stripped( + self, client, gm_headers, player_headers, campaign_with_member + ): cid = campaign_with_member body = "Before.\n||line one\nline two||\nAfter." page = _create(client, gm_headers, cid, title="Multi", body=body, visibility="group").json() - got = client.get(f"/api/campaigns/{cid}/wiki/{page['id']}", headers=player_headers).json() + got = self._get(client, cid, page["id"], player_headers) assert "line one" not in got["body"] assert "line two" not in got["body"] assert got["body"] == "Before.\n\nAfter." @@ -199,7 +214,7 @@ def test_personal_campaign_keeps_secrets(self, client, player_headers): "/api/campaigns", json={"name": f"Personal {uid()}"}, headers=player_headers ).json() page = _create(client, player_headers, c["id"], title="Mine", body=self.BODY).json() - got = client.get(f"/api/campaigns/{c['id']}/wiki/{page['id']}", headers=player_headers).json() + got = self._get(client, c["id"], page["id"], player_headers) assert got["body"] == self.BODY def test_search_snippet_hides_secret_from_player( @@ -218,11 +233,115 @@ def test_search_snippet_hides_secret_from_player( resp = client.get(f"/api/campaigns/{cid}/wiki/search?q=treasure", headers=player_headers) assert resp.status_code == 200 assert resp.json()["results"] == [] - # ...but a visible word still matches, with the secret stripped from the snippet. + # ...but a visible word still matches; the snippet carries no secret text. resp2 = client.get(f"/api/campaigns/{cid}/wiki/search?q=visible", headers=player_headers) hit = next(r for r in resp2.json()["results"] if r["title"] == "Findable") assert "treasure" not in hit["snippet"] + def _authored_page_with_secret(self, client, gm_headers, player_headers, cid, body): + """Player authors a group page; the GM edits it to a body with ||secret||s. + Returns (page_id, player's clean stripped view of the body).""" + page = _create( + client, player_headers, cid, title=f"Log {uid()}", body="placeholder", + visibility="group", + ).json() + pid = page["id"] + assert self._patch(client, cid, pid, body, gm_headers).status_code == 200 + seen = self._get(client, cid, pid, player_headers)["body"] + # Sanity: the player's copy never contains the secret or a marker. + assert "||" not in seen and "⟦" not in seen + return pid, seen + + def test_player_edit_above_secret_keeps_it_in_place( + self, client, gm_headers, player_headers, campaign_with_member + ): + cid = campaign_with_member + pid, seen = self._authored_page_with_secret( + client, gm_headers, player_headers, cid, + "We met the duke. ||He is a doppelganger.|| The feast ended.", + ) + # Player edits the text BEFORE the (invisible) secret and re-saves. + new = seen.replace("We met the duke.", "We met the duke at dusk.") + assert self._patch(client, cid, pid, new, player_headers).status_code == 200 + + gm_body = self._get(client, cid, pid, gm_headers)["body"] + assert gm_body == ( + "We met the duke at dusk. ||He is a doppelganger.|| The feast ended." + ) + assert "doppelganger" not in self._get(client, cid, pid, player_headers)["body"] + + def test_player_edit_below_secret_keeps_it_in_place( + self, client, gm_headers, player_headers, campaign_with_member + ): + cid = campaign_with_member + pid, seen = self._authored_page_with_secret( + client, gm_headers, player_headers, cid, + "We met the duke. ||He is a doppelganger.|| The feast ended.", + ) + new = seen.replace("The feast ended.", "The feast ended in a brawl.") + assert self._patch(client, cid, pid, new, player_headers).status_code == 200 + gm_body = self._get(client, cid, pid, gm_headers)["body"] + assert gm_body == ( + "We met the duke. ||He is a doppelganger.|| The feast ended in a brawl." + ) + + def test_player_edit_both_sides_does_not_move_secret_below( + self, client, gm_headers, player_headers, campaign_with_member + ): + # The reporter's 3-paragraph case: editing the public blocks above AND below + # a secret must keep the secret between them, never dropped to the bottom. + cid = campaign_with_member + pid, seen = self._authored_page_with_secret( + client, gm_headers, player_headers, cid, + "Para A.\n\n||the hidden twist||\n\nPara B.", + ) + new = seen.replace("Para A.", "Para A edited.").replace("Para B.", "Para B edited.") + assert self._patch(client, cid, pid, new, player_headers).status_code == 200 + gm_body = self._get(client, cid, pid, gm_headers)["body"] + assert gm_body == "Para A edited.\n\n||the hidden twist||\n\nPara B edited." + + def test_player_rewriting_around_secret_preserves_it_at_bottom( + self, client, gm_headers, player_headers, campaign_with_member + ): + # When the text on both sides of the secret is rewritten past recognition, + # the position can't be recovered — the secret survives, appended at the end. + cid = campaign_with_member + pid, _seen = self._authored_page_with_secret( + client, gm_headers, player_headers, cid, + "Original intro line. ||dont lose me|| Original outro line.", + ) + new = "A totally rewritten note with nothing in common." + assert self._patch(client, cid, pid, new, player_headers).status_code == 200 + gm_body = self._get(client, cid, pid, gm_headers)["body"] + assert "||dont lose me||" in gm_body # survived + assert gm_body.startswith("A totally rewritten note") + + def test_player_edit_preserves_multiple_secrets( + self, client, gm_headers, player_headers, campaign_with_member + ): + cid = campaign_with_member + pid, seen = self._authored_page_with_secret( + client, gm_headers, player_headers, cid, + "A ||first secret|| B ||second secret|| C", + ) + new = seen.replace("A ", "A (edited) ").replace(" C", " C!") + assert self._patch(client, cid, pid, new, player_headers).status_code == 200 + gm_body = self._get(client, cid, pid, gm_headers)["body"] + assert "||first secret||" in gm_body + assert "||second secret||" in gm_body + assert gm_body.index("first secret") < gm_body.index("second secret") + + def test_owner_edit_stores_body_verbatim( + self, client, gm_headers, player_headers, campaign_with_member + ): + # The owner submits raw ||...||, which is stored as-is (no merge for them). + cid = campaign_with_member + page = _create(client, gm_headers, cid, title="Owned", body="x", visibility="group").json() + pid = page["id"] + body = "Alpha ||owner secret|| Omega" + assert self._patch(client, cid, pid, body, gm_headers).status_code == 200 + assert self._get(client, cid, pid, gm_headers)["body"] == body + class TestWikiLinks: def test_link_autocreates_stub_and_backlink(self, client, gm_headers, gm_campaign): diff --git a/docs/api.md b/docs/api.md index e63cc3d..cbb6040 100644 --- a/docs/api.md +++ b/docs/api.md @@ -425,7 +425,9 @@ Pages nest: each page has an optional `parent_id` (null = top level), forming a Each page has a **visibility**: `gm` (owner only), `group` (all accepted members), or `members` (owner plus the users in `shared_user_ids`). The owner may create/edit/delete any page; a member may create `group` pages and edit/delete pages they authored, but cannot set `gm`/`members` visibility. -Within a page body, text wrapped in `||double pipes||` is a **GM-only secret** - finer-grained than page visibility, it hides a span inside an otherwise shared page. Those spans (markers and enclosed text, which may span multiple lines) are stripped server-side from the `body` returned to anyone other than the campaign owner, and from `search` snippets/matches for non-owners. The owner always receives the raw `||...||`. Personal (non-GM) campaigns are never stripped, since only the owner can view them. +Within a page body, text wrapped in `||double pipes||` is a **GM-only secret** - finer-grained than page visibility, it hides a span inside an otherwise shared page. The owner always receives the raw `||...||`. For everyone else the secret (markers and enclosed text, which may span multiple lines) is **fully stripped** from the page `body` server-side, leaving no trace — no secret text and no placeholder — so a non-owner never learns a secret exists or where, whether in the rendered page, the raw editor body, or a `search` snippet/match. Personal (non-GM) campaigns are never stripped, since only the owner can view them. + +Because a non-owner edits this stripped body, a player saving an edit to a page they author would otherwise erase the secrets. On such a save the stored secrets are **re-woven back by position** server-side: the stripped text the player was last shown is diffed against their submission, and each secret is re-inserted at the point its surrounding text maps to. A secret therefore stays exactly where the GM placed it even when the player edits the text above and/or below it (it does not drift past later paragraphs). If the text on both sides of a secret was rewritten past recognition, that secret is appended at the end of the body — preserved, never lost. The owner submits the raw body (secrets and all), which is stored verbatim. | Endpoint | Method | Auth | Description | |----------|--------|------|-------------| diff --git a/frontend/src/components/campaigns/WikiMarkdown.jsx b/frontend/src/components/campaigns/WikiMarkdown.jsx index 1ce7af6..ee344c1 100644 --- a/frontend/src/components/campaigns/WikiMarkdown.jsx +++ b/frontend/src/components/campaigns/WikiMarkdown.jsx @@ -17,9 +17,9 @@ const LINK_RE = /\[\[([^\]|]+?)(?:\|([^\]]+))?\]\]/g const EMBED_PREFIXES = ['book:', 'map:', 'token:', 'audio:', 'file:', 'image:'] // ||GM-only text||. Only the owner ever receives a body still containing these -// (the backend strips them for everyone else), so rendering them as a tinted -// "GM only" span just helps the owner see what players won't. The match spans -// newlines so a secret can wrap several lines/paragraphs. +// (the backend strips them entirely for everyone else — no text, no marker), so +// rendering them as a tinted "GM only" span just helps the owner see what players +// won't. The match spans newlines so a secret can wrap several lines/paragraphs. const SECRET_RE = /\|\|([\s\S]*?)\|\|/g function slugify(title) {