fix(navigation): block invalid book names and preserve them after syn…

sharedUtils/bookNameValidation.ts

@@ -0,0 +1,71 @@
+/**
+ * Validation rules for user-edited book display names (`fileDisplayName`).
+ *
+ * Keep this list narrow and conservative. The primary motivation (issue #1013)
+ * is that earlier metadata-cleanup logic ran `path.extname()` over the display
+ * name and treated anything after a `.` as a strippable extension, silently
+ * truncating names like "1. New Items" to "1" on every sync. The safest fix
+ * is to disallow filesystem-unsafe characters at the entry point so the name
+ * round-trips cleanly through metadata, file IO, and any future tooling that
+ * may interpret the value as a path.
+ *
+ * Shared between the extension host (defensive validation in providers) and
+ * the webview (live input warnings + disabled save).
+ */
+
+/**
+ * Characters that must not appear in a book display name.
+ *
+ * `.` is the bug from issue #1013. The remaining characters are reserved on
+ * common filesystems / URLs and would similarly create downstream surprises
+ * if they leaked into the display name.
+ */
+export const DISALLOWED_BOOK_NAME_CHARS: readonly string[] = [
+    ".",
+    "/",
+    "\\",
+    ":",
+    "*",
+    "?",
+    '"',
+    "<",
+    ">",
+    "|",
+];
+
+/**
+ * Returns the subset of {@link DISALLOWED_BOOK_NAME_CHARS} present in `name`.
+ * Returns an empty array if the name is valid (or empty).
+ */
+export function findDisallowedBookNameChars(name: string): string[] {
+    if (!name) return [];
+    const seen = new Set<string>();
+    for (const ch of name) {
+        if (DISALLOWED_BOOK_NAME_CHARS.includes(ch)) {
+            seen.add(ch);
+        }
+    }
+    return Array.from(seen);
+}
+
+/**
+ * True if the name contains any disallowed character. Whitespace-only / empty
+ * names are NOT considered "invalid" by this function — those are handled
+ * separately by callers that already trim and reject empty input.
+ */
+export function bookNameHasDisallowedChars(name: string): boolean {
+    return findDisallowedBookNameChars(name).length > 0;
+}
+
+/**
+ * Human-friendly explanation of why a name is rejected, suitable for showing
+ * inline beneath an input field. Returns `null` when the name is valid.
+ */
+export function getBookNameValidationMessage(name: string): string | null {
+    const offenders = findDisallowedBookNameChars(name);
+    if (offenders.length === 0) return null;
+
+    const list = offenders.map((c) => `"${c}"`).join(", ");
+    const noun = offenders.length === 1 ? "character" : "characters";
+    return `${list} ${offenders.length === 1 ? "is" : "are"} not allowed in book names. Use dashes (-) or underscores (_) instead.`;
+}

sharedUtils/index.ts

@@ -283,3 +283,4 @@ export const deriveTargetPathFromSource = (sourcePath: string): string => {
 // Re-export corpus utilities
 export * from "./corpusUtils";
 export * from "./exportOptionsEligibility";
+export * from "./bookNameValidation";

src/providers/navigationWebview/navigationWebviewProvider.ts

@@ -7,7 +7,7 @@ import { BaseWebviewProvider } from "../../globalProvider";
 import { getWebviewHtml } from "../../utils/webviewTemplate";
 import { safePostMessageToView } from "../../utils/webviewUtils";
 import { CodexItem } from "types";
-import { getCellValueData, cellHasAudioUsingAttachments, computeValidationStats, computeProgressPercents, shouldExcludeCellFromProgress, shouldExcludeQuillCellFromProgress, countActiveValidations, hasTextContent } from "../../../sharedUtils";
+import { getCellValueData, cellHasAudioUsingAttachments, computeValidationStats, computeProgressPercents, shouldExcludeCellFromProgress, shouldExcludeQuillCellFromProgress, countActiveValidations, hasTextContent, getBookNameValidationMessage } from "../../../sharedUtils";
 import { normalizeCorpusMarker } from "../../utils/corpusMarkerUtils";
 import { addMetadataEdit, addProjectMetadataEdit, EditMapUtils } from "../../utils/editMapUtils";
 import { MetadataManager } from "../../utils/metadataManager";
@@ -957,6 +957,21 @@ export class NavigationWebviewProvider extends BaseWebviewProvider {
             return;
         }
 
+        // Defensive: reject names containing characters that would later be misinterpreted
+        // by filesystem-style cleanup (see issue #1013) or are otherwise unsafe in paths.
+        // The webview also blocks these client-side, but we re-validate here in case a
+        // stale webview, command, or future caller bypasses that check.
+        const trimmedNewBookName = (newBookName ?? "").trim();
+        if (!trimmedNewBookName) {
+            vscode.window.showErrorMessage("Book name cannot be empty.");
+            return;
+        }
+        const validationError = getBookNameValidationMessage(trimmedNewBookName);
+        if (validationError) {
+            vscode.window.showErrorMessage(`Cannot update book name: ${validationError}`);
+            return;
+        }
+
         try {
             // Find codex files matching the book abbreviation and read metadata from first file
             const { matchingUris, corpusMarker } = await findCodexFilesByBookAbbr(bookAbbr, { readMetadata: true });
@@ -1019,15 +1034,15 @@ export class NavigationWebviewProvider extends BaseWebviewProvider {
                             const oldValue = metadata.fileDisplayName;
 
                             // Only add edit if value is actually changing
-                            if (oldValue !== newBookName) {
+                            if (oldValue !== trimmedNewBookName) {
                                 // Add edit history entry before updating metadata
-                                addMetadataEdit(metadata, "fileDisplayName", newBookName, currentUser);
+                                addMetadataEdit(metadata, "fileDisplayName", trimmedNewBookName, currentUser);
                             }
 
                             // Update metadata to add fileDisplayName (preserve originalName)
                             notebookData.metadata = {
                                 ...metadata,
-                                fileDisplayName: newBookName,
+                                fileDisplayName: trimmedNewBookName,
                                 // Preserve originalName if it exists, don't modify it
                             };
 
@@ -1068,15 +1083,15 @@ export class NavigationWebviewProvider extends BaseWebviewProvider {
                                     const sourceOldValue = sourceMetadata.fileDisplayName;
 
                                     // Only add edit if value is actually changing
-                                    if (sourceOldValue !== newBookName) {
+                                    if (sourceOldValue !== trimmedNewBookName) {
                                         // Add edit history entry before updating metadata
-                                        addMetadataEdit(sourceMetadata, "fileDisplayName", newBookName, currentUser);
+                                        addMetadataEdit(sourceMetadata, "fileDisplayName", trimmedNewBookName, currentUser);
                                     }
 
                                     // Update metadata to add fileDisplayName (preserve originalName)
                                     sourceNotebookData.metadata = {
                                         ...sourceMetadata,
-                                        fileDisplayName: newBookName,
+                                        fileDisplayName: trimmedNewBookName,
                                         // Preserve originalName if it exists, don't modify it
                                     };
 
@@ -1104,11 +1119,11 @@ export class NavigationWebviewProvider extends BaseWebviewProvider {
 
                 if (updatedCount > 0) {
                     vscode.window.showInformationMessage(
-                        `Book name updated: "${bookAbbr}" → "${newBookName}" (${updatedCount} file(s) updated)`
+                        `Book name updated: "${bookAbbr}" → "${trimmedNewBookName}" (${updatedCount} file(s) updated)`
                     );
                 } else {
                     vscode.window.showInformationMessage(
-                        `Book name updated: "${bookAbbr}" → "${newBookName}" (no matching codex files found)`
+                        `Book name updated: "${bookAbbr}" → "${trimmedNewBookName}" (no matching codex files found)`
                     );
                 }
             } catch (error) {

src/test/suite/bookNameValidation.test.ts

@@ -0,0 +1,78 @@
+import * as assert from "assert";
+import {
+    DISALLOWED_BOOK_NAME_CHARS,
+    findDisallowedBookNameChars,
+    bookNameHasDisallowedChars,
+    getBookNameValidationMessage,
+} from "../../../sharedUtils/bookNameValidation";
+
+suite("bookNameValidation Test Suite", () => {
+    test("DISALLOWED_BOOK_NAME_CHARS includes the period that triggered issue #1013", () => {
+        assert.ok(
+            DISALLOWED_BOOK_NAME_CHARS.includes("."),
+            "'.' must be in the disallowed list (root cause of #1013)"
+        );
+    });
+
+    test("DISALLOWED_BOOK_NAME_CHARS includes filesystem-unsafe characters", () => {
+        const expected = ["/", "\\", ":", "*", "?", '"', "<", ">", "|"];
+        for (const ch of expected) {
+            assert.ok(
+                DISALLOWED_BOOK_NAME_CHARS.includes(ch),
+                `'${ch}' should be disallowed in book names`
+            );
+        }
+    });
+
+    test("findDisallowedBookNameChars returns offenders without duplicates", () => {
+        assert.deepStrictEqual(findDisallowedBookNameChars("1. New Items"), ["."]);
+        assert.deepStrictEqual(
+            findDisallowedBookNameChars("a/b\\c/d"),
+            ["/", "\\"],
+            "Repeated offenders should be deduplicated"
+        );
+        assert.deepStrictEqual(findDisallowedBookNameChars("My Book"), []);
+        assert.deepStrictEqual(findDisallowedBookNameChars(""), []);
+    });
+
+    test("bookNameHasDisallowedChars distinguishes valid and invalid names", () => {
+        assert.strictEqual(bookNameHasDisallowedChars("1. New Items"), true);
+        assert.strictEqual(bookNameHasDisallowedChars("1.New"), true);
+        assert.strictEqual(bookNameHasDisallowedChars(".leading"), true);
+        assert.strictEqual(bookNameHasDisallowedChars("trailing."), true);
+
+        assert.strictEqual(bookNameHasDisallowedChars("1 New Items"), false);
+        assert.strictEqual(bookNameHasDisallowedChars("1-New-Items"), false);
+        assert.strictEqual(bookNameHasDisallowedChars("1_New_Items"), false);
+        assert.strictEqual(bookNameHasDisallowedChars("Genesis"), false);
+        assert.strictEqual(bookNameHasDisallowedChars(""), false);
+    });
+
+    test("getBookNameValidationMessage returns null for valid names", () => {
+        assert.strictEqual(getBookNameValidationMessage("Genesis"), null);
+        assert.strictEqual(getBookNameValidationMessage("1-New-Items"), null);
+        assert.strictEqual(getBookNameValidationMessage("1_New_Items"), null);
+        assert.strictEqual(getBookNameValidationMessage(""), null);
+    });
+
+    test("getBookNameValidationMessage describes the offending characters", () => {
+        const message = getBookNameValidationMessage("1. New Items");
+        assert.ok(message, "Should return a non-null message for invalid names");
+        assert.ok(message!.includes('"."'), "Message should reference the period");
+        assert.ok(
+            message!.toLowerCase().includes("dash") || message!.includes("-"),
+            "Message should suggest dashes as an alternative"
+        );
+        assert.ok(
+            message!.toLowerCase().includes("underscore") || message!.includes("_"),
+            "Message should suggest underscores as an alternative"
+        );
+    });
+
+    test("getBookNameValidationMessage handles multiple offenders", () => {
+        const message = getBookNameValidationMessage("a/b.c");
+        assert.ok(message, "Should return a non-null message for invalid names");
+        assert.ok(message!.includes('"/"'), "Message should reference '/'");
+        assert.ok(message!.includes('"."'), "Message should reference '.'");
+    });
+});

src/test/suite/navigationWebviewProvider.test.ts

@@ -445,6 +445,95 @@ suite("NavigationWebviewProvider Test Suite", () => {
         }
     });
 
+    test("updateBookName rejects names containing periods (issue #1013)", async () => {
+        if (!vscode.workspace.workspaceFolders?.[0]) {
+            return;
+        }
+
+        const codexUri = await createCodexFileWithMetadata("GEN", {
+            fileDisplayName: "Genesis",
+        });
+
+        const loadBibleBookMap = (provider as any).loadBibleBookMap.bind(provider);
+        loadBibleBookMap();
+
+        const showErrorMessageStub = sinon.stub(vscode.window, "showErrorMessage");
+        const showInformationMessageStub = sinon.stub(vscode.window, "showInformationMessage");
+        const withProgressStub = sinon.stub(vscode.window, "withProgress").callsFake(async (options, callback) => {
+            return callback({ report: () => { } } as any, new vscode.CancellationTokenSource().token);
+        });
+
+        try {
+            const updateBookName = (provider as any).updateBookName.bind(provider);
+            await updateBookName("GEN", "1. New Items");
+
+            assert.ok(
+                showErrorMessageStub.called,
+                "Should show an error when the new name contains a disallowed character"
+            );
+
+            // The original fileDisplayName should be untouched on disk.
+            const content = await vscode.workspace.fs.readFile(codexUri);
+            const serializer = new CodexContentSerializer();
+            const notebookData = await serializer.deserializeNotebook(
+                content,
+                new vscode.CancellationTokenSource().token
+            );
+            assert.strictEqual(
+                (notebookData.metadata as any).fileDisplayName,
+                "Genesis",
+                "fileDisplayName must not be changed when validation fails"
+            );
+        } finally {
+            showErrorMessageStub.restore();
+            showInformationMessageStub.restore();
+            withProgressStub.restore();
+        }
+    });
+
+    test("updateBookName accepts names with dashes and underscores", async () => {
+        if (!vscode.workspace.workspaceFolders?.[0]) {
+            return;
+        }
+
+        const codexUri = await createCodexFileWithMetadata("EXO", {});
+
+        const loadBibleBookMap = (provider as any).loadBibleBookMap.bind(provider);
+        loadBibleBookMap();
+
+        const showErrorMessageStub = sinon.stub(vscode.window, "showErrorMessage");
+        const showInformationMessageStub = sinon.stub(vscode.window, "showInformationMessage");
+        const withProgressStub = sinon.stub(vscode.window, "withProgress").callsFake(async (options, callback) => {
+            return callback({ report: () => { } } as any, new vscode.CancellationTokenSource().token);
+        });
+
+        try {
+            const updateBookName = (provider as any).updateBookName.bind(provider);
+            await updateBookName("EXO", "1-New_Items");
+
+            assert.ok(
+                !showErrorMessageStub.called,
+                "Should not show an error for a valid name with dashes/underscores"
+            );
+
+            const content = await vscode.workspace.fs.readFile(codexUri);
+            const serializer = new CodexContentSerializer();
+            const notebookData = await serializer.deserializeNotebook(
+                content,
+                new vscode.CancellationTokenSource().token
+            );
+            assert.strictEqual(
+                (notebookData.metadata as any).fileDisplayName,
+                "1-New_Items",
+                "fileDisplayName should be updated to the valid value"
+            );
+        } finally {
+            showErrorMessageStub.restore();
+            showInformationMessageStub.restore();
+            withProgressStub.restore();
+        }
+    });
+
     test("deleteFile closes open webview panels for codex file", async () => {
         // Skip if no workspace folder
         if (!vscode.workspace.workspaceFolders?.[0]) {