Skip to content

Create Lean projects atomically - #26

Open
Deicyde wants to merge 42 commits into
facebookresearch:mainfrom
VivienCabannes:split/07-project-create
Open

Deicyde wants to merge 42 commits into
facebookresearch:mainfrom
VivienCabannes:split/07-project-create

Conversation

@Deicyde

@Deicyde Deicyde commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add autoform project new for an absent target and an explicit bundled
    Lean/Mathlib release
  • generate the Lean shell, Autoform blueprint, documentation config, complete
    nine-package Lake lock, and optional commit-pinned workflows without running
    Git, Lake, Lean, subprocesses, or the network
  • serialize cooperative creators and publish with an atomic no-replace rename
  • validate staged files by descriptor, including exact bytes, types, modes,
    link counts, and the initial roadmap; check project compatibility by
    inspecting the stage and confirming it is still the directory held open
  • preserve failed stages for inspection while reporting late or uncertain
    publication states accurately
  • reject package names that shadow module roots used by the selected release
  • document new-project and existing-project setup paths

Depends on #16 and is merged onto its head ce81bef, which carries the
cut-down #14 (merge 0789dc6). Review the diff against #16; earlier commits
are dependency history.

Validation

  • At 0789dc6, after the Add bounded Lean project inspection #14 cut: full suite 1,155 passed, 2 skipped on Python 3.13;
    ruff check autoform_cli servers tests. The lines below were measured at
    94c4604, before that merge.
  • exact-head GitHub CI is green: full Python 3.10 and 3.13 suites (including
    installed-wheel runtime coverage), Windows subprocess/publication hardening,
    and the real-Lean fixture suite
  • local make lint and make check-example
  • complete bundled manifest matches Lake's generated nine-package closure
  • fresh generated project: lake build completed successfully (8,656 jobs)
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 19, 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.

Review result: changes needed.

  • project/create.py:119-157 never revalidates that the requested parent pathname still names the opened directory. A reproduced concurrent rename made project new return success while the project appeared under the moved parent and the requested target remained absent.
  • The sticky-directory check trusts any sticky parent from a pre-open stat. A directory owner can still rename another user's entry; validate ownership and mode on the bound descriptor.
  • Target names permit C0/C1 terminal controls and print them raw.
  • An unpaired surrogate in the target path escapes the structured error contract as UnicodeEncodeError.
  • The command requires POSIX dir-fd, flock, and platform-specific atomic rename support, but the quick start does not disclose that Windows is unsupported.

Revalidate parent identity through a fresh no-follow traversal before publication and after fsync. If identity is lost after rename, report an uncertain commit rather than success.

@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.

Re-reviewed current head 97f46d6. It has the same parent and exact tree (a5f57a9) as previously reviewed commit 3256182; only the committer timestamp changed. There is no code or test delta. The five findings in the prior review therefore remain current and unresolved.

@Deicyde Deicyde added the review: ready Review complete with no known merge blockers label Oct 2, 2026
@Deicyde Deicyde added awaiting author Review is complete and author action is required and removed review: ready Review complete with no known merge blockers labels Oct 2, 2026
@Deicyde Deicyde added review: ready Review complete with no known merge blockers and removed awaiting author Review is complete and author action is required labels Oct 3, 2026
Compatibility now comes from lean-toolchain plus the Mathlib entry that
lake-manifest.json locks (or package-overrides.json replaces), compared
by commit against the catalog. Files are read with plain size-capped
reads, so inspection works on Windows and through symlinked directories
such as macOS /tmp. Drops the descriptor walk, generation snapshots,
bounded TOML, Lean-name lexer, and material-identity tuple.
A differential run of the previous 190-test suite against the slim
inspector, plus checks against real elan 4.2.0 and Lake 4.32.0, showed
behaviours the cut lost:

- lean-toolchain follows elan: the trimmed first line decides, and a
  blank or malformed first line means elan uses the default toolchain
- a direct Mathlib lock that lakefile.toml no longer requires is unused,
  and lakefile.lean projects stay indeterminate
- a catalog match also needs Mathlib loaded from its repository root
  with lakefile.lean and lake-manifest.json; commits compare in any case
- legacy manifest versions are advisory, null packages mean none, NaN
  is rejected, overrides apply only over a manifest, path Mathlib is
  indeterminate
- lakefile.toml needs a name, a StdVer version, named entries, and
  distinct targets, as Lake requires
- credentials in a Mathlib URL are redacted from reports
- the blueprint vault must be a directory; «mathlib» is mathlib
PR 14 no longer uses bounded_toml.py, so the module stays here with the
provenance code that parses pyproject.toml and uv.lock, and its depth
tests move from the old inspection suite to tests/test_bounded_toml.py.
The project command keeps provenance beside the new inspect and versions
paths.
Project inspection no longer exposes a descriptor entry point, so
creation inspects its staged project through the public inspect_project
and then checks that the inspected path is still the directory it holds
open. The release catalog is flat, so the plan and the bundled manifest
check read lean_toolchain, mathlib_git, mathlib_rev, and mathlib_commit.
@Deicyde Deicyde removed the review: ready Review complete with no known merge blockers label Oct 3, 2026
@Deicyde
Deicyde force-pushed the split/07-project-create branch from 33270cd to 0789dc6 Compare October 3, 2026 03:14
@Deicyde Deicyde added the awaiting author Review is complete and author action is required label Oct 3, 2026
@Deicyde

Deicyde commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Current head 0789dc6 is not merge-ready.

  • Renaming the requested parent immediately before the real no-replace publish
    returns ok: true even though the requested path is absent and the project
    landed under the renamed parent.
  • The release module-root exclusion omits at least Mathlib's production
    Archive and Counterexamples libraries; a hardcoded root list is brittle.
  • No test exercises the real no-replace syscall when the destination appears
    late; the only race test mocks the helper.
  • With PYTHONIOENCODING=ascii, human output raises UnicodeEncodeError after
    the project has already been published, producing exit 1 for a committed
    success. Surrogate targets also escape the stable error contract.
  • The branch conflicts with current main in README.md and
    tests/test_skill_examples.py, and has no fresh exact-head test CI despite a
    stale body claiming it is green.

After #14/#16 settle, rebuild the clean #26 delta rather than preserving the
long merge history. Do not restore removed #14 private APIs: semantic validation
should be a pure release/plan check, while exact bytes/types/modes remain bound
to the retained descriptor. Rebind and compare the requested parent dev/inode
immediately before publish and after fsync; generate per-release production
module-root metadata; add a real EEXIST syscall test and ASCII-safe postcommit
output tests. Preserve main's Quick Start and both test families on conflict.

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.

2 participants