Skip to content

Publish blueprint sites transactionally - #35

Closed
Deicyde wants to merge 5 commits into
facebookresearch:mainfrom
VivienCabannes:split/12-transactional-publication
Closed

Deicyde wants to merge 5 commits into
facebookresearch:mainfrom
VivienCabannes:split/12-transactional-publication

Conversation

@Deicyde

@Deicyde Deicyde commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • render from bounded immutable blueprint and Lean snapshots into a frozen file plan
  • materialize through retained directory descriptors with no-follow, exclusive writes
  • install new sites atomically and replace owned sites with one verified exchange
  • fsync staged files and every newly created output-parent generation
  • track exact publication inventories for safe cleanup and truthful recovery reporting
  • reject source, stage, destination, and parent-generation changes without rollback after commit begins

Depends on #28. The reviewable change is commit aa789e4; earlier commits belong to the dependency stack.

Validation

  • Python 3.13 full suite: 714 passed, 4 skipped
  • Python 3.10 full suite: 715 passed, 3 skipped
  • final independent focused review: 226 passed
  • make lint
  • make check-example
  • sdist, wheel, and clean installed-CLI smoke
  • git diff --check
  • independent exact-diff review: SHIP

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 21, 2026

@Deicyde Deicyde left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The immutable-plan and atomic-exchange core looks sound, but I found four correctness gaps before this should merge:

  1. parent_changed is sampled before workspace cleanup (render.py:791) and reused afterward. If the output-parent pathname is replaced during cleanup, render_site returns success with warnings=[] even though report.output_dir no longer exists there; the publication is under the renamed old parent. Reverify the retained parent after cleanup before reporting success.

  2. _is_generated_path case-folds root filenames, so an authored blueprint/Progress.md (also Dependencies.md, Structure.md, etc.) is silently excluded on a case-sensitive filesystem. The source remains, but neither that page nor its bytes appear in the publication inventory. Reject noncanonical aliases explicitly, or exclude only the exact generated spelling and let collision validation fail closed.

  3. Ordinary pre-commit failures leak staging directories. _open_workspace_directory runs before the enclosing try, and a materialization failure occurs before stage_cleanup_inventory is assigned. Injected EMFILE left an empty workspace; injected ENOSPC left a partial staged tree. Both are known pre-commit states and should be cleaned through retained descriptors, not accumulate on retries.

  4. The new provenance fields are not actually bound together. Lean/blueprint bytes are captured, then HEAD is detected independently; the final guard checks only working-tree generations. With stable dirty Lean, links use captured line numbers against different committed bytes. With an untracked source note, publication suppresses the local copy and emits a blob/<HEAD>/... URL that 404s. This is the ref-binding defect already raised on #28, but #35's exact-generation claim and manifest pairing depend on its fix.

Validation on the exact head: focused render/graph tests passed (140); full suite passed (714, 4 skipped).

@Deicyde Deicyde added the awaiting author Review is complete and author action is required label Oct 3, 2026
@Deicyde
Deicyde marked this pull request as draft October 3, 2026 00:46
@Deicyde

Deicyde commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Moved to #73. VivienCabannes/autoform-bot is being deleted, so this PR now lives on the same-named branch in facebookresearch/autoform-bot (head aa789e4, unchanged). Please continue review on #73; new commits go to facebookresearch:split/12-transactional-publication, not the fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting author Review is complete and author action is required CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant