feat(visual-check): add --out-dir to write sidecars outside the artifact's folder - #290
Open
julianwirawan-hash wants to merge 1 commit into
Open
Conversation
…act's folder visual-check always wrote its receipt, contact sheet, and 4 screenshots beside the input HTML with no way to redirect them. Projects that keep delivered .json/.html result pairs separate from testing/evidence artifacts (e.g. a docs/ folder with a nested visual-checks/ subfolder) had to manually git mv every output after every single run. Add an optional --out-dir <dir> (also --out-dir=<dir>) that redirects all visual-check sidecars into that directory instead, creating it if missing. Omitting the flag keeps today's behavior byte-for-byte unchanged. Threaded through sidecarPaths() -> runVisualCheck() -> commandVisualCheck(), following the existing extractRepoRootArgs() flag-parsing pattern. Updated the usage string and the delivery-contract reference doc; added unit coverage in visual-check.test.mjs (sidecarPaths + end-to-end runVisualCheck) and cli.test.mjs (the real CLI subprocess, including the missing-value rejection case). Ran the full visual-check.test.mjs (13/13 pass) and cli.test.mjs (39/40 pass; the one failure -- a preview-command server-lifecycle test unrelated to this change -- reproduces identically on unpatched HEAD in this environment).
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.
Problem and value
visual-checkalways writes its receipt, contact sheet, and 4 screenshots beside the input HTML, with no way to redirect them. Projects that keep delivered.json/.htmlresult pairs separate from testing/evidence artifacts (e.g. a docs folder with a nestedvisual-checks/subfolder convention) have to manuallygit mvevery output after every single run. No existing issue link — found this while adopting archify for a project's own diagram set.Scope
--out-dir <dir>(also--out-dir=<dir>) tovisual-checkthat redirects all sidecars into that directory instead, creating it if missing. Threaded throughsidecarPaths()->runVisualCheck()->commandVisualCheck(), following the existingextractRepoRootArgs()flag-parsing pattern already used for--repo-root. Updated the usage string and thedelivery-contract.mdreference doc.bin/archify.mjs,bin/visual-check.mjs,references/delivery-contract.md, and the two touched test files.Stability impact
--out-diris omitted.--out-dirwith no value fails closed (fail('--out-dir requires a directory path.'), matching--repo-root's own behavior) rather than silently ignoring the flag.Tests run
From
archify/:node --test test/visual-check.test.mjs— 13/13 pass (2 new:sidecarPathswithoutDir, end-to-endrunVisualCheck({ outDir })).node --test test/cli.test.mjs— 39/40 pass (2 new: real-subprocess--out-dirredirect, and the missing-value rejection case). The one pre-existing failure (cli: preview runs from an installed skill without node_modules and exits cleanly) reproduces identically on unpatchedmainin my sandboxed environment (a live-server-lifecycle issue, unrelated to this change) — confirmed by running it againstmaindirectly before making any change.npm test(fromarchify/, full suite): could not complete — running the full suite as onenode --test test/*.mjsglob hangs indefinitely in my sandboxed Windows environment (near-zero CPU for 1.5+ hours, confirmed via process inspection, not legitimate progress). This reproduces identically on unpatchedmain, so it predates this change. Every individual test file listed above runs and completes normally.Visual evidence
Not applicable — CLI/file-output behavior only, no rendered diagram or viewer change.
Generated artifacts
archify.zip: left unchanged.scripts/build-zip.shrequires Node 22 exactly for canonical byte-identical output ("CI and releases use Node 22, so fail clearly instead of publishing different bytes from another Node major"); my environment runs Node 24.19.0. Rebuilding here would produce non-canonical bytes, which seemed worse than leaving it stale — happy to rebuild if you'd rather I attempt it anyway, or if a maintainer rebuilds on merge.Checklist
npm testinarchify/. — targeted tests run and pass;npm test's full suite hangs in my environment (pre-existing, see above) so I could not complete it here.archify.zipneeds a Node-22 rebuild I can't produce correctly here (see above).