You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(artifacts): keep saved findings consistent through recovery - #1310
Use consistent finding identities and source ordering across publication, interruption, and resume, while preserving finalized artifacts.
Changes
Reconcile revisions without collapsing independent reports at the same location.
Retain unfinished work, legacy identities, annotations, and immutable saved sources.
Preserve artifact aliases and report export failures through the existing diagnostics.
Testing
Runtime validation on ded8582f8d62 passed both full SDK orders, 884 Python tests with 69 skips and 100 subtests, and 900 selected MCP cases on rerun. The final 49e0c1cde6cd follow-up only explains the existing optional previous-head fallback and moves fixture mutations outside assertions; its production AST is identical. All 96 focused identity tests, portable checks, types, and formatting passed again. Three fresh native reviews and independent source verification found no blocking issues. Hosted CI supplies native macOS and Windows coverage.
Risk and rollout
The main risk is inconsistent matching between TypeScript publication and Python recovery for ambiguous saved observations. The regressions check finalized findings, revision history, frozen-source replay, and unchanged source bytes. Existing artifact formats and CLI behavior are retained. Local checks ran on Linux; native macOS and Windows results remain subject to hosted CI. Portable filesystem tests adapt to the actual volume's alias behavior.
Public disclosure review
No customer, partner, prospect, or user identities, data, or identifying details are included.
No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the diff and relevant callers at 7dce657, with an independent second pass. No actionable findings.
Focused validation: All production changes and current historical comments reviewed. Focused Python identity/finalization/progress suites: 263 tests and 70 subtests passed, zero failures, exit 0. A duplicate run also passed; an initially loaded ddtrace plugin emitted unrelated sandbox host-inspection warnings at shutdown.
Validation limits: No native Windows or full SDK/MCP suite. The reported hash-suffix identity collision allocation predates this diff; the retained hash branch is unchanged behavior.
CI still has failed, canceled, or unfinished checks. This source-review approval does not clear those checks.
When an imported sealed manifest contains a remote such as https://exa\nmple.com/repo (similarly with a tab or carriage return), the WHATWG URL constructor silently strips that character, so every check here passes even though the original, unmodified remote is returned and can later be rendered in publication output. The Python contract reader explicitly rejects these controls via REMOTE_CONTROL_RE, so the SDK accepts artifacts the producer rejects and admits terminal controls; validate the original string against the same control-character class before parsing it.
mldangelo-oai
changed the title
fix(artifacts): preserve findings and finalized artifacts during scan recovery
fix(artifacts): keep saved findings consistent through recovery
Oct 12, 2026
This reconciliation phase adds more than 500 lines of nested, dictionary-shaped state to an already large merge_saved_results function, combining causal source selection, identity matching, revision ranking, and publication ordering in one scope. That makes the recovery invariants difficult to review or change safely. Please extract this into focused reconciliation helpers/types (for example, a structured row-group model plus separate matching and identity-assignment phases) while keeping merge_saved_results as orchestration.
The reason will be displayed to describe this comment to others. Learn more.
Apply the draft predicate before custom validation
When an unsealed custom-validation draft contains artifacts: [], this new predicate now deliberately treats it as unfinished, but runCustomValidation still rejects any defined manifest.scan.artifacts at sdk/typescript/src/custom-validation.ts:216-220. Such a draft can therefore pass the revised completion/resume checks yet fail before custom validation runs; update that gate to use the same empty-array semantics.
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
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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
zcrab-oai
left a comment
Reject control characters before parsing remotes
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Summary
Use consistent finding identities and source ordering across publication, interruption, and resume, while preserving finalized artifacts.
Changes
Testing
Runtime validation on
ded8582f8d62passed both full SDK orders, 884 Python tests with 69 skips and 100 subtests, and 900 selected MCP cases on rerun. The final49e0c1cde6cdfollow-up only explains the existing optional previous-head fallback and moves fixture mutations outside assertions; its production AST is identical. All 96 focused identity tests, portable checks, types, and formatting passed again. Three fresh native reviews and independent source verification found no blocking issues. Hosted CI supplies native macOS and Windows coverage.Risk and rollout
The main risk is inconsistent matching between TypeScript publication and Python recovery for ambiguous saved observations. The regressions check finalized findings, revision history, frozen-source replay, and unchanged source bytes. Existing artifact formats and CLI behavior are retained. Local checks ran on Linux; native macOS and Windows results remain subject to hosted CI. Portable filesystem tests adapt to the actual volume's alias behavior.
Public disclosure review