feat(storage): bind strict publication producers - #620
Conversation
github-actions
Bot
added
type/docs
13d3ecc to
9ac426a
Compare
Preserve exact provider callback outcomes, authenticate manifest containment, reroot frozen source paths, and support universal newlines without publication drift. Verified with 319 relevant tests, Python 3.12/3.13 compatibility checks, adversarial path tests, and static checks.
Bind static export manifest comparisons to a verified detached source identity instead of mutable public projections. Keep the restacked PID and publication-race regressions aligned with the hardened authority behavior. Verified with the focused producer/export suite (734 passed, 15 skipped) and changed-file pre-commit hooks.
0971030 to
0eeb10b
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eeb10b4d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if os.path.isabs(candidate): | ||
| if not repo_dir: | ||
| return None | ||
| try: | ||
| candidate = os.path.relpath(candidate, os.path.abspath(repo_dir)) | ||
| except ValueError: |
There was a problem hiding this comment.
When a manifest is consumed from a checkout at a different absolute location than the one where its graph was built, graph vertices can still contain paths such as /old/build/repo/pkg/a.py. The authenticated reader deliberately maps these frozen absolute suffixes to captured files, but this helper instead computes a relative path against the current repo_dir, obtains ../../old/build/..., and marks every such symbol external. Static exports from relocated artifacts consequently publish an incorrect external-only hierarchy even though citations and source excerpts resolve successfully. Use the reader's captured-path mapping (or equivalent suffix resolution) rather than relpath against the current checkout.
Useful? React with 👍 / 👎.
| if os.path.isabs(candidate): | ||
| if not repo_dir: | ||
| return None | ||
| try: | ||
| candidate = os.path.relpath(candidate, os.path.abspath(repo_dir)) | ||
| except ValueError: |
There was a problem hiding this comment.
When a manifest is consumed from a checkout at a different absolute location than the one where its graph was built, graph vertices can still contain paths such as /old/build/repo/pkg/a.py. The authenticated reader deliberately maps these frozen absolute suffixes to captured files, but this helper instead computes a relative path against the current repo_dir, obtains ../../old/build/..., and marks every such symbol external. Static exports from relocated artifacts consequently publish an incorrect external-only hierarchy even though citations and source excerpts resolve successfully. Use the reader's captured-path mapping (or equivalent suffix resolution) rather than relpath against the current checkout.
Useful? React with 👍 / 👎.
| stage = OwnedDirectoryStage.prepare( | ||
| output_dir, | ||
| required_destination_file=STATIC_EXPORT_MANIFEST, | ||
| allow_empty_destination=True, | ||
| ) |
There was a problem hiding this comment.
On Windows, every static export now fails when it reaches this call: OwnedDirectoryStage.prepare() requests _open_publication_authority(..., create_missing=True), whose Windows implementation rejects safe parent creation unconditionally, and the stage itself subsequently relies on POSIX-only directory descriptors and flags. The previous tempfile.mkdtemp plus platform-specific publish_staged_directory path supported Windows, so switching the exporter to this POSIX stage removes the feature for all Windows users even when the output parent already exists.
Useful? React with 👍 / 👎.
| cache_key = (source_path, start_line, stop_line) | ||
| if cache_key not in cache: | ||
| payload = self._source_reader.read_line_range( | ||
| source_path, | ||
| start_line=start_line + 1, | ||
| end_line=stop_line, |
There was a problem hiding this comment.
For source-bound exports, the cache is keyed by the exact symbol range, so each distinct symbol in the same file calls read_line_range() again. That method authenticates by reading and hashing the complete file, making _compute_symbols() perform O(symbol-count × file-size) I/O; for example, a large generated file containing hundreds of indexed symbols is reread hundreds of times before page ranking even limits what will be published. Group ranges or authenticate each file once while retaining the bounded excerpts needed for its symbols.
Useful? React with 👍 / 👎.
1 participant
Summary
Restack the strict publication producer work onto the focused workspace authority from #619 and the source, vector, atomic-cleanup, and resource-ownership foundations already merged to
main.This replacement removes the obsolete stacked history while preserving the producer contracts, authenticated repository reads, and static export hardening.
Changes
Type of Change
Testing
Tests pass locally
Added new tests for the changes
post-feat(storage): expose workspace publication gate #619 restack focused producer/export suite: 754 passed, 15 skipped
all 11 commits are range-diff equivalent to the original replacement stack
changed-range pre-commit hooks
git diff --check origin/main...HEADChecklist
Supersedes the now-closed #591. #619 and its #622 resource-cleanup prerequisite are merged, and this branch is restacked directly onto current
main.