Skip to content

fix(test): preserve colliding diagnostic artifacts - #2507

Open
PLASMA-FR wants to merge 1 commit into
callstack:mainfrom
PLASMA-FR:fix/test-artifact-name-collisions-b-20260911-6i2eaxsf
Open

fix(test): preserve colliding diagnostic artifacts#2507
PLASMA-FR wants to merge 1 commit into
callstack:mainfrom
PLASMA-FR:fix/test-artifact-name-collisions-b-20260911-6i2eaxsf

Conversation

@PLASMA-FR

Copy link
Copy Markdown
Contributor

Summary

agent-device test ./flows --artifacts-dir ./out could silently lose diagnostic files: copying two device.log files generated device-2.log, which a later artifact with that name overwrote. Diagnostics could also overwrite replay.ad or the timing trace, or disappear under the final manifests.

This completes the existing attempt-artifact retention contract from #266 and the replay documentation. The artifact writer now chooses unused numbered names, reserves manifest names, preserves files already in the attempt directory, and deduplicates failure logs. result.txt lists the retained filenames. Five files change, including regressions, help, docs, and changelog.

Validation

Tested 8f6acb16e39cea991df6f1611a43aef5edcbc8a2 with Node 24.13.1 and pnpm 11.17.0 on Linux:

  • Nine new regression cases fail without the fix; all 15 artifact tests pass with it.
  • pnpm exec vitest run over the focused replay/suite/startup files: 87 tests pass.
  • Remote artifact materialization: two provider integration tests pass.
  • Public scheduler probes preserve all seven diagnostics, source, timing trace, manifests, and suite outcome for passing and failing attempts.
  • pnpm build, pnpm check:command-docs, and built test --help pass.
  • pnpm check:affected --run: all runnable checks pass, including 2,801 tests across 384 files.

CI pending; no local device run. The changed boundary copies files after an attempt completes.

@thymikee

Copy link
Copy Markdown
Member

No code findings at 8f6acb1. Artifact allocation preserves colliding diagnostics, existing attempt files and manifests, with regression coverage for ordering and duplicate logs. There are no conflicts; ready for human review. The iOS smoke check still fails because a depth-1 snapshot falls back to XCTest and carries unexpected tree-quality metadata; this appears unrelated to artifact copying, but the check needs to pass before merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants