Repository navigation
feat(server)!: log an unknown publication's graph commit as its own field - #1124
Merged
aaltshuler merged 11 commits intoOct 28, 2026
Merged
Conversation
An in-doubt publication's graph commit lived only in its error message, which the server never logs because a message can carry storage paths. `ManifestError` now carries it as `publication_id`, the in-doubt constructor requires it, and `OmniError::publication_id` reads it through completion evidence and the DataFusion stream carrier. The server's `engine failure without definite completion evidence` line names it in a `publication` field, so an operator can look the commit up later.
…field The separate `publication_id: Option<String>` beside the `publication_in_doubt` flag grew `OmniError` to 128 bytes, clippy's large-error bound for every `Result<_, OmniError>`. The flag and the id now are one field, `publication_in_doubt: Option<Box<String>>`: present means a publication is in doubt and names its graph commit. One word keeps `OmniError` at 120 bytes, and one field cannot disagree with itself.
This was referenced Oct 21, 2026
aaltshuler
changed the title
aaltshuler
marked this pull request as ready for review
October 28, 2026 17:32
4 of 7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1067 and the accepted storage-outcomes RFC #1093. An unknown publication's graph commit previously appeared only in a diagnostic message that the server deliberately redacts. Carry the commit separately through the manifest error and log it as
publication, so operators can look up whether the attempted publication landed without exposing storage paths.The constructor requires the existing commit ID at every in-doubt publication site. The accessor retains it through completion evidence and the DataFusion error carrier; unrelated uncertain failures return no ID. The warning uses the server's existing final settled error path, so its status agrees with the response and it is emitted once. Publication, retry, retention and recovery semantics are unchanged.
This changes the Rust source API exported in 0.14.0:
manifest_publish_in_doubtnow takes(graph_commit_id, message), andManifestError.publication_in_doubtchanges frombooltoOption<Box<String>>. Rust embedders must update constructor calls and direct field access; the storage-failures guide and breaking release note describe the change. HTTP error responses and CLI syntax are unchanged.The branch includes current main's telemetry and OIDC admission changes. The telemetry uncertainty fixture uses the new constructor, and the existing OIDC and OTLP release notes have been shortened to restore release-note validation without changing their contracts.
On head
87b1d8e7, all 370 CLI and 225 server library tests pass, with 11 existing ignored nightly/helper tests. The existing publication-log owner covers text and JSON, exact ID presence, absence on unrelated uncertainty, and storage-path redaction; the settled-status and OTLP uncertainty owners also pass. Before the fix, the publication-log owner failed on main4f47b976because the field was missing. Formatting, AGENTS links and documentation checks pass. Required CI passed at this exact head, including the full workspace and failpoint tests (covering the core/catalog publication owners), Clippy, GQT, pinned DST, AWS/server, and storage/Azure integration checks.