diff --git a/coulomb_social/apps/spaces/pageops.py b/coulomb_social/apps/spaces/pageops.py index dd01854..0ad7b16 100644 --- a/coulomb_social/apps/spaces/pageops.py +++ b/coulomb_social/apps/spaces/pageops.py @@ -288,6 +288,7 @@ def _recovery_dir(): if path.is_symlink(): raise PageOpsError("Content paths must not be symlinks") path.mkdir(parents=True, exist_ok=True) + _fsync_directory(path.parent) return path @@ -660,7 +661,7 @@ _BODY_ASSET = re.compile( ) -def _copy_assets(source_space, dest_space, dest_page, visual, body, on_created=None): +def _copy_assets(source_space, dest_space, dest_page, visual, body, before_create=None): """Copy local Visual and Markdown link/image destinations as one staged set.""" from .assets import asset_parts, open_local_asset @@ -691,14 +692,25 @@ def _copy_assets(source_space, dest_space, dest_page, visual, body, on_created=N ) dest_name = f"{dest_page}{suffix}-{parts[-1]}" with source_file: + destination = assets_dir(dest_space) / dest_name + # The enclosing content writer lock serializes asset creation. + # Refuse existing entries before recording cleanup authority, + # then persist intent before any partial destination can exist. + try: + destination.lstat() + except FileNotFoundError: + pass + else: + raise FileExistsError("Destination attachment already exists") + if before_create is not None: + before_create(dest_name) with open_local_asset(dest_space, dest_name, create=True) as stream: - created.append(assets_dir(dest_space) / dest_name) + created.append(destination) shutil.copyfileobj(source_file, stream) stream.flush() os.fsync(stream.fileno()) + _fsync_directory(destination.parent) copied[path] = f"assets/{quote(dest_name)}" - if on_created is not None: - on_created(dest_name) suffix = (f"?{parsed.query}" if parsed.query else "") suffix += f"#{parsed.fragment}" if parsed.fragment else "" return copied[path] + suffix diff --git a/docs/release/content-lifecycle-validation-2026-10-31.json b/docs/release/content-lifecycle-validation-2026-10-31.json new file mode 100644 index 0000000..f426e59 --- /dev/null +++ b/docs/release/content-lifecycle-validation-2026-10-31.json @@ -0,0 +1,58 @@ +{ + "schema": "coulomb.content-lifecycle-validation.v1", + "recorded_at": "2026-10-31T11:54:24.215512+00:00", + "status": "source_fix_verified", + "workplan_task": "CSOC-WP-0013-T05", + "bug": "A process exit during attachment creation left bytes absent from the copy/move cleanup journal; the orphan blocked retry.", + "regression_observed_before_fix": { + "baseline": "f70708839cd5ca43310f6891c8d94aaff915139c", + "operations": [ + "copy_page", + "transfer_page" + ], + "interruption": "partial first attachment flushed, immediate os._exit(86)", + "result": "both failed because the partial destination survived recovery" + }, + "fix": [ + "Refuse existing attachment entries before recording cleanup authority.", + "Persist intended new attachment name before exclusive creation under the content writer lock.", + "Flush recovery and attachment directory entries before destination publication." + ], + "focused_cases": { + "total": 29, + "abrupt_child_process_exit": 14, + "existing_destination_collision": 8, + "prior_retention_and_recovery": 7 + }, + "full_gate": { + "command": "make check", + "tests": 546, + "failures": 0, + "errors": 0, + "skipped": 0, + "warnings": 69, + "junit_sha256": "dd3d0433c8109f00f6dddd119cfdf8281915f94da2da1f5e6009c31cfe76c65b", + "log_sha256": "eb59a394c95c5307c17069874640d1655ef61c5aa7bb426ef6d21db4929f748e" + }, + "files": [ + { + "path": "coulomb_social/apps/spaces/pageops.py", + "sha256": "055ef25b5efc75798cf17ffa68739de4d4099d2cab4d041e7c7eacdfaf43803e" + }, + { + "path": "tests/test_content_recovery.py", + "sha256": "84a25a22a3cbda397e39c0508db8ea5aa90e6c467ccfa7da2b568d174d1ec449" + } + ], + "policy": "User accepted retain-all default and separately reviewed manual cutoff; keep live page and all history of deleted pages.", + "live_purge_authorized": false, + "live_purge_executed": false, + "production_deployed": false, + "productive_acceptance": false, + "limits": [ + "Child process termination on synthetic local files is proved; physical power-loss durability is not experimentally proved.", + "Cooperating application/import writers must use the content writer lock.", + "Release delivery and productive copy/move review remain open." + ], + "policy_decision_id": "c3a191f2-ce06-483f-82d7-ef8c54ea0d5e" +} diff --git a/docs/usecases/sharing-portability-review.md b/docs/usecases/sharing-portability-review.md index 3522b9b..683d00c 100644 --- a/docs/usecases/sharing-portability-review.md +++ b/docs/usecases/sharing-portability-review.md @@ -3,11 +3,12 @@ Review on `https://app.coulomb.social`, using a disposable page in an intended space and a second destination space. Use an explicitly chosen reader and an ungranted same-tenant member; keep their identities and real notes private. -The storage release is deployed and independently verified at Helm revision 18, -image digest `512819463a1902b43a615beee14be7bf97e6a05e016385082b9380692ad1a8cd` -(source 17d0eb6). See the [release evidence](../release/storage-release-review-2026-10-31.md). Record the -actual release/date with each verdict. Later source changes are not implicitly -part of that image. Automated checks do not supply these verdicts. +The last independently verified application release is Helm revision 19, image +`522bcd7fce92436765a60c1e4afa2598f3cc40913352153ff1c4319a99ca92ed` +(source 23ffe2c). See the [release return](../release/identity-export-release-return-2026-10-31.md). +Record the actual release/date with each verdict. Later source changes, including +the attachment interruption fix below, are not implicitly part of that image. +Automated checks do not supply these verdicts. | Journey | Review steps | Expected result | | --- | --- | --- | @@ -47,3 +48,35 @@ Review the body-missing records as source exceptions; never substitute an empty page. Card samples disable resource loading and interaction, so image/resource fidelity needs the separate authorized asset review. Then apply UC-003–006 above to disposable pages on the exact intended release. + +## Revision and interrupted-operation lifecycle — WP-0013-T05 + +Policy accepted by Bernd on 2026-10-31 (decision c3a191f2-ce06-483f-82d7-ef8c54ea0d5e): retain every page revision by default. +There is no scheduled revision purge. A separately reviewed manual purge may remove +only inactive revisions older than an explicit timezone-aware cutoff. Keep current +page bytes and all retained revisions of deleted pages. Validate every revision +hash before deleting any candidate. Backup copies have their own T04 retention +obligations; purging a local revision does not remove it from a backup. Policy +acceptance and any concrete deletion authorization are separate decisions. + +Policy acceptance is recorded. No cutoff has been selected +and no productive purge has run. Local history/retention does not replace independent +paired recovery or the requirement to preserve original source creation dates. + +Source investigation reproduced a process-crash gap in attachment copy/move: +partial attachment bytes could exist before their name entered the recovery journal. +The corrected source records the intended new attachment before exclusive creation, +after refusing any existing entry, under the existing content writer lock. Attachment +bytes and directory entries are flushed before publishing the destination page. +Recovery discards unconfirmed destinations or finishes a confirmed move, preserving +the source unless a matching destination is confirmed. It never grants destination +access through an old address. All application/import writers must use the same +content lock; raw concurrent filesystem edits are outside that serialization model. + +Synthetic subprocesses terminate abruptly at 14 copy/move interruption points, +including first/second partial attachments, page publication, alias publication and +source deletion. Eight collision cases preserve existing files, directories and +symlinks. Existing retention/incoming-address permission tests remain required. +Evidence: `../release/content-lifecycle-validation-2026-10-31.json`. This is source +and isolated-fixture evidence; the fix still needs release delivery and productive +review. Do not kill or mutate the productive application to perform this review. diff --git a/tests/test_content_recovery.py b/tests/test_content_recovery.py index 63ef72f..cef858f 100644 --- a/tests/test_content_recovery.py +++ b/tests/test_content_recovery.py @@ -156,3 +156,119 @@ def test_purge_stops_before_deleting_when_a_revision_hash_does_not_match(trees): with pytest.raises(pageops.PageOpsError, match="integrity"): pageops.purge_revisions(older_than=past) assert len(list((trees / ".revisions" / "source" / "notes").glob("*.md"))) == 2 + + +@pytest.mark.parametrize('operation,phase', [ + (operation, phase) + for operation in ('copy_page', 'transfer_page') + for phase in ('before_asset_create', 'during_first_asset', 'during_second_asset', + 'after_assets', 'after_page', 'published') +] + [('transfer_page', phase) for phase in ('aliased', 'source_deleted')]) +def test_process_death_during_copy_or_move_is_recoverable(trees, operation, phase): + import subprocess + import sys + + source = _notes('Retained content [attachment](assets/second.txt)') + (pageops.assets_dir('source') / 'second.txt').write_bytes(b'synthetic attachment') + original = Path(source.path).read_bytes() + child = r""" +import os, sys +from django.conf import settings +settings.configure(CONTENT_ROOT=sys.argv[1]) +from coulomb_social.apps.spaces import pageops, assets +phase = sys.argv[3] + +def stop(): + os._exit(86) + +original_open = assets.open_local_asset +def opening(*args, **kwargs): + if kwargs.get('create') and phase == 'before_asset_create': + stop() + return original_open(*args, **kwargs) +assets.open_local_asset = opening + +original_copy, count = pageops.shutil.copyfileobj, 0 +def copying(source, destination): + global count + count += 1 + if phase == 'during_first_asset' or (phase == 'during_second_asset' and count == 2): + destination.write(source.read(4)) + destination.flush() + os.fsync(destination.fileno()) + stop() + original_copy(source, destination) +pageops.shutil.copyfileobj = copying + +original_write = pageops.write_page +def writing(*args, **kwargs): + if phase == 'after_assets': + stop() + result = original_write(*args, **kwargs) + if phase == 'after_page': + stop() + return result +pageops.write_page = writing + +original_record = pageops._write_record +def recording(record): + result = original_record(record) + if record['phase'] == phase: + stop() + return result +pageops._write_record = recording + +original_delete = pageops.delete_page +def deleting(*args, **kwargs): + original_delete(*args, **kwargs) + if phase == 'source_deleted': + stop() +pageops.delete_page = deleting +getattr(pageops, sys.argv[2])('source', 'notes', 'destination', 'notes') +""" + result = subprocess.run([sys.executable, '-c', child, str(trees), operation, phase], + capture_output=True, timeout=15) + assert result.returncode == 86, result.stderr.decode() + assert pageops.recover_interrupted_content() == {'cleared': 1, 'held': 0} + published = phase in {'published', 'aliased', 'source_deleted'} + if published and operation == 'transfer_page': + assert not Path(source.path).exists() + assert pageops.page_aliases('source')['notes'] == ['destination', 'notes'] + else: + assert Path(source.path).read_bytes() == original + assert pageops.page_aliases('source') == {} + assert pageops.read_page('destination', 'notes').exists == published + if not published: + assert not list(pageops.assets_dir('destination').iterdir()) + copied = getattr(pageops, operation)('source', 'notes', 'destination', 'notes') + assert copied.exists + destination = pageops.assets_dir('destination') + assert (destination / 'notes-diagram.png').read_bytes() == b'synthetic image' + assert (destination / 'notes-second.txt').read_bytes() == b'synthetic attachment' + assert not list((trees / '.recovery').glob('*.json')) + assert pageops.recover_interrupted_content() == {'cleared': 0, 'held': 0} + + +@pytest.mark.parametrize('operation', ['copy_page', 'transfer_page']) +@pytest.mark.parametrize('kind', ['file', 'directory', 'symlink', 'dangling_symlink']) +def test_existing_attachment_never_becomes_recovery_cleanup_authority(trees, operation, kind): + source = _notes() + original = Path(source.path).read_bytes() + destination = pageops.assets_dir('destination') / 'notes-diagram.png' + if kind == 'file': + destination.write_bytes(b'existing attachment') + elif kind == 'directory': + destination.mkdir() + else: + target = (pageops.assets_dir('source') / 'diagram.png' if kind == 'symlink' + else trees / 'missing') + destination.symlink_to(target) + before = destination.lstat() + with pytest.raises(pageops.PageOpsError, match='Cannot copy'): + getattr(pageops, operation)('source', 'notes', 'destination', 'notes') + assert destination.lstat() == before + if kind == 'file': + assert destination.read_bytes() == b'existing attachment' + assert Path(source.path).read_bytes() == original + assert not pageops.read_page('destination', 'notes').exists + assert pageops.recover_interrupted_content() == {'cleared': 0, 'held': 0} diff --git a/workplans/CSOC-WP-0013-sharing-content-hardening.md b/workplans/CSOC-WP-0013-sharing-content-hardening.md index be2bbd9..83b915d 100644 --- a/workplans/CSOC-WP-0013-sharing-content-hardening.md +++ b/workplans/CSOC-WP-0013-sharing-content-hardening.md @@ -196,13 +196,26 @@ Captured-source audit and private source/product cards cover 245 records/242 bod ```task id: CSOC-WP-0013-T05 -status: todo +status: progress priority: high state_hub_task_id: "c2986a84-4da2-5fb3-b021-7bbe5104fe47" ``` Explicit lifecycle portion of T02: review the existing revision purge/retention policy and failed multi-file copy/move recovery, preserve source/incoming-address authorization, and obtain the required operational/product acceptance. Reuse existing source and crash-recovery tests; investigate concrete gaps before adding implementation. No live purge is authorized by task creation. Close with the agreed policy and evidence for the intended lifecycle. +2026-10-31 lifecycle investigation: actual child-process termination reproduced an +orphaned attachment in both copy and move: destination bytes were created before +the cleanup journal recorded the filename. Corrected source persists intent before +exclusive creation, refuses existing destination entries before recording cleanup +authority, and flushes attachment/recovery directory entries before publication. +Fourteen abrupt-exit scenarios exercise rollback/finish/retry across the multi-file +operation; eight collision cases preserve existing entries. This advances T05 from +todo to progress. Evidence: docs/release/content-lifecycle-validation-2026-10-31.json. +The review guide separates retention-policy acceptance, exact purge authorization, +release delivery and productive review. User accepted the retain-all/manual-cutoff policy; decision c3a191f2-ce06-483f-82d7-ef8c54ea0d5e. No cutoff, +production purge, image deployment or user acceptance is inferred. The current +verified release remains revision19/image522bcd7f until a new owner return. + ## Completion Finish when every task above has its stated evidence and acceptance, or hand off any newly discovered residual to a live work record first. Cross-owner dependencies remain their existing canonical records.