Validate progress snapshot in revision and export endpoints (#135)

Without a snapshot, /revise_chapter would silently strip every planning
context (architecture, timeline, fate, arc, antagonist, technology,
theme, POV) before overwriting the chapter, and /export +
/export_editors_notes would collapse the filename to "Novel.md" /
"Novel-Editors_Notes.md" — silently overwriting prior exports.

Reachable when a session is restored from on-disk JSON that predates
the snapshot field. All three endpoints now reject early with HTTP 400
and a clear message instead of producing degraded or destructive output.

Closes #135

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: CyberSecDef

novelforge/routes/export.py

@@ -120,7 +120,17 @@ def export_novel() -> Response | tuple[Response, int]:
     if not progress_data or progress_data.get("status") != "done":
         return jsonify({"error": "Novel generation not complete."}), 400
 
-    title = (progress_data.get("snapshot") or {}).get("title", "Novel")
+    snapshot = progress_data.get("snapshot")
+    if not isinstance(snapshot, dict) or not snapshot:
+        # Without a snapshot the filename collapses to "Novel.md" and silently
+        # overwrites any prior export. Reject early instead of clobbering.
+        logger.warning("Export rejected: progress entry %s has no snapshot", token)
+        return jsonify({
+            "error": "Progress data is incomplete (missing generation snapshot). "
+                     "Reload or regenerate the novel before exporting."
+        }), 400
+
+    title = snapshot.get("title", "Novel")
     chapters_done = progress_data.get("chapters_done", [])
 
     markdown_content = _format_manuscript(title, chapters_done)
@@ -148,7 +158,17 @@ def export_editors_notes() -> Response | tuple[Response, int]:
     if not progress_data or progress_data.get("status") != "done":
         return jsonify({"error": "Novel generation not complete."}), 400
 
-    title = (progress_data.get("snapshot") or {}).get("title", "Novel")
+    snapshot = progress_data.get("snapshot")
+    if not isinstance(snapshot, dict) or not snapshot:
+        # Same overwrite hazard as /export — without a snapshot the filename
+        # collapses to "Novel-Editors_Notes.md" for every novel.
+        logger.warning("Editor's notes export rejected: progress entry %s has no snapshot", token)
+        return jsonify({
+            "error": "Progress data is incomplete (missing generation snapshot). "
+                     "Reload or regenerate the novel before exporting."
+        }), 400
+
+    title = snapshot.get("title", "Novel")
     consistency = progress_data.get("consistency") or {}
     global_continuity_audit = progress_data.get("global_continuity_audit") or {}
     narrative_compression_report = progress_data.get("narrative_compression_report") or {}

novelforge/routes/generation/revision.py

@@ -78,7 +78,19 @@ def revise_chapter() -> Response | tuple[Response, int]:
     if target_idx is None:
         return jsonify({"error": "Selected chapter was not found."}), 404
 
-    snap = progress_data.get("snapshot", {})
+    snap = progress_data.get("snapshot")
+    if not isinstance(snap, dict) or not snap:
+        # Missing snapshot would silently strip every planning context (architecture,
+        # timeline, fate, arc, etc.) before overwriting the chapter. Reachable via
+        # restored sessions whose on-disk JSON predates the snapshot field.
+        logger.warning(
+            "Revision rejected: progress entry %s has no snapshot (chapter %d)",
+            token, chapter_number,
+        )
+        return jsonify({
+            "error": "Progress data is incomplete (missing generation snapshot). "
+                     "Reload or regenerate the novel before revising chapters."
+        }), 400
     title = snap.get("title", "Novel")
     genre = snap.get("genre", "")
     total_chapters = int(snap.get("chapters", len(chapters_done) or 1))

tests/test_coverage.py

@@ -24,6 +24,7 @@ class TestEditorsNotesFormatting:
     def _full_progress_data(self):
         return {
             "status": "done",
+            "snapshot": {"title": "Full Notes Test", "genre": "Fantasy"},
             "chapters_done": [
                 {"number": 1, "title": "Ch1", "content": "Text", "summary": "S1"},
                 {"number": 2, "title": "Ch2", "content": "Text", "summary": "S2"},
@@ -161,7 +162,11 @@ def test_full_editors_notes_export(self, client, tmp_path, monkeypatch):
 
     def test_editors_notes_no_content(self, client):
         token = "00000000-0000-4000-8000-000000000021"
-        progress_manager.create(token, {"status": "done", "current": 0, "total": 0, "step": "", "chapters_done": [], "error": None})
+        progress_manager.create(token, {
+            "status": "done", "current": 0, "total": 0, "step": "",
+            "snapshot": {"title": "Empty", "genre": ""},
+            "chapters_done": [], "error": None,
+        })
         with client.session_transaction() as sess:
             sess["title"] = "Empty"
 

tests/test_export_snapshot.py

@@ -111,8 +111,8 @@ def test_title_reproducible_across_session_changes(self, client, tmp_path, monke
         assert "Session Title A" not in content1
         assert "Session Title B" not in content2
 
-    def test_no_snapshot_falls_back_to_default(self, client, tmp_path, monkeypatch):
-        """When a token has no snapshot the title defaults to 'Novel'."""
+    def test_missing_snapshot_returns_400(self, client, tmp_path, monkeypatch):
+        """Without a snapshot the export would silently overwrite Novel.md — reject early."""
         import novelforge.config as config
         monkeypatch.setattr(config, "EXPORT_DIR", str(tmp_path))
 
@@ -127,18 +127,37 @@ def test_no_snapshot_falls_back_to_default(self, client, tmp_path, monkeypatch):
                 {"number": 1, "title": "Ch1", "content": "Text.", "summary": "S"},
             ],
         })
-        # Session sets a title but snapshot is absent – route should still work.
         with client.session_transaction() as sess:
             sess["title"] = "Session Only Title"
 
         r = client.post("/export", data=json.dumps({"token": token}),
                         content_type="application/json")
-        assert r.status_code == 200
-        url = r.get_json()["download_url"]
-        content = client.get(url).data.decode("utf-8")
-        # Falls back to "Novel" heading, not the session title.
-        assert "# Novel" in content
-        assert "Session Only Title" not in content
+        assert r.status_code == 400
+        assert "snapshot" in r.get_json()["error"].lower()
+        # Nothing should be written to the export dir.
+        assert not list(tmp_path.glob("*.md"))
+
+    def test_empty_snapshot_returns_400(self, client, tmp_path, monkeypatch):
+        """A snapshot key whose value is an empty dict is treated the same as missing."""
+        import novelforge.config as config
+        monkeypatch.setattr(config, "EXPORT_DIR", str(tmp_path))
+
+        token = "00000000-0000-4000-8000-000000000006"
+        progress_manager.create(token, {
+            "status": "done",
+            "current": 1,
+            "total": 1,
+            "step": "Complete",
+            "error": None,
+            "snapshot": {},
+            "chapters_done": [
+                {"number": 1, "title": "Ch1", "content": "Text.", "summary": "S"},
+            ],
+        })
+
+        r = client.post("/export", data=json.dumps({"token": token}),
+                        content_type="application/json")
+        assert r.status_code == 400
 
 
 # ---------------------------------------------------------------------------
@@ -193,6 +212,31 @@ def test_heading_reflects_snapshot_title(self, client, tmp_path, monkeypatch):
         assert "Correct Notes Title" in content
         assert "Wrong Session Title" not in content
 
+    def test_missing_snapshot_returns_400(self, client, tmp_path, monkeypatch):
+        """Without a snapshot the notes file would collapse to a generic name — reject early."""
+        import novelforge.config as config
+        monkeypatch.setattr(config, "EXPORT_DIR", str(tmp_path))
+
+        token = "00000000-0000-4000-8000-000000000007"
+        progress_manager.create(token, {
+            "status": "done",
+            "current": 1,
+            "total": 1,
+            "step": "Complete",
+            "error": None,
+            "chapters_done": [
+                {"number": 1, "title": "Ch1", "content": "Text.", "summary": "S"},
+            ],
+            "consistency": {"overall_assessment": "Good.", "issues": []},
+        })
+
+        r = client.post("/export_editors_notes",
+                        data=json.dumps({"token": token}),
+                        content_type="application/json")
+        assert r.status_code == 400
+        assert "snapshot" in r.get_json()["error"].lower()
+        assert not list(tmp_path.glob("*.md"))
+
 
 # ---------------------------------------------------------------------------
 # /generate_illustrations  –  metadata comes from snapshot

tests/test_integration.py

@@ -440,6 +440,7 @@ def _make_token(self, suffix: str = "") -> str:
             "invalidation": "00000000-0000-4000-8000-000000000072",
             "persist": "00000000-0000-4000-8000-000000000073",
             "persist-invalidate": "00000000-0000-4000-8000-000000000074",
+            "no-snap": "00000000-0000-4000-8000-000000000075",
         }
         return _suffix_map.get(suffix, f"00000000-0000-4000-8000-{suffix[:12].ljust(12, '0')}")
 
@@ -543,6 +544,28 @@ def test_revise_persists_chapters(self, client, mock_llm, tmp_path, monkeypatch)
         assert len(saved.get("completed_chapters", [])) == 1
         assert saved["completed_chapters"][0]["content"] != "Original text."
 
+    def test_revise_missing_snapshot_returns_400(self, client, mock_llm):
+        """Without a snapshot the revision would proceed with empty planning context
+        and overwrite the chapter — reject early instead."""
+        token = self._make_token("no-snap")
+        progress_manager.create(token, {
+            "status": "done",
+            "current": 1,
+            "total": 1,
+            "step": "Complete",
+            "error": None,
+            "chapters_done": [
+                {"number": 1, "title": "Ch1", "content": "Original text.", "summary": "Old"},
+            ],
+            "consistency": {"issues": [], "overall_assessment": ""},
+        })
+        r = self._post_revise(client, token)
+        assert r.status_code == 400
+        assert "snapshot" in r.get_json()["error"].lower()
+        # Chapter content must remain untouched.
+        pd = progress_manager.get(token)
+        assert pd["chapters_done"][0]["content"] == "Original text."
+
     def test_revise_persistence_includes_invalidated_reports(self, client, mock_llm, tmp_path, monkeypatch):
         """Persisted state after revision reflects the invalidated (None) report values."""
         from novelforge.routes.generation import _DERIVED_REPORT_FIELDS
@@ -582,6 +605,7 @@ def _seed_done(self, client, token="00000000-0000-4000-8000-000000000080"):
             "total": 2,
             "step": "Complete",
             "error": None,
+            "snapshot": {"title": "Export Test Novel", "genre": "Fantasy"},
             "chapters_done": [
                 {"number": 1, "title": "Ch1", "content": "Chapter 1 text.", "summary": "S1"},
                 {"number": 2, "title": "Ch2", "content": "Chapter 2 text.", "summary": "S2"},