Skip to content

fix(storage): verify publication callbacks after errors - #592

Closed
fishmingyu wants to merge 3 commits into
fix/strict-publication-producers-v2from
fix/publication-callback-postvalidation-v2
Closed

fix(storage): verify publication callbacks after errors#592
fishmingyu wants to merge 3 commits into
fix/strict-publication-producers-v2from
fix/publication-callback-postvalidation-v2

Conversation

@fishmingyu

@fishmingyu fishmingyu commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary\n\nMake directory publication callbacks and authenticated-file cleanup preserve the exact first failure while still running every required postcondition and cleanup action.\n\nThis is a focused stacked PR based on #591 and is a prerequisite for the strict BM25/context producer migration.\n\n## Changes\n\n- add one ordered callback/post-validation runner that always attempts every postcondition\n- preserve the exact first callback or cancellation BaseException and attach later validation failures as diagnostics\n- reject callback results when reader validity, child namespace, ownership, or retained authority checks fail after callback execution\n- apply the same first-primary semantics to authenticated-directory reopen and directory-orphan callbacks\n- run POSIX and Windows authenticated-file finalize/file-close/directory-close actions without replacing the body primary\n- keep the first cleanup error primary when there is no body failure, while still attempting later cleanup actions\n- add fault matrices for callback errors, ambient exceptions, cancellation windows, suppressed reader failures, ownership drift, finalize failures, and multiple close failures\n\n## Type of Change\n\n- [x] Bug fix\n- [ ] New feature\n- [ ] Breaking change\n- [ ] Documentation update\n- [ ] Refactoring\n- [ ] Performance improvement\n- [x] Tests\n\n## Testing\n\n- [x] Tests pass locally\n- [x] Added new tests for the changes\n- Focused changed surface: 418 passed, 10 skipped\n- Authenticated-file POSIX/Windows failure matrix: 10 passed\n- Black, isort with the Black profile, flake8, py_compile, and git diff --check passed\n\n## Checklist\n\n- [x] My code follows the project style guidelines\n- [x] I have performed a self-review of my code\n- [x] I have commented my code, particularly in hard-to-understand areas\n- [x] My changes generate no new warnings\n- [ ] Any dependent changes have been merged and published\n\nDependency: #591 is published, green, and Ready for Review, but intentionally not merged yet.

@github-actions github-actions Bot added the type/test Test-related label Aug 11, 2026
@fishmingyu
fishmingyu marked this pull request as draft August 11, 2026 22:08

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60b2594965

ℹ️ 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".

Comment thread codenib/_atomic_directory.py
Comment thread codenib/_atomic_directory.py Outdated
Comment thread test/test_atomic_directory.py
Comment thread codenib/_atomic_directory.py Outdated
@fishmingyu
fishmingyu force-pushed the fix/publication-callback-postvalidation-v2 branch from 60b2594 to efc6137 Compare August 11, 2026 23:06
@fishmingyu

Copy link
Copy Markdown
Member Author

Updated the stack after #591 moved to b0b508e:\n\n- rebased the original callback-postvalidation commit without content drift (range-diff is exact)\n- added efc6137 to preserve authenticated-file body failures across POSIX/Windows finalize and close failures\n- reran the focused surface: 418 passed, 10 skipped\n- Black, isort, flake8, py_compile, and diff-check are green\n\nThe force update used an explicit lease against the previous remote tip 60b2594. This PR remains Draft until the refreshed remote checks complete.

@fishmingyu
fishmingyu marked this pull request as ready for review August 11, 2026 23:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: efc6137fa7

ℹ️ 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".

Comment thread codenib/_atomic_directory.py Outdated
@fishmingyu
fishmingyu marked this pull request as draft August 11, 2026 23:49
@fishmingyu
fishmingyu force-pushed the fix/strict-publication-producers-v2 branch from b0b508e to 0fb85b1 Compare August 12, 2026 00:25
Run every authenticated-file cleanup action while retaining the exact body exception as the primary failure. Cover POSIX and Windows finalize/close combinations, including cancellation-class exceptions.\n\nVerified with 263 focused atomic, captured-directory, and workspace tests plus Black, isort, flake8, py_compile, and git diff --check.
Resume ordered post-validations and authenticated cleanup after loop-edge cancellation while preserving the exact first failure and running every remaining action.

Harden diagnostic notes against hostile exception subclasses and make the cancellation fault seams work across supported Python versions.
@fishmingyu
fishmingyu force-pushed the fix/publication-callback-postvalidation-v2 branch from efc6137 to 1f987a5 Compare August 12, 2026 00:38
@github-actions github-actions Bot added the type/docs Documentation label Aug 12, 2026

Copy link
Copy Markdown
Member Author

Superseded by #617, which rebases the atomic callback/cleanup slice directly onto current main, includes the downstream completion-aware cleanup correction, and closes additional bounded-retry and retry-owner gaps found during independent review. I am keeping this stacked PR open only until #617 reaches an exact-head green/mergeable state; it should not be merged as-is.

Copy link
Copy Markdown
Member Author

Superseded by #617, which merged into main as 36ee064b802030aec0fda8c302fb9996eaeff048 after exact-head Full CI light and formal pull-request CI both passed. Closing this older stacked implementation so it cannot be merged independently.

@fishmingyu fishmingyu closed this Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/docs Documentation type/test Test-related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant