Address PR #1825 review follow-ups: CAML edit lock ordering + description follow-up tracking (#1848) (#1855)

* Harden Readme.CAML edit lock ordering; track description follow-ups (#1848)

- caml_article: acquire a row lock on the Corpus inside the atomic CAML
  edit block (select_for_update) so a concurrent corpus delete or
  readme_caml_document cache refresh cannot race between the Document
  lock and import_document. Documents the Document -> Corpus ->
  DocumentPath lock order (issue #1848 item 5).
- descriptions/signals: replace bare follow-up prose with TODO(#1848)
  references for threading the live caller through get_corpus_description
  and for the readme-document signal-guard hardening (items 1 and 3).

* Revert historical migration 0018; fix CAML test class typo (#1848)

- Restore opencontractserver/corpuses/migrations/0018_* to its pre-#1825
  contents (the no-op RunPython appended in #1825 mutated an
  already-shipped migration, which can diverge migration hash-checks in
  CI). The op was a no-op so reverting has no schema/data effect
  (issue #1848 item 6).
- Rename test class AapplyCamlArticleEditCreatesVersionTreeSiblingTest ->
  ApplyCamlArticleEditCreatesVersionTreeSiblingTest (merge-artifact typo).

* Restore migration 0018 inline upload_to helper (#1848)

The prior revert (b504705) reintroduced a dangling reference to
opencontractserver.corpuses.models.calculate_description_filepath, which
the canonical-CAML refactor (#1825) removed. Django cannot import the
historical migration, breaking the test suite. Restore the inlined
calculate_description_filepath callable so 0018 stays importable on
fresh installs.

https://claude.ai/code/session_01Wa4U1t8WG2uAff2JY9d7z1

* Address PR #1855 review: clarify DocumentPath lock comment

The lock-order comment implied DocumentPath is always locked inside
import_document; it is locked via .select_for_update().first() only when the
path already exists (the new-document case has no row to lock). Comment-only;
no behavior change.

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: JSv4

opencontractserver/corpuses/signals.py

@@ -329,7 +329,8 @@ def _is_readme_caml_document(doc: Document) -> bool:
     trip these signals into a spurious cache refresh. The refresh is
     idempotent and corpus-scoped (it re-derives from the current CAML head via
     DocumentPath), so a false positive is wasteful but not corrupting. A
-    model-level flag would harden this; left as a follow-up.
+    model-level flag (e.g. a ``Document.is_corpus_readme`` boolean) would
+    harden this; tracked as TODO(#1848).
     """
     from opencontractserver.constants.document_processing import (
         CAML_ARTICLE_TITLE,

opencontractserver/llms/tools/core_tools/caml_article.py

@@ -522,7 +522,15 @@ def _apply_caml_article_edit(
         from opencontractserver.corpuses.models import Corpus
         from opencontractserver.documents.versioning import import_document
 
-        corpus = Corpus.objects.get(pk=corpus_id)
+        # Lock order inside this atomic block: Document (locked above via
+        # ``select_for_update``) -> Corpus -> DocumentPath (locked inside
+        # ``import_document`` via ``.select_for_update().first()`` — only when
+        # the path already exists; the new-document case has no row to lock).
+        # Lock the Corpus row too so a concurrent corpus
+        # delete, or a ``readme_caml_document_id`` cache refresh, cannot race
+        # between the Document lock and ``import_document`` and leave this edit
+        # operating on a stale Corpus.
+        corpus = Corpus.objects.select_for_update().get(pk=corpus_id)
         new_doc, _status, _path = import_document(
             corpus=corpus,
             path=CAML_ARTICLE_TITLE,

opencontractserver/llms/tools/core_tools/descriptions.py

@@ -56,7 +56,9 @@ def get_corpus_description(
     # description (a corpus-level README, not per-document data) is effectively
     # gated upstream. Threading the live caller through here for defence-in-depth
     # would require a tool-signature + injection change and is tracked as a
-    # follow-up rather than done inline.
+    # follow-up rather than done inline.  TODO(#1848): thread the invoking user
+    # through ``get_corpus_description`` once the agent framework injects it,
+    # and drop the ``corpus.creator`` fallback.
     #
     # Defensive: ``corpus.creator`` can be NULL if the creating user was hard-
     # deleted/deactivated. Without a creator there is no identity to scope the

opencontractserver/tests/test_caml_review_tools.py

@@ -1148,7 +1148,7 @@ def test_preview_window_clamps_at_document_boundaries(self):
         self.assertEqual(result["char_offset"], 0)
 
 
-class AapplyCamlArticleEditCreatesVersionTreeSiblingTest(TransactionTestCase):
+class ApplyCamlArticleEditCreatesVersionTreeSiblingTest(TransactionTestCase):
     """Pin Task 8.5: agent CAML edits create a version-tree sibling.
 
     After the refactor, ``_apply_caml_article_edit`` routes its write through