fix(traces): include finding.json in the export manifest for flagged traces#63
Open
dark-sorceror wants to merge 1 commit into
Open
fix(traces): include finding.json in the export manifest for flagged traces#63dark-sorceror wants to merge 1 commit into
dark-sorceror wants to merge 1 commit into
Conversation
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.
Summary
Fixes #56
traces exporton a flagged trace writes five files and announces all five — stderr saysWrote 5 files: … finding.json,--jsonlists the same five — butmanifest.json's ownfilesarray only listed the four core files. The bundle's self-describing index disagreed with its own contents: anything iteratingmanifest.filessilently skipped the finding, the most important file in a flagged trace's bundle.How it works
The server has no knowledge of the finding (the CLI fetches it separately), so
response.manifest.filesnever includesfinding.json. The fix computes thefileslist once, up front —BUNDLE_FILESplusfinding.jsonwhen a finding is present — and uses that single array everywhere: it's spread intomanifest.jsonbefore writing (overriding onlyfiles;bundle_version,project_id,trace_idpass through untouched), and it's the same array the stderr summary and--jsonoutput already reported. One source of truth, so the manifest can no longer drift from what the CLI actually announces.What's included
src/commands/traces/export.ts—filesis now computed before the manifest is written (not appended after), andmanifest.json's contents are{ ...response.manifest, files }instead of the raw server manifest.tests/commands/traces.export.test.ts— three new tests: flagged export's manifest lists 5 files and matches the--jsonsummary exactly, withbundle_version/project_id/trace_idpassed through untouched; unflagged export's manifest is byte-identical to the server's;--forcere-export from flagged to unflagged removesfinding.jsonfrom disk and shrinksmanifest.filesback to 4.Commit:
3dabf02fix(traces): include finding.json in the export manifest for flagged tracesTest plan
--json, andmanifest.jsonall agree on 5 files includingfinding.json, withbundle_version/project_id/trace_idintact--json, andmanifest.jsonall agree on 4 files--forcere-export of a flagged trace's directory as unflagged:finding.jsonremoved from disk and from the manifestSummary by cubic
Fixes traces export so
manifest.jsonincludesfinding.jsonfor flagged traces. The manifest now matches the files written and the CLI summaries, so consumers don’t skip the finding.fileslist (BUNDLE_FILES+finding.jsonwhen present) and use it for the manifest and CLI summaries.manifest.jsonas{ ...response.manifest, files }to keepbundle_version,project_id, andtrace_idunchanged.--forcere-export from flagged to unflagged, remove stalefinding.jsonand shrinkmanifest.filesback to 4; tests added for flagged, unflagged, and re-export cases.Written for commit 3dabf02. Summary will update on new commits.