[CSR-3436] feat: support artifacts upload#318
Open
miguelangaranocurrents wants to merge 29 commits intomainfrom
Open
[CSR-3436] feat: support artifacts upload#318miguelangaranocurrents wants to merge 29 commits intomainfrom
miguelangaranocurrents wants to merge 29 commits intomainfrom
Conversation
vCaisim
requested changes
Feb 20, 2026
Collaborator
vCaisim
left a comment
vCaisim
left a comment
There was a problem hiding this comment.
@miguelangaranocurrents nice job! I tested with server changes - works well.
Notes:
- please carefully review the generated
mdfiles, leave only one and adjust it to the documentation style - add more artifacts types for
vitestexample - it will help to test better theconvertcommand. Thejestexample is good.
vCaisim
requested changes
Feb 23, 2026
Collaborator
vCaisim
left a comment
vCaisim
left a comment
There was a problem hiding this comment.
@miguelangarano nice job.
I left some small refactor notes.
Still need to be addressed:
- please carefully review the generated md files, leave only one and adjust it to the documentation style
- add more artifacts types for vitest example - it will help to test better the convert command. The jest example is good.
Questions:
- shouldn't the stdout also be uploaded? The stdout was written to a txt file, but I don't see the content in the dashboard.
Contributor
Author
Removed not required md files
Added more, specifically for attachments
fixed now |
📝 WalkthroughWalkthroughThis PR introduces comprehensive artifact attachment functionality to the Currents test reporting system. It adds helper functions for attaching artifacts (videos, screenshots, files) at spec/test/attempt levels, implements artifact processing pipelines for Jest and Vitest reporters, enables artifact conversion from JUnit XML, and provides artifact upload capabilities. Includes examples, documentation, and type definitions across Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant API as attachArtifact API
participant FS as File System
participant Store as .currents-artifacts
participant Reporter as Jest Reporter
participant Output as Report JSON
Test->>API: attachArtifact(path, type, contentType, level)
API->>FS: Read artifact file
Note over API: Infer type/contentType from extension if needed<br/>Compute MD5 hash of test path<br/>Get current attempt via Jest state
API->>Store: Append JSONL with artifact metadata
API->>Test: Return (silent on error)
Note over Reporter: Test execution completes
Reporter->>Store: Read .currents-artifacts for test file
Reporter->>FS: Parse artifact metadata from JSONL
Note over Reporter: Group artifacts by test + attempt<br/>Map test titles to line numbers<br/>Deduplicate and cleanup
Reporter->>Reporter: Process spec/test/attempt artifacts
Reporter->>FS: Copy artifacts to artifacts/ dir with hashes
Reporter->>Output: Build InstanceReport with artifact references
sequenceDiagram
participant JUnit as JUnit XML Report
participant Convert as Conversion Service
participant Props as Property Parser
participant Logs as Log Parser
participant FS as File System
participant API as Server API
participant Upload as Upload Service
JUnit->>Convert: Parse XML with test properties
Convert->>Props: Extract currents.artifact.* properties
Props->>Convert: Return Artifact objects (spec/test/attempt)
Convert->>Logs: Extract [[CURRENTS.ATTACHMENT]] markers<br/>and currents.artifact JSON blocks
Logs->>Convert: Return log-based Artifact objects
Note over Convert: Create artifacts/ directory
Convert->>FS: Copy each artifact file<br/>Hash filename<br/>Update artifact.path
FS->>Convert: Return new relative path
Convert->>Convert: Build InstanceReport with artifacts
Convert->>FS: Write instances/*.json<br/>with embedded artifact refs
Convert->>API: Create run with instances
API->>Convert: Return artifactUploadUrls
Convert->>Upload: Process chunks with upload URLs
Upload->>FS: Read artifact file content
Upload->>API: PUT to uploadUrl for each artifact
API->>API: Store artifact binary
Upload->>Upload: Aggregate stdout per instance
Upload->>API: Upload aggregated stdout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The PR introduces a substantial, multi-faceted artifact system spanning type definitions, client-side collection (Jest), server-side conversion (CMD), and upload orchestration. While changes follow consistent patterns, they involve dense logic in artifact preparation, parsing from multiple sources (logs, properties), file hashing, attempt tracking via Jest internals, and integration across multiple lifecycle points. The heterogeneity—from simple type additions to complex state management in the reporter—demands careful review of interactions between components. Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/jest/src/reporter.ts (1)
250-273:⚠️ Potential issue | 🟡 MinorRemove unnecessary
asynckeyword from forEach callback.The callback contains no
awaitstatements or async operations. Theasynckeyword makes the callback return an unhandled Promise thatforEachnever awaits. This is a code smell that can mislead developers into thinking async work should occur here or will be safely handled. While currently harmless, it's an anti-pattern that should be removed.Suggested fix
- testResult.testResults.forEach(async (testCaseResult) => { + testResult.testResults.forEach((testCaseResult) => { const testId = getTestCaseId(test, testCaseResult);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jest/src/reporter.ts` around lines 250 - 273, The forEach callback passed to testResult.testResults.forEach is marked async but contains no awaits, causing it to return unused Promises; remove the unnecessary async modifier from the callback so it is a plain synchronous function that still calls getTestCaseId, getTestCaseKey and mutates this.specInfo[specKey].testCaseList and this.resultsDeferred[testCaseKey] as before (preserve all existing logic inside the callback).
🧹 Nitpick comments (21)
examples/jest/src/artifacts/artifacts.test.ts (3)
59-59: Movejest.retryTimesto the top of the file.Placing
jest.retryTimes(2)at the end of the file after the test definitions is unconventional and may confuse readers. Jest configuration calls are typically placed at the top of the file for visibility.♻️ Proposed fix
import * as fs from 'fs'; import * as path from 'path'; import { attachFile } from '@currents/jest'; + +jest.retryTimes(2); const artifactsDir = path.join(__dirname, '..', '..', 'artifacts');And remove from the end:
-jest.retryTimes(2);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/jest/src/artifacts/artifacts.test.ts` at line 59, Move the jest.retryTimes(2) call from the end of artifacts.test.ts into the top-level setup area of the file (above the test suite definitions) so Jest configuration is visible immediately; remove the trailing jest.retryTimes(2) call at the bottom to avoid duplicate/confusing placement and ensure the retry setting applies globally to the tests in this file.
34-36: Remove shadowed variable declaration.
artifactsDiron line 35 shadows the module-level variable declared on line 5. Since they have the same value, use the existing variable.♻️ Proposed fix
it('generates artifacts for multiple attempts', () => { - const artifactsDir = path.join(__dirname, '..', '..', 'artifacts'); const attemptFile = path.join(artifactsDir, 'jest-attempt-count.txt');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/jest/src/artifacts/artifacts.test.ts` around lines 34 - 36, The local declaration of artifactsDir inside the test "generates artifacts for multiple attempts" shadows the module-level artifactsDir; remove the const artifactsDir = ... so the test reuses the top-level artifactsDir variable and update any dependent expressions (e.g., attemptFile) to reference the existing artifactsDir; locate this in the test function in artifacts.test.ts and delete the redundant variable declaration to avoid shadowing.
36-43: Consider cleanup of state file between test suite runs.The
jest-attempt-count.txtfile persists across test runs, which could cause unexpected behavior if the test suite is run multiple times without cleanup. Consider adding cleanup in anafterAllhook or documenting this behavior.♻️ Proposed fix - add cleanup
+afterAll(() => { + const attemptFile = path.join(artifactsDir, 'jest-attempt-count.txt'); + if (fs.existsSync(attemptFile)) { + fs.unlinkSync(attemptFile); + } +}); + describe('Artifacts Test', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/jest/src/artifacts/artifacts.test.ts` around lines 36 - 43, The test currently persists a state file via attemptFile and increments attempt across runs; add teardown to remove or reset that file after the suite so repeated runs start clean. Update the test file (artifacts.test.ts) to add an afterAll (or afterEach if appropriate) hook that checks for attemptFile (the same path/variable used when creating jest-attempt-count.txt) and deletes it (or writes "0") using fs.unlinkSync or fs.writeFileSync, ensuring the attempt counter logic (attempt variable and its read/write) runs against a fresh state on subsequent runs.docs/artifact-workflow.md (1)
353-364: Add a language specifier to the code block.The fenced code block for the directory structure is missing a language identifier. While this is a minor formatting issue, adding a specifier improves rendering consistency.
📝 Proposed fix
-``` +```text .currents/ ├── {TIMESTAMP-UUID}/ # Unique Report ID🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/artifact-workflow.md` around lines 353 - 364, The fenced code block showing the directory tree starting with ".currents/" is missing a language specifier; update that fenced block to include a language identifier (e.g., change ``` to ```text) so the directory structure (the block beginning with ".currents/") renders consistently.examples/jest/src/artifacts/media.test.ts (1)
1-4: Consider using ES module imports for TypeScript consistency.This TypeScript file uses CommonJS
require()syntax. For consistency with TypeScript conventions and better type inference, consider using ES module imports.♻️ Proposed fix
- -const fs = require('fs'); -const path = require('path'); -const { attachVideo, attachScreenshot } = require('@currents/jest'); +import * as fs from 'fs'; +import * as path from 'path'; +import { attachVideo, attachScreenshot } from '@currents/jest';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/jest/src/artifacts/media.test.ts` around lines 1 - 4, Replace the CommonJS requires with ES module imports to keep TypeScript consistency: change the top-of-file requires for fs and path and the import of attachVideo and attachScreenshot from '@currents/jest' to ES module import statements, updating any uses to match (refer to the variables fs, path and the functions attachVideo and attachScreenshot to locate where to change).packages/jest/src/artifacts.ts (2)
283-301: Add debug logging for JSON parse failures in artifact parsing.Similar to the spec artifact parsing, this empty catch block silently swallows errors, making debugging difficult.
♻️ Proposed fix
if (log.key === 'JSON_ARTIFACT') { try { const artifact = JSON.parse(log.value); // ... artifact processing ... - } catch (e) {} + } catch (e) { + debug('Failed to parse JSON artifact: %s - %o', log.value, e); + } continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jest/src/artifacts.ts` around lines 283 - 301, The empty catch swallowing JSON.parse errors for 'JSON_ARTIFACT' makes debugging impossible; update the catch in the loop that handles log.key === 'JSON_ARTIFACT' to log the parse failure (include the thrown error and the offending log.value and/or log) instead of ignoring it — use the same logger used elsewhere in this module (e.g., processLogger.error or the module's debug logger) so you get a descriptive message whenever JSON.parse(log.value) fails while populating jsonArtifacts.
186-194: Add debug logging for JSON parse failures.The empty catch block silently swallows JSON parse errors. Adding debug logging would help troubleshoot malformed artifact entries.
♻️ Proposed fix
try { const artifact = JSON.parse(log.value); if (artifact.path && artifact.type && artifact.contentType && artifact.level === 'spec') { jsonArtifacts.push(artifact); } - } catch (e) {} + } catch (e) { + debug('Failed to parse JSON artifact from log: %s - %o', log.value, e); + } continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jest/src/artifacts.ts` around lines 186 - 194, The catch block swallowing JSON.parse errors for entries where log.key === 'JSON_ARTIFACT' should be replaced with debug logging: inside the catch for JSON.parse(log.value) log the parse error and relevant context (e.g., the offending log.value or a truncated version and the log key) instead of doing nothing so malformed artifact entries are visible during debugging; update the catch in the loop that iterates over logs (references: logs, log.key, 'JSON_ARTIFACT', JSON.parse, jsonArtifacts) to emit a debug message via the project's logger or console.debug including the error and identifying information.packages/jest/src/lib/fs.ts (1)
60-68: AligncopyFileAsyncwith the equivalent function inpackages/cmd/src/lib/fs.tsby ensuring the destination directory exists.This function lacks the directory existence check that the similar function in
packages/cmd/src/lib/fs.ts(lines 90) includes. While current callers pre-create the artifacts directory, adding this safety check makes the function more robust and consistent across the codebase.♻️ Proposed fix
+import { dirname } from 'path'; + export async function copyFileAsync(source: string, destination: string) { try { + await fs.ensureDir(dirname(destination)); await fs.copy(source, destination); debug('File copied from %s to %s', source, destination); } catch (err) { error(`Error copying file from ${source} to ${destination}:`, err); throw err; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jest/src/lib/fs.ts` around lines 60 - 68, copyFileAsync currently copies a file without ensuring the destination directory exists, unlike the counterpart in packages/cmd; update copyFileAsync to compute the destination directory (using path.dirname(destination)) and call the fs.ensureDir (or equivalent) on that directory before calling fs.copy, preserving the existing try/catch and debug/error logging in the same function so the operation is robust if callers don't pre-create the artifacts directory.packages/jest/src/types.ts (1)
44-52: Align the Artifact type definitions across jest and cmd packages to prevent data loss during serialization.The Artifact interfaces differ: jest uses
type: ArtifactTypewith anattempt?: numberfield and namedArtifactLevel, while cmd usestype: stringwith an inline level union and noattemptfield. Although packages don't directly import each other's types, if artifacts are serialized and consumed across package boundaries, cmd's interface won't account for theattemptfield or the stricter typing.Recommend aligning the interfaces (preferably by adopting jest's more type-safe structure) or adding explicit type conversions at any integration points.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jest/src/types.ts` around lines 44 - 52, The Artifact interface in packages/jest (Artifact, ArtifactType, ArtifactLevel, attempt?) is stricter than the cmd package's artifact shape (which uses type: string, inline level union, and lacks attempt), causing potential serialization/deserialization data loss; align the definitions by updating the cmd package to use the same exported types/names (Artifact, ArtifactType, ArtifactLevel) or export a shared artifact type and replace the cmd inline union and string-typed type with the shared types, and ensure the optional attempt?: number field is present in both sides or add explicit conversion/mapping at integration points to preserve attempt and stricter typing during serialization.examples/vitest/package.json (1)
12-14: Consider pinning the@currents/cmdversion.Using
"*"as a version specifier can lead to unexpected breaking changes when the package is updated. For an example project, consider pinning to a specific version or using"workspace:*"if this is a monorepo workspace.♻️ Suggested version specifier
"devDependencies": { - "@currents/cmd": "*", + "@currents/cmd": "workspace:*", "vitest": "^2.1.9" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/vitest/package.json` around lines 12 - 14, Replace the wildcard "*" for the "@currents/cmd" devDependency in package.json with a pinned version or workspace specifier to avoid unexpected upgrades; update the "devDependencies" entry for "@currents/cmd" (currently "*") to a specific semver like "1.2.3" or "workspace:*" depending on whether this repo is a workspace, then run install to lock the chosen version.examples/vitest/junit.xml (1)
14-67: Hardcoded absolute paths reduce portability.The JUnit XML fixture contains absolute paths to a specific developer's machine (e.g.,
/Users/miguelangarano/Documents/GitHub/currents-reporter/...). For example fixtures, consider using relative paths or documenting that these paths are illustrative and will be replaced during actual test runs.If this file is auto-generated by running the tests locally, consider adding it to
.gitignoreand providing instructions to regenerate it, or use placeholder paths that clearly indicate they should be replaced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/vitest/junit.xml` around lines 14 - 67, The JUnit XML fixture contains hardcoded absolute paths in system-out entries (e.g., lines with currents.artifact.{... "path": "..."} and markers like [[CURRENTS.ATTACHMENT|...]]), which reduces portability; update those entries to use relative paths or clear placeholders (e.g., ./examples/vitest/artifacts/... or /PATH/TO/REPO/artifacts/...) in the testcase blocks with names like "Vitest JUnit artifacts › generates spec, test, and attempt level artifacts via JSON logs" and others, and either add this fixture to .gitignore if it is auto-generated or add a short README/script note that documents how to regenerate the file during local test runs so contributors know it will be replaced.packages/cmd/src/services/upload/index.ts (2)
6-6: Unused import:generateShortHash.The
generateShortHashfunction is imported but not used in this file.♻️ Remove unused import
-import { maskRecordKey, nanoid, readJsonFile, writeFileAsync, generateShortHash } from '@lib'; +import { maskRecordKey, nanoid, readJsonFile, writeFileAsync } from '@lib';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cmd/src/services/upload/index.ts` at line 6, The import list in packages/cmd/src/services/upload/index.ts includes an unused symbol generateShortHash; remove generateShortHash from the named imports (alongside maskRecordKey, nanoid, readJsonFile, writeFileAsync) in the import statement to eliminate the unused import and restore linting/build hygiene.
323-327: Static analysis: forEach callback should not return a value.The
forEachcallback implicitly returns the result ofcontentTypeMap.set(). While this doesn't cause runtime issues, it's cleaner to make the return explicit or use afor...ofloop.♻️ Proposed fix
const processArtifacts = (artifacts?: any[]) => { if (artifacts) { - artifacts.forEach((a) => contentTypeMap.set(a.path, a.contentType)); + for (const a of artifacts) { + contentTypeMap.set(a.path, a.contentType); + } } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cmd/src/services/upload/index.ts` around lines 323 - 327, The forEach callback in processArtifacts implicitly returns the Map.set value; change the implementation to avoid returning from the callback—e.g., iterate with a for...of loop over artifacts (in function processArtifacts) and call contentTypeMap.set(a.path, a.contentType) inside the loop, or otherwise ensure the callback returns void; update the code around the processArtifacts declaration to use the new loop so the forEach callback no longer returns a value.packages/cmd/src/helpers/index.ts (2)
13-49: Duplicate type inference logic across packages.The extension-based type/contentType inference logic here is duplicated in
packages/jest/src/api.ts(lines 20-44). Consider extracting this to a shared utility to maintain consistency and reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cmd/src/helpers/index.ts` around lines 13 - 49, The extension-based type/contentType inference in attachArtifact is duplicated elsewhere; extract the logic into a shared utility (e.g., inferArtifactType or getArtifactTypeFromExt) and replace the inline switch in attachArtifact with a call to that utility, returning { type, contentType } based on the file extension; update other callers (the duplicate in api.ts) to use the same utility so inference is centralized and consistent across packages.
55-55:attachVideohardcodesvideo/mp4but doesn't verify the extension.If a user calls
attachVideo('/path/to/video.webm'), it will still be marked asvideo/mp4. Consider either:
- Inferring the content type from the extension (webm → video/webm)
- Documenting that this helper is specifically for MP4 files
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cmd/src/helpers/index.ts` at line 55, attachVideo currently hardcodes "video/mp4" which mislabels other formats; update the attachVideo implementation (in packages/cmd/src/helpers/index.ts) to derive the MIME type from the file extension before calling attachArtifact: parse the extension from the path, map common video extensions (.mp4 -> video/mp4, .webm -> video/webm, .ogg/.ogv -> video/ogg, .mov -> video/quicktime, etc.), use the mapped MIME when calling attachArtifact(path, 'video', inferredMime), and choose a clear fallback (either default to video/mp4 or throw/log and surface an error) if the extension is unrecognized; keep attachArtifact usage and behavior otherwise.examples/vitest/artifacts.test.ts (2)
194-209: File-based attempt counter may cause issues with parallel test execution.The attempt tracking mechanism writes to a shared file (
attempt-count.txt). If Vitest runs tests in parallel (which is the default), multiple tests or test files accessing this file could cause race conditions.For a demonstration/example test this is acceptable, but consider adding a comment noting this limitation or using
--no-threads/--single-threadin the example's run configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/vitest/artifacts.test.ts` around lines 194 - 209, The attempt counter uses a shared file (attempt-count.txt) via attemptFile/attempt variable which can race under parallel Vitest workers; either add a concise comment above the block (near artifactsDir/attemptFile/attachFile) stating this example is not safe for parallel execution and recommend running the example with single-threading (e.g., --no-threads/--single-thread) or change the example to use a per-process counter (e.g., process.pid-based filename) to avoid shared-file races.
93-101: Inconsistent attachment marker format.This test uses
[[ATTACHMENT|${videoPath}]](line 99), while the test at lines 67-74 uses[[CURRENTS.ATTACHMENT|${path}|level]]. According to the convert logic inpackages/cmd/src/services/convert/index.ts, the legacy format is[[ATTACHMENT|path]]and the new JSON format usescurrents.artifact.{...}.If both formats are intentionally supported, consider adding a comment to clarify. Otherwise, align on one approach for documentation clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/vitest/artifacts.test.ts` around lines 93 - 101, The test "generates a video artifact and logs attachment marker" currently emits the legacy marker `[[ATTACHMENT|${videoPath}]]` which is inconsistent with the other test that uses the `[[CURRENTS.ATTACHMENT|${path}|level]]` style and the newer currents.artifact JSON format expected by the convert logic in convert/index.ts; either update the console.log in this test to emit the new currents.artifact JSON structure (matching the other test's output and the convert service) or add a short inline comment above the test clarifying that both legacy `[[ATTACHMENT|path]]` markers and the new currents.artifact format are intentionally supported so future readers know this divergence, referencing the test name and the convert/index.ts behavior when making the change.packages/jest/src/api.ts (1)
82-86: Silent error handling may hide configuration issues.All errors in
attachArtifactare silently caught and ignored. While this prevents test crashes, it could hide issues like:
- Missing
.currents-artifactsdirectory permissions- Invalid artifact paths
- Disk space issues
Consider at minimum logging a warning in development/debug mode.
♻️ Add optional debug logging
} catch (e) { - // Ignore errors or let them propagate? - // User requested "no fallback", so we just silence the error or throw it? - // Silencing is safer for test execution not to crash because of reporting. + // Silencing is safer for test execution not to crash because of reporting. + // Log to stderr only if DEBUG is set to help with troubleshooting. + if (process.env.DEBUG) { + console.error('[currents] Failed to attach artifact:', e); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jest/src/api.ts` around lines 82 - 86, The catch block in attachArtifact currently swallows all errors; change it to log a warning with the caught error and context (e.g., path being attached and operation name) when running in dev/debug mode (check process.env.NODE_ENV !== 'production' or an existing debug flag), rather than silently ignoring; keep silent behavior in production but ensure the log includes error.message/stack so issues like permission, path, or disk errors are visible during development/troubleshooting.packages/jest/src/reporter.artifacts.test.ts (1)
9-173: Missing cleanup of temporary directory.The test creates a temporary directory but never cleans it up after the test completes. This can lead to disk space accumulation over repeated test runs, especially in CI environments.
♻️ Proposed fix to add cleanup
describe('CustomReporter artifacts', () => { + let baseDir: string; + + afterEach(async () => { + if (baseDir) { + await fs.remove(baseDir); + } + }); + it('creates artifacts from console logs via properties', async () => { - const baseDir = await fs.mkdtemp( + baseDir = await fs.mkdtemp( join(os.tmpdir(), 'currents-jest-artifacts-') );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jest/src/reporter.artifacts.test.ts` around lines 9 - 173, The test creates a temporary directory (baseDir from fs.mkdtemp) but never deletes it; add cleanup to remove baseDir after the test finishes—either wrap the test body in try/finally and call fs.remove(fsPath) / fs.rm(baseDir, { recursive: true, force: true }) in the finally, or register an afterEach that deletes baseDir if defined; update the test named "creates artifacts from console logs via properties" to ensure baseDir is removed to avoid leaking temp dirs.packages/cmd/src/types.ts (1)
40-47: Type inconsistency between packages.The
Artifactinterface here usestype: string, but inpackages/jest/src/types.ts(lines 43-51), the equivalentArtifactinterface usestype: ArtifactTypewhich is a union of specific string literals ('screenshot' | 'video' | 'trace' | 'attachment' | 'stdout' | 'coverage').This inconsistency may cause type-safety gaps when artifacts flow between Jest and cmd packages. Consider aligning these definitions or creating a shared type.
♻️ Proposed fix to use a stricter type
+export type ArtifactType = 'screenshot' | 'video' | 'trace' | 'attachment' | 'stdout' | 'coverage'; + export interface Artifact { path: string; - type: string; + type: ArtifactType | string; // Allow known types with fallback contentType: string; name?: string; metadata?: Record<string, string>; level?: 'spec' | 'test' | 'attempt'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cmd/src/types.ts` around lines 40 - 47, The Artifact interface's type property is currently a plain string (Artifact) which conflicts with the stricter ArtifactType union used elsewhere; change Artifact.type to use the shared union type (ArtifactType) instead of string by creating or using a single shared type definition and importing it where needed (or copy the union into this module if sharing isn't possible), update the Artifact interface to reference ArtifactType, and adjust any imports/exports and downstream usages (e.g., where Artifact is constructed or validated) so TypeScript compiles without widening the artifact type.packages/cmd/src/services/convert/utils.ts (1)
181-212: Remove the speculative narrative comment block from parser logic.This block reads like investigation notes and makes the parsing behavior harder to maintain. Keep a short intent-focused comment only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cmd/src/services/convert/utils.ts` around lines 181 - 212, Remove the long investigative/narrative comment block inside the branch that checks prop.name === 'currents.artifact.JSON_ARTIFACT' in packages/cmd/src/services/convert/utils.ts; replace it with a concise intent-focused comment (one or two lines) that states what the branch does (e.g., "Handle JSON artifact entries extracted from JUnit properties or logs") so the parser logic remains clear and maintainable—do not change parsing behavior in the prop.name branch or touch the related extractAttachmentsFromLog logic, only simplify the comments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/client-side-gap-analysis.md`:
- Line 10: The table cell containing the marker `[[ATTACHMENT|path]]` is
breaking the Markdown table; update the cell so the pipe is escaped inside the
inline code span (change `[[ATTACHMENT|path]]` to `[[ATTACHMENT\|path]]` within
the backticks) so the Markdown parser treats it as literal text and the table
columns stay aligned.
In `@examples/jest/package.json`:
- Line 8: The package.json "report" script currently hardcodes the API key
IRmZjZWBkkd3jqve; replace that literal with an environment variable (e.g.
CURRENTS_API_KEY) so the "report" script uses the env var instead of the
committed key, and update any README or comments to document setting
CURRENTS_API_KEY for local/dev use (or explicitly mark the key as a
non-production test key if it must remain visible).
In `@examples/vitest/package.json`:
- Line 9: The "report" npm script currently hardcodes an API key and should be
changed to use an environment variable instead: update the "report" script (the
"report" entry in package.json) to read the key from an env var (e.g.,
CURRENTS_API_KEY) rather than the literal "IRmZjZWBkkd3jqve", and document that
callers must set CURRENTS_API_KEY (and keep the existing CURRENTS_API_URL
handling) so credentials are not stored in the example.
In `@packages/cmd/src/api/create-run.ts`:
- Around line 91-110: In uploadStdout, change the initial warn() call to debug()
and await the makeRequest promise so rejections are caught by the try/catch;
specifically, in the uploadStdout function replace warn(...) with debug(...) and
call await makeRequest<void, { stdout: string }>(ClientType.API, { ... }) inside
the try block (keeping the existing debug('Error uploading stdout: %o', e) in
the catch).
In `@packages/cmd/src/services/convert/index.ts`:
- Line 36: The current extraction using
log.matchAll(/currents\.artifact\.(\{.*?\})/g) can break on nested JSON; update
the extraction in packages/cmd/src/services/convert/index.ts to iterate lines
(split log by '\n') and for each line use a regex that captures the JSON to the
end of the line (e.g., match /currents\.artifact\.(\{.+\})$/) and then
JSON.parse inside a try/catch; replace the existing use of
jsonMatches/log.matchAll with this line-based parsing and ensure parsing
failures are ignored or logged.
- Around line 19-55: The function extractAttachmentsFromLog is dead code; remove
its entire definition (the extractAttachmentsFromLog function block) from the
file, and also remove any now-unused imports it relies on (e.g., extname) to
avoid lint errors; alternatively, if you intend to keep it, replace the body
with a short TODO comment and export or document expected usage, but by default
delete the function and cleanup related unused symbols like extname.
In `@packages/cmd/src/services/convert/utils.ts`:
- Around line 66-88: extractArtifactsFromLog(stdout) returns artifacts that may
include a `level` indicating whether they belong to the test-level or
attempt-level; the current code blindly pushes the same artifacts into both
testArtifacts and attemptArtifacts.get(0), causing duplicate uploads and
ignoring `level`. Update the block that handles testCase['system-out'] so you
first map/filter stdoutArtifacts by their `level` (e.g., artifacts with level
=== 'attempt' -> push clones into attemptArtifacts.get(0), artifacts with level
=== 'test' or unspecified -> push clones into testArtifacts), clone each
artifact when inserting (to avoid in-place path mutation), and do not add the
same artifact to both collections; keep usage of functions/vars
extractArtifactsFromLog, testArtifacts, attemptArtifacts, and ensureArray to
locate and modify the logic.
- Around line 121-129: The current regex in utils.ts (jsonMatches =
log.matchAll(/currents\.artifact\.(\{.*?\})/g)) fails for nested JSON and
silently drops artifacts; replace the fragile regex approach with a robust
extractor: search for the "currents.artifact.{" literal, then walk the string
from the opening brace to the matching closing brace using a brace counter to
capture the full balanced JSON substring, then JSON.parse it and push to
artifacts (same check for path/type/contentType). Update the loop that currently
iterates jsonMatches to use this brace-matching extraction before parsing so
nested objects (e.g. metadata) are preserved and parse errors are not silent.
In `@packages/cmd/src/services/upload/index.ts`:
- Around line 157-159: The log is inverted: the info(...) message is emitted
when response.artifactUploadUrls exists and has items. Update the condition to
log only when there are no upload URLs (e.g., if (!response.artifactUploadUrls
|| response.artifactUploadUrls.length === 0)) and reword the message for clarity
(suggestion: "No artifacts to handle: initial run created without instances").
Ensure you update the code that references response.artifactUploadUrls and the
info(...) call accordingly.
In `@packages/jest/src/api.ts`:
- Around line 41-44: The fallback contentType in this file currently sets
contentType = 'text/plain' while the helpers package uses
'application/octet-stream'; update the default in the else branch where
variables type and contentType are assigned (the contentType and type assignment
block) to use 'application/octet-stream' so the artifact processing defaults are
consistent with the helpers package; ensure no other logic depends on plain text
as the implicit default.
- Around line 119-130: Update getAttempt to treat the jest-internal Symbol
lookup as an optional optimization and make the fallback heuristic (using
expect.getState()/expect.setState()) the primary code path: add a
top-of-function comment stating this behavior and that the implementation is
tested against Jest 29.5.0+ and may break on future Jest versions; move the
existing Symbol(JEST_STATE_SYMBOL) branch to run only if the fallback fails or
as a short-circuit optimization, and add a brief inline comment above the Symbol
branch explaining it is used for performance/reliability when available but
relies on internal APIs; keep the existing try/catch and defensive checks around
global[stateSymbol] and the expect-based fallback intact and ensure the function
still returns the same fallback value when symbol access is unavailable.
In `@packages/jest/src/artifacts.ts`:
- Around line 391-434: The regex in parseAttachmentLogs currently only matches
the marker with the CURRENTS. prefix; update the pattern used in
parseAttachmentLogs to accept both forms ([[ATTACHMENT|...]] and
[[CURRENTS.ATTACHMENT|...]]). Modify the matchAll regex (in parseAttachmentLogs)
to make the "CURRENTS\." prefix optional (e.g. allow an optional group before
ATTACHMENT) while preserving the existing capture groups for path and optional
level, then run/adjust tests in packages/cmd and the convert-artifacts tests to
ensure backward compatibility.
In `@packages/jest/src/reporter.ts`:
- Around line 363-383: The code currently only sets `_testArtifacts` when `index
=== 0` and later promotes artifacts from `t.attempts[0]`, which drops artifacts
emitted on retries; remove the `index === 0` gating so each attempt can set
`_testArtifacts`, and change the promotion logic in the `tests.forEach` block to
locate the first (or last) attempt that has a `_testArtifacts` property (e.g.,
find(attempt => attempt._testArtifacts) or scan from last to first) and assign
that attempt's `_testArtifacts` to `t.artifacts`, then delete the
`_testArtifacts` from that attempt; update references to `_testArtifacts`,
`tests.forEach`, and `t.attempts` accordingly.
---
Outside diff comments:
In `@packages/jest/src/reporter.ts`:
- Around line 250-273: The forEach callback passed to
testResult.testResults.forEach is marked async but contains no awaits, causing
it to return unused Promises; remove the unnecessary async modifier from the
callback so it is a plain synchronous function that still calls getTestCaseId,
getTestCaseKey and mutates this.specInfo[specKey].testCaseList and
this.resultsDeferred[testCaseKey] as before (preserve all existing logic inside
the callback).
---
Nitpick comments:
In `@docs/artifact-workflow.md`:
- Around line 353-364: The fenced code block showing the directory tree starting
with ".currents/" is missing a language specifier; update that fenced block to
include a language identifier (e.g., change ``` to ```text) so the directory
structure (the block beginning with ".currents/") renders consistently.
In `@examples/jest/src/artifacts/artifacts.test.ts`:
- Line 59: Move the jest.retryTimes(2) call from the end of artifacts.test.ts
into the top-level setup area of the file (above the test suite definitions) so
Jest configuration is visible immediately; remove the trailing
jest.retryTimes(2) call at the bottom to avoid duplicate/confusing placement and
ensure the retry setting applies globally to the tests in this file.
- Around line 34-36: The local declaration of artifactsDir inside the test
"generates artifacts for multiple attempts" shadows the module-level
artifactsDir; remove the const artifactsDir = ... so the test reuses the
top-level artifactsDir variable and update any dependent expressions (e.g.,
attemptFile) to reference the existing artifactsDir; locate this in the test
function in artifacts.test.ts and delete the redundant variable declaration to
avoid shadowing.
- Around line 36-43: The test currently persists a state file via attemptFile
and increments attempt across runs; add teardown to remove or reset that file
after the suite so repeated runs start clean. Update the test file
(artifacts.test.ts) to add an afterAll (or afterEach if appropriate) hook that
checks for attemptFile (the same path/variable used when creating
jest-attempt-count.txt) and deletes it (or writes "0") using fs.unlinkSync or
fs.writeFileSync, ensuring the attempt counter logic (attempt variable and its
read/write) runs against a fresh state on subsequent runs.
In `@examples/jest/src/artifacts/media.test.ts`:
- Around line 1-4: Replace the CommonJS requires with ES module imports to keep
TypeScript consistency: change the top-of-file requires for fs and path and the
import of attachVideo and attachScreenshot from '@currents/jest' to ES module
import statements, updating any uses to match (refer to the variables fs, path
and the functions attachVideo and attachScreenshot to locate where to change).
In `@examples/vitest/artifacts.test.ts`:
- Around line 194-209: The attempt counter uses a shared file
(attempt-count.txt) via attemptFile/attempt variable which can race under
parallel Vitest workers; either add a concise comment above the block (near
artifactsDir/attemptFile/attachFile) stating this example is not safe for
parallel execution and recommend running the example with single-threading
(e.g., --no-threads/--single-thread) or change the example to use a per-process
counter (e.g., process.pid-based filename) to avoid shared-file races.
- Around line 93-101: The test "generates a video artifact and logs attachment
marker" currently emits the legacy marker `[[ATTACHMENT|${videoPath}]]` which is
inconsistent with the other test that uses the
`[[CURRENTS.ATTACHMENT|${path}|level]]` style and the newer currents.artifact
JSON format expected by the convert logic in convert/index.ts; either update the
console.log in this test to emit the new currents.artifact JSON structure
(matching the other test's output and the convert service) or add a short inline
comment above the test clarifying that both legacy `[[ATTACHMENT|path]]` markers
and the new currents.artifact format are intentionally supported so future
readers know this divergence, referencing the test name and the convert/index.ts
behavior when making the change.
In `@examples/vitest/junit.xml`:
- Around line 14-67: The JUnit XML fixture contains hardcoded absolute paths in
system-out entries (e.g., lines with currents.artifact.{... "path": "..."} and
markers like [[CURRENTS.ATTACHMENT|...]]), which reduces portability; update
those entries to use relative paths or clear placeholders (e.g.,
./examples/vitest/artifacts/... or /PATH/TO/REPO/artifacts/...) in the testcase
blocks with names like "Vitest JUnit artifacts › generates spec, test, and
attempt level artifacts via JSON logs" and others, and either add this fixture
to .gitignore if it is auto-generated or add a short README/script note that
documents how to regenerate the file during local test runs so contributors know
it will be replaced.
In `@examples/vitest/package.json`:
- Around line 12-14: Replace the wildcard "*" for the "@currents/cmd"
devDependency in package.json with a pinned version or workspace specifier to
avoid unexpected upgrades; update the "devDependencies" entry for
"@currents/cmd" (currently "*") to a specific semver like "1.2.3" or
"workspace:*" depending on whether this repo is a workspace, then run install to
lock the chosen version.
In `@packages/cmd/src/helpers/index.ts`:
- Around line 13-49: The extension-based type/contentType inference in
attachArtifact is duplicated elsewhere; extract the logic into a shared utility
(e.g., inferArtifactType or getArtifactTypeFromExt) and replace the inline
switch in attachArtifact with a call to that utility, returning { type,
contentType } based on the file extension; update other callers (the duplicate
in api.ts) to use the same utility so inference is centralized and consistent
across packages.
- Line 55: attachVideo currently hardcodes "video/mp4" which mislabels other
formats; update the attachVideo implementation (in
packages/cmd/src/helpers/index.ts) to derive the MIME type from the file
extension before calling attachArtifact: parse the extension from the path, map
common video extensions (.mp4 -> video/mp4, .webm -> video/webm, .ogg/.ogv ->
video/ogg, .mov -> video/quicktime, etc.), use the mapped MIME when calling
attachArtifact(path, 'video', inferredMime), and choose a clear fallback (either
default to video/mp4 or throw/log and surface an error) if the extension is
unrecognized; keep attachArtifact usage and behavior otherwise.
In `@packages/cmd/src/services/convert/utils.ts`:
- Around line 181-212: Remove the long investigative/narrative comment block
inside the branch that checks prop.name === 'currents.artifact.JSON_ARTIFACT' in
packages/cmd/src/services/convert/utils.ts; replace it with a concise
intent-focused comment (one or two lines) that states what the branch does
(e.g., "Handle JSON artifact entries extracted from JUnit properties or logs")
so the parser logic remains clear and maintainable—do not change parsing
behavior in the prop.name branch or touch the related extractAttachmentsFromLog
logic, only simplify the comments.
In `@packages/cmd/src/services/upload/index.ts`:
- Line 6: The import list in packages/cmd/src/services/upload/index.ts includes
an unused symbol generateShortHash; remove generateShortHash from the named
imports (alongside maskRecordKey, nanoid, readJsonFile, writeFileAsync) in the
import statement to eliminate the unused import and restore linting/build
hygiene.
- Around line 323-327: The forEach callback in processArtifacts implicitly
returns the Map.set value; change the implementation to avoid returning from the
callback—e.g., iterate with a for...of loop over artifacts (in function
processArtifacts) and call contentTypeMap.set(a.path, a.contentType) inside the
loop, or otherwise ensure the callback returns void; update the code around the
processArtifacts declaration to use the new loop so the forEach callback no
longer returns a value.
In `@packages/cmd/src/types.ts`:
- Around line 40-47: The Artifact interface's type property is currently a plain
string (Artifact) which conflicts with the stricter ArtifactType union used
elsewhere; change Artifact.type to use the shared union type (ArtifactType)
instead of string by creating or using a single shared type definition and
importing it where needed (or copy the union into this module if sharing isn't
possible), update the Artifact interface to reference ArtifactType, and adjust
any imports/exports and downstream usages (e.g., where Artifact is constructed
or validated) so TypeScript compiles without widening the artifact type.
In `@packages/jest/src/api.ts`:
- Around line 82-86: The catch block in attachArtifact currently swallows all
errors; change it to log a warning with the caught error and context (e.g., path
being attached and operation name) when running in dev/debug mode (check
process.env.NODE_ENV !== 'production' or an existing debug flag), rather than
silently ignoring; keep silent behavior in production but ensure the log
includes error.message/stack so issues like permission, path, or disk errors are
visible during development/troubleshooting.
In `@packages/jest/src/artifacts.ts`:
- Around line 283-301: The empty catch swallowing JSON.parse errors for
'JSON_ARTIFACT' makes debugging impossible; update the catch in the loop that
handles log.key === 'JSON_ARTIFACT' to log the parse failure (include the thrown
error and the offending log.value and/or log) instead of ignoring it — use the
same logger used elsewhere in this module (e.g., processLogger.error or the
module's debug logger) so you get a descriptive message whenever
JSON.parse(log.value) fails while populating jsonArtifacts.
- Around line 186-194: The catch block swallowing JSON.parse errors for entries
where log.key === 'JSON_ARTIFACT' should be replaced with debug logging: inside
the catch for JSON.parse(log.value) log the parse error and relevant context
(e.g., the offending log.value or a truncated version and the log key) instead
of doing nothing so malformed artifact entries are visible during debugging;
update the catch in the loop that iterates over logs (references: logs, log.key,
'JSON_ARTIFACT', JSON.parse, jsonArtifacts) to emit a debug message via the
project's logger or console.debug including the error and identifying
information.
In `@packages/jest/src/lib/fs.ts`:
- Around line 60-68: copyFileAsync currently copies a file without ensuring the
destination directory exists, unlike the counterpart in packages/cmd; update
copyFileAsync to compute the destination directory (using
path.dirname(destination)) and call the fs.ensureDir (or equivalent) on that
directory before calling fs.copy, preserving the existing try/catch and
debug/error logging in the same function so the operation is robust if callers
don't pre-create the artifacts directory.
In `@packages/jest/src/reporter.artifacts.test.ts`:
- Around line 9-173: The test creates a temporary directory (baseDir from
fs.mkdtemp) but never deletes it; add cleanup to remove baseDir after the test
finishes—either wrap the test body in try/finally and call fs.remove(fsPath) /
fs.rm(baseDir, { recursive: true, force: true }) in the finally, or register an
afterEach that deletes baseDir if defined; update the test named "creates
artifacts from console logs via properties" to ensure baseDir is removed to
avoid leaking temp dirs.
In `@packages/jest/src/types.ts`:
- Around line 44-52: The Artifact interface in packages/jest (Artifact,
ArtifactType, ArtifactLevel, attempt?) is stricter than the cmd package's
artifact shape (which uses type: string, inline level union, and lacks attempt),
causing potential serialization/deserialization data loss; align the definitions
by updating the cmd package to use the same exported types/names (Artifact,
ArtifactType, ArtifactLevel) or export a shared artifact type and replace the
cmd inline union and string-typed type with the shared types, and ensure the
optional attempt?: number field is present in both sides or add explicit
conversion/mapping at integration points to preserve attempt and stricter typing
during serialization.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/cmd/src/services/convert/__tests__/__snapshots__/instances.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (35)
.gitignoredocs/artifact-workflow.mddocs/client-side-gap-analysis.mdexamples/jest/jest.config.jsexamples/jest/package.jsonexamples/jest/src/artifacts/artifacts.test.tsexamples/jest/src/artifacts/media.test.tsexamples/vitest/artifacts.test.tsexamples/vitest/junit.xmlexamples/vitest/package.jsonexamples/vitest/properties-junit.xmlexamples/vitest/vitest.config.tspackages/cmd/package.jsonpackages/cmd/src/api/create-run.tspackages/cmd/src/helpers/index.tspackages/cmd/src/lib/fs.tspackages/cmd/src/services/convert/__tests__/convert-artifacts.test.tspackages/cmd/src/services/convert/__tests__/fixtures/index.tspackages/cmd/src/services/convert/index.tspackages/cmd/src/services/convert/postman/instances.tspackages/cmd/src/services/convert/utils.tspackages/cmd/src/services/upload/index.tspackages/cmd/src/types.tspackages/cmd/tsup.config.tspackages/jest/README.mdpackages/jest/jest.config.cjspackages/jest/package.jsonpackages/jest/src/api.tspackages/jest/src/artifacts.tspackages/jest/src/index.tspackages/jest/src/lib/fs.tspackages/jest/src/lib/index.tspackages/jest/src/reporter.artifacts.test.tspackages/jest/src/reporter.tspackages/jest/src/types.ts
| | Feature | Plan (Docs) | Implementation (Current State) | Status | | ||
| | :--- | :--- | :--- | :--- | | ||
| | **Artifact Levels** | Support **Spec**, **Test**, and **Attempt** levels. | Only **Attempt-level** is fully implemented. | ⚠️ Partial | | ||
| | **Discovery Mechanism** | Parse custom **JUnit XML properties** (e.g., `currents.artifact.path`). | Parses **Console Logs** for markers (`[[ATTACHMENT|path]]`). | ⚠️ Changed | |
There was a problem hiding this comment.
Escape the pipe character in the table cell.
The | character inside `[[ATTACHMENT|path]]` is being interpreted as a column delimiter, causing the table to have mismatched columns. Escape it with a backslash.
📝 Proposed fix
-| **Discovery Mechanism** | Parse custom **JUnit XML properties** (e.g., `currents.artifact.path`). | Parses **Console Logs** for markers (`[[ATTACHMENT|path]]`). | ⚠️ Changed |
+| **Discovery Mechanism** | Parse custom **JUnit XML properties** (e.g., `currents.artifact.path`). | Parses **Console Logs** for markers (`[[ATTACHMENT\|path]]`). | ⚠️ Changed |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | **Discovery Mechanism** | Parse custom **JUnit XML properties** (e.g., `currents.artifact.path`). | Parses **Console Logs** for markers (`[[ATTACHMENT|path]]`). | ⚠️ Changed | | |
| | **Discovery Mechanism** | Parse custom **JUnit XML properties** (e.g., `currents.artifact.path`). | Parses **Console Logs** for markers (`[[ATTACHMENT\|path]]`). | ⚠️ Changed | |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 10-10: Table column count
Expected: 4; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/client-side-gap-analysis.md` at line 10, The table cell containing the
marker `[[ATTACHMENT|path]]` is breaking the Markdown table; update the cell so
the pipe is escaped inside the inline code span (change `[[ATTACHMENT|path]]` to
`[[ATTACHMENT\|path]]` within the backticks) so the Markdown parser treats it as
literal text and the table columns stay aligned.
| "scripts": { | ||
| "test": "jest", | ||
| "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=xW2Ijf --key=9bqJY1huXL2l3ONF", | ||
| "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=PrZ3j8 --key=IRmZjZWBkkd3jqve", |
There was a problem hiding this comment.
Hardcoded API key should be replaced with environment variable.
The API key IRmZjZWBkkd3jqve is committed to the repository. Even for local development or test environments, hardcoding credentials is a security risk and bad practice. Use an environment variable instead.
🔒 Proposed fix
- "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=PrZ3j8 --key=IRmZjZWBkkd3jqve",
+ "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=${CURRENTS_PROJECT_ID:-PrZ3j8} --key=${CURRENTS_KEY}",Alternatively, if this is intentionally a public/test key for demonstration purposes, add a comment in the repository documenting this, and consider using a clearly named test key that cannot be mistaken for a production credential.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=PrZ3j8 --key=IRmZjZWBkkd3jqve", | |
| "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=${CURRENTS_PROJECT_ID:-PrZ3j8} --key=${CURRENTS_KEY}", |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/jest/package.json` at line 8, The package.json "report" script
currently hardcodes the API key IRmZjZWBkkd3jqve; replace that literal with an
environment variable (e.g. CURRENTS_API_KEY) so the "report" script uses the env
var instead of the committed key, and update any README or comments to document
setting CURRENTS_API_KEY for local/dev use (or explicitly mark the key as a
non-production test key if it must remain visible).
| "scripts": { | ||
| "test": "vitest --reporter=default --reporter=junit --outputFile=./junit.xml || true", | ||
| "convert": "npx currents convert --input-format=junit --input-file=./junit.xml --framework=vitest", | ||
| "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=PrZ3j8 --key=IRmZjZWBkkd3jqve", |
There was a problem hiding this comment.
Hardcoded API key detected in example script.
Even in example code, hardcoding API keys is a security risk as it can lead to accidental credential exposure. Consider using an environment variable placeholder instead.
🔒 Proposed fix to use environment variable
- "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=PrZ3j8 --key=IRmZjZWBkkd3jqve",
+ "report": "CURRENTS_API_URL=http://localhost:1234 currents --project-id=${CURRENTS_PROJECT_ID:-PrZ3j8} --key=${CURRENTS_API_KEY}",🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/vitest/package.json` at line 9, The "report" npm script currently
hardcodes an API key and should be changed to use an environment variable
instead: update the "report" script (the "report" entry in package.json) to read
the key from an env var (e.g., CURRENTS_API_KEY) rather than the literal
"IRmZjZWBkkd3jqve", and document that callers must set CURRENTS_API_KEY (and
keep the existing CURRENTS_API_URL handling) so credentials are not stored in
the example.
Comment on lines
+91
to
+110
| export async function uploadStdout( | ||
| instanceId: string, | ||
| stdout: string, | ||
| config: RunCreationConfig | ||
| ) { | ||
| warn(`Uploading stdout for instance ${instanceId} with ${stdout.length} bytes`); | ||
| try { | ||
| return makeRequest<void, { stdout: string }>(ClientType.API, { | ||
| url: `instances/${instanceId}/stdout`, | ||
| method: 'PUT', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| data: { stdout }, | ||
| }); | ||
| } catch (e) { | ||
| debug('Error uploading stdout: %o', e); | ||
| throw e; | ||
| } | ||
| } |
There was a problem hiding this comment.
The try-catch is ineffective and log level is inappropriate.
Two issues:
-
Missing
await: ThemakeRequestcall isn't awaited, so the Promise is returned immediately and any rejection won't be caught by the try-catch block. -
Inappropriate log level: Using
warn()for routine upload operations pollutes warning logs. This should usedebug()for consistency with other API functions in this file.
🐛 Proposed fix
export async function uploadStdout(
instanceId: string,
stdout: string,
config: RunCreationConfig
) {
- warn(`Uploading stdout for instance ${instanceId} with ${stdout.length} bytes`);
+ debug('Uploading stdout for instance %s with %d bytes', instanceId, stdout.length);
try {
- return makeRequest<void, { stdout: string }>(ClientType.API, {
+ return await makeRequest<void, { stdout: string }>(ClientType.API, {
url: `instances/${instanceId}/stdout`,
method: 'PUT',
headers: {
'Content-Type': 'application/json',
},
data: { stdout },
});
} catch (e) {
debug('Error uploading stdout: %o', e);
throw e;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cmd/src/api/create-run.ts` around lines 91 - 110, In uploadStdout,
change the initial warn() call to debug() and await the makeRequest promise so
rejections are caught by the try/catch; specifically, in the uploadStdout
function replace warn(...) with debug(...) and call await makeRequest<void, {
stdout: string }>(ClientType.API, { ... }) inside the try block (keeping the
existing debug('Error uploading stdout: %o', e) in the catch).
Comment on lines
+19
to
+55
| function extractAttachmentsFromLog( | ||
| log: string | ||
| ): { sourcePath: string; ext: string; type?: string; contentType?: string }[] { | ||
| const out: { sourcePath: string; ext: string; type?: string; contentType?: string }[] = []; | ||
|
|
||
| // Legacy format: [[ATTACHMENT|path]] | ||
| const matches = log.matchAll(/\[\[ATTACHMENT\|([^\]]+)\]\]/g); | ||
| for (const match of matches) { | ||
| const sourcePath = match[1]; | ||
| const ext = extname(sourcePath).slice(1).toLowerCase(); | ||
| out.push({ sourcePath, ext }); | ||
| } | ||
|
|
||
| // New JSON format: currents.artifact.{"path":...} | ||
| // The log might contain multiple artifacts or lines. | ||
| // We need to scan for "currents.artifact.{...}" | ||
| // Since logs can be messy, let's use a regex that captures the JSON part. | ||
| const jsonMatches = log.matchAll(/currents\.artifact\.(\{.*?\})/g); | ||
| for (const match of jsonMatches) { | ||
| try { | ||
| const artifact = JSON.parse(match[1]); | ||
| if (artifact.path) { | ||
| const ext = extname(artifact.path).slice(1).toLowerCase(); | ||
| out.push({ | ||
| sourcePath: artifact.path, | ||
| ext, | ||
| type: artifact.type, | ||
| contentType: artifact.contentType | ||
| }); | ||
| } | ||
| } catch (e) { | ||
| // Ignore invalid JSON | ||
| } | ||
| } | ||
|
|
||
| return out; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'extractAttachmentsFromLog' --type tsRepository: currents-dev/currents-reporter
Length of output: 520
🏁 Script executed:
#!/bin/bash
# Search for function calls (with parentheses)
echo "=== Function calls (invocations) ==="
rg 'extractAttachmentsFromLog\s*\(' --type ts
echo ""
echo "=== Export statements ==="
rg 'export.*extractAttachmentsFromLog' --type ts
echo ""
echo "=== Import statements ==="
rg 'import.*extractAttachmentsFromLog' --type tsRepository: currents-dev/currents-reporter
Length of output: 243
Function extractAttachmentsFromLog is unused and should be removed.
This function is defined but never called anywhere in the codebase. It is not exported or imported elsewhere. Remove it to keep the codebase clean, or if it's intended for future use, add a TODO comment and document the expected usage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cmd/src/services/convert/index.ts` around lines 19 - 55, The
function extractAttachmentsFromLog is dead code; remove its entire definition
(the extractAttachmentsFromLog function block) from the file, and also remove
any now-unused imports it relies on (e.g., extname) to avoid lint errors;
alternatively, if you intend to keep it, replace the body with a short TODO
comment and export or document expected usage, but by default delete the
function and cleanup related unused symbols like extname.
Comment on lines
+157
to
+159
| if (response.artifactUploadUrls && response.artifactUploadUrls.length > 0) { | ||
| info('No artifacts handling: initial run created without instances'); | ||
| } |
There was a problem hiding this comment.
Log message logic appears inverted.
The message "No artifacts handling: initial run created without instances" is logged when artifactUploadUrls exists and has items. This seems backwards - the presence of upload URLs suggests artifacts are being handled.
If the intent is to note that the initial run (with empty instances) won't process artifacts yet, consider rewording for clarity.
📝 Suggested rewording
if (response.artifactUploadUrls && response.artifactUploadUrls.length > 0) {
- info('No artifacts handling: initial run created without instances');
+ debug('Initial run created; artifact upload URLs received, will process with instance chunks');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (response.artifactUploadUrls && response.artifactUploadUrls.length > 0) { | |
| info('No artifacts handling: initial run created without instances'); | |
| } | |
| if (response.artifactUploadUrls && response.artifactUploadUrls.length > 0) { | |
| debug('Initial run created; artifact upload URLs received, will process with instance chunks'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cmd/src/services/upload/index.ts` around lines 157 - 159, The log is
inverted: the info(...) message is emitted when response.artifactUploadUrls
exists and has items. Update the condition to log only when there are no upload
URLs (e.g., if (!response.artifactUploadUrls ||
response.artifactUploadUrls.length === 0)) and reword the message for clarity
(suggestion: "No artifacts to handle: initial run created without instances").
Ensure you update the code that references response.artifactUploadUrls and the
info(...) call accordingly.
Comment on lines
+41
to
+44
| }else { | ||
| type = type || 'attachment'; | ||
| contentType = contentType || 'text/plain'; | ||
| } |
There was a problem hiding this comment.
Default contentType differs from helpers package.
The default fallback contentType here is 'text/plain', but in packages/cmd/src/helpers/index.ts (line 37), the fallback is 'application/octet-stream'. This inconsistency could cause unexpected behavior when artifacts are processed.
♻️ Align with helpers package
}else {
type = type || 'attachment';
- contentType = contentType || 'text/plain';
+ contentType = contentType || 'application/octet-stream';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| }else { | |
| type = type || 'attachment'; | |
| contentType = contentType || 'text/plain'; | |
| } | |
| }else { | |
| type = type || 'attachment'; | |
| contentType = contentType || 'application/octet-stream'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/jest/src/api.ts` around lines 41 - 44, The fallback contentType in
this file currently sets contentType = 'text/plain' while the helpers package
uses 'application/octet-stream'; update the default in the else branch where
variables type and contentType are assigned (the contentType and type assignment
block) to use 'application/octet-stream' so the artifact processing defaults are
consistent with the helpers package; ensure no other logic depends on plain text
as the implicit default.
Comment on lines
+119
to
+130
| export function getAttempt(): number { | ||
| try { | ||
| const symbols = Object.getOwnPropertySymbols(global); | ||
| const stateSymbol = symbols.find(s => s.toString() === 'Symbol(JEST_STATE_SYMBOL)'); | ||
|
|
||
| if (stateSymbol) { | ||
| // @ts-ignore | ||
| const jestState = global[stateSymbol]; | ||
| if (jestState && jestState.currentlyRunningTest && typeof jestState.currentlyRunningTest.invocations === 'number') { | ||
| return jestState.currentlyRunningTest.invocations - 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Jest 29 JEST_STATE_SYMBOL public API documentation
💡 Result:
JEST_STATE_SYMBOL is not a documented public Jest API. In Jest 29.7.0, it’s just the description string used when jest-circus creates an internal symbol:
packages/jest-circus/src/types.ts:export const STATE_SYM = Symbol('JEST_STATE_SYMBOL');(this symbol is used as a key onglobal[STATE_SYM]). (raw.githubusercontent.com)packages/jest-circus/src/state.ts: the circus runner state is stored/read viaglobal[STATE_SYM]and exposed viaresetState(),getState(),setState(). (raw.githubusercontent.com)
If you’re looking for the “public” way to access that state, use the exported functions (instead of trying to access the symbol):
jest-circusexportsgetState,setState,resetStatefrom its public entrypoint. (raw.githubusercontent.com)
Note: jest-circus explicitly warns that mutating event/state is unsupported and may break without warning. (raw.githubusercontent.com)
Citations:
- 1: https://raw.githubusercontent.com/jestjs/jest/v29.7.0/packages/jest-circus/src/types.ts
- 2: https://raw.githubusercontent.com/jestjs/jest/v29.7.0/packages/jest-circus/src/state.ts
- 3: https://raw.githubusercontent.com/jestjs/jest/v29.7.0/packages/jest-circus/src/index.ts
- 4: https://raw.githubusercontent.com/jestjs/jest/v29.7.0/packages/jest-circus/README.md
🏁 Script executed:
# Find and read the complete function to see the fallback heuristic
fd -t f "api.ts" packages/jest/src/Repository: currents-dev/currents-reporter
Length of output: 98
🏁 Script executed:
# Check the Jest version requirement in package.json
fd -t f "package.json" packages/jest/Repository: currents-dev/currents-reporter
Length of output: 100
🏁 Script executed:
# Read the complete getAttempt function
cat -n packages/jest/src/api.ts | sed -n '119,162p'Repository: currents-dev/currents-reporter
Length of output: 1742
🏁 Script executed:
# Check Jest version constraint
cat packages/jest/package.jsonRepository: currents-dev/currents-reporter
Length of output: 2338
Jest's JEST_STATE_SYMBOL is not a public API; document version dependency and consider the fallback approach as primary.
The symbol lookup (lines 119-130) accesses Jest's internal state via an undocumented implementation detail. Web verification confirms JEST_STATE_SYMBOL is intentionally internal to jest-circus (Jest 29.7.0); jest-circus explicitly warns that state mutation is unsupported and may break without notice.
The current code has good defensive layers: try-catch, symbol presence check, and a fallback heuristic (lines 132-162). However, the fallback also relies on internal APIs (expect.getState()/expect.setState()).
Consider:
- Add a comment noting this code is tested against Jest 29.5.0+ and may require updates for future Jest versions
- Reorder the logic to rely on the fallback heuristic as the primary approach, with the symbol lookup as an optimization only
- Document why the Symbol approach exists (performance, reliability) if that's the intent
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/jest/src/api.ts` around lines 119 - 130, Update getAttempt to treat
the jest-internal Symbol lookup as an optional optimization and make the
fallback heuristic (using expect.getState()/expect.setState()) the primary code
path: add a top-of-function comment stating this behavior and that the
implementation is tested against Jest 29.5.0+ and may break on future Jest
versions; move the existing Symbol(JEST_STATE_SYMBOL) branch to run only if the
fallback fails or as a short-circuit optimization, and add a brief inline
comment above the Symbol branch explaining it is used for
performance/reliability when available but relies on internal APIs; keep the
existing try/catch and defensive checks around global[stateSymbol] and the
expect-based fallback intact and ensure the function still returns the same
fallback value when symbol access is unavailable.
Comment on lines
+391
to
+434
| function parseAttachmentLogs( | ||
| consoleEntries: TestResult['console'], | ||
| testFilePath: string | ||
| ): PropertyLog[] { | ||
| return (consoleEntries ?? []) | ||
| .map((log) => { | ||
| // Look for [[ATTACHMENT|path]] or [[ATTACHMENT|path|level]] pattern | ||
|
|
||
| // We use a regex that optionally captures a second parameter (level) | ||
| // Format: [[CURRENTS.ATTACHMENT|path]] or [[CURRENTS.ATTACHMENT|path|level]] | ||
| // The path cannot contain ']', so we use [^\]|]+ for path segment if followed by another |? | ||
| // Simpler: [[CURRENTS.ATTACHMENT| part1 (| part2)? ]] | ||
|
|
||
| const matches = [...log.message.matchAll(/\[\[CURRENTS\.ATTACHMENT\|([^|\]]+)(?:\|([^\]]+))?\]\]/g)]; | ||
| if (matches.length === 0) return []; | ||
|
|
||
| const line = getLineFromOrigin(log.origin, testFilePath); | ||
|
|
||
| return matches.map(match => { | ||
| const filePath = match[1].trim(); | ||
| const levelRaw = match[2]?.trim(); | ||
|
|
||
| // Validate level if provided, default to 'attempt' | ||
| let level = 'attempt'; | ||
| if (levelRaw && ['spec', 'test', 'attempt'].includes(levelRaw)) { | ||
| level = levelRaw; | ||
| } | ||
|
|
||
| const artifact = { | ||
| path: filePath, | ||
| type: inferArtifactType(filePath), | ||
| contentType: 'application/octet-stream', | ||
| level: level, | ||
| }; | ||
|
|
||
| return { | ||
| key: 'JSON_ARTIFACT', | ||
| value: JSON.stringify(artifact), | ||
| line | ||
| }; | ||
| }); | ||
| }) | ||
| .flat(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the documentation file
fd artifact-workflow.mdRepository: currents-dev/currents-reporter
Length of output: 99
🏁 Script executed:
# Search for ATTACHMENT patterns in the codebase to understand usage
rg "ATTACHMENT" --type=ts --type=md -B2 -A2Repository: currents-dev/currents-reporter
Length of output: 11239
🏁 Script executed:
# Look specifically for examples and test files mentioning ATTACHMENT
fd -e test.ts -e spec.ts -e example.ts | xargs rg -l "ATTACHMENT" 2>/dev/null || echo "No test files found with ATTACHMENT"Repository: currents-dev/currents-reporter
Length of output: 177
Regex does not match documented marker format.
The regex on line 404 of packages/jest/src/artifacts.ts expects [[CURRENTS.ATTACHMENT|...]], but documentation (docs/artifact-workflow.md lines 75, 87, 150) and existing code in packages/cmd/src/services/convert/__tests__/convert-artifacts.test.ts use [[ATTACHMENT|...]] without the CURRENTS. prefix. The vitest examples also use both formats, indicating this inconsistency affects multiple code paths.
Update the regex to accept both formats for backward compatibility:
Proposed fix
- const matches = [...log.message.matchAll(/\[\[CURRENTS\.ATTACHMENT\|([^|\]]+)(?:\|([^\]]+))?\]\]/g)];
+ const matches = [...log.message.matchAll(/\[\[(?:CURRENTS\.)?ATTACHMENT\|([^|\]]+)(?:\|([^\]]+))?\]\]/g)];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function parseAttachmentLogs( | |
| consoleEntries: TestResult['console'], | |
| testFilePath: string | |
| ): PropertyLog[] { | |
| return (consoleEntries ?? []) | |
| .map((log) => { | |
| // Look for [[ATTACHMENT|path]] or [[ATTACHMENT|path|level]] pattern | |
| // We use a regex that optionally captures a second parameter (level) | |
| // Format: [[CURRENTS.ATTACHMENT|path]] or [[CURRENTS.ATTACHMENT|path|level]] | |
| // The path cannot contain ']', so we use [^\]|]+ for path segment if followed by another |? | |
| // Simpler: [[CURRENTS.ATTACHMENT| part1 (| part2)? ]] | |
| const matches = [...log.message.matchAll(/\[\[CURRENTS\.ATTACHMENT\|([^|\]]+)(?:\|([^\]]+))?\]\]/g)]; | |
| if (matches.length === 0) return []; | |
| const line = getLineFromOrigin(log.origin, testFilePath); | |
| return matches.map(match => { | |
| const filePath = match[1].trim(); | |
| const levelRaw = match[2]?.trim(); | |
| // Validate level if provided, default to 'attempt' | |
| let level = 'attempt'; | |
| if (levelRaw && ['spec', 'test', 'attempt'].includes(levelRaw)) { | |
| level = levelRaw; | |
| } | |
| const artifact = { | |
| path: filePath, | |
| type: inferArtifactType(filePath), | |
| contentType: 'application/octet-stream', | |
| level: level, | |
| }; | |
| return { | |
| key: 'JSON_ARTIFACT', | |
| value: JSON.stringify(artifact), | |
| line | |
| }; | |
| }); | |
| }) | |
| .flat(); | |
| } | |
| function parseAttachmentLogs( | |
| consoleEntries: TestResult['console'], | |
| testFilePath: string | |
| ): PropertyLog[] { | |
| return (consoleEntries ?? []) | |
| .map((log) => { | |
| // Look for [[ATTACHMENT|path]] or [[ATTACHMENT|path|level]] pattern | |
| // We use a regex that optionally captures a second parameter (level) | |
| // Format: [[CURRENTS.ATTACHMENT|path]] or [[CURRENTS.ATTACHMENT|path|level]] | |
| // The path cannot contain ']', so we use [^\]|]+ for path segment if followed by another |? | |
| // Simpler: [[CURRENTS.ATTACHMENT| part1 (| part2)? ]] | |
| const matches = [...log.message.matchAll(/\[\[(?:CURRENTS\.)?ATTACHMENT\|([^|\]]+)(?:\|([^\]]+))?\]\]/g)]; | |
| if (matches.length === 0) return []; | |
| const line = getLineFromOrigin(log.origin, testFilePath); | |
| return matches.map(match => { | |
| const filePath = match[1].trim(); | |
| const levelRaw = match[2]?.trim(); | |
| // Validate level if provided, default to 'attempt' | |
| let level = 'attempt'; | |
| if (levelRaw && ['spec', 'test', 'attempt'].includes(levelRaw)) { | |
| level = levelRaw; | |
| } | |
| const artifact = { | |
| path: filePath, | |
| type: inferArtifactType(filePath), | |
| contentType: 'application/octet-stream', | |
| level: level, | |
| }; | |
| return { | |
| key: 'JSON_ARTIFACT', | |
| value: JSON.stringify(artifact), | |
| line | |
| }; | |
| }); | |
| }) | |
| .flat(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/jest/src/artifacts.ts` around lines 391 - 434, The regex in
parseAttachmentLogs currently only matches the marker with the CURRENTS. prefix;
update the pattern used in parseAttachmentLogs to accept both forms
([[ATTACHMENT|...]] and [[CURRENTS.ATTACHMENT|...]]). Modify the matchAll regex
(in parseAttachmentLogs) to make the "CURRENTS\." prefix optional (e.g. allow an
optional group before ATTACHMENT) while preserving the existing capture groups
for path and optional level, then run/adjust tests in packages/cmd and the
convert-artifacts tests to ensure backward compatibility.
Comment on lines
+363
to
+383
| _testArtifacts: index === 0 ? testArtifacts : [], | ||
|
|
||
| errors, | ||
| error: errors[0], | ||
| }; | ||
| }) | ||
| ), | ||
| }; | ||
| } | ||
| ) | ||
| ); | ||
|
|
||
| const flakyCount = tests.filter((t) => t.isFlaky).length; | ||
|
|
||
| // Move test-level artifacts from attempts to test object | ||
| tests.forEach((t) => { | ||
| const firstAttempt = t.attempts[0]; | ||
| if (firstAttempt && (firstAttempt as any)._testArtifacts) { | ||
| t.artifacts = (firstAttempt as any)._testArtifacts; | ||
| delete (firstAttempt as any)._testArtifacts; | ||
| } |
There was a problem hiding this comment.
Test-level artifacts are currently sourced only from attempt 0.
Line 363 gates _testArtifacts to index === 0, and Lines 379-383 only promote from the first attempt. If test-level artifacts are emitted during retries, they can be lost.
💡 Suggested fix
- _testArtifacts: index === 0 ? testArtifacts : [],
+ _testArtifacts: testArtifacts,
...
- tests.forEach((t) => {
- const firstAttempt = t.attempts[0];
- if (firstAttempt && (firstAttempt as any)._testArtifacts) {
- t.artifacts = (firstAttempt as any)._testArtifacts;
- delete (firstAttempt as any)._testArtifacts;
- }
- });
+ tests.forEach((t) => {
+ const collected = t.attempts.flatMap(
+ (a) => ((a as any)._testArtifacts as any[]) ?? []
+ );
+ if (collected.length > 0) {
+ t.artifacts = collected;
+ }
+ t.attempts.forEach((a) => delete (a as any)._testArtifacts);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| _testArtifacts: index === 0 ? testArtifacts : [], | |
| errors, | |
| error: errors[0], | |
| }; | |
| }) | |
| ), | |
| }; | |
| } | |
| ) | |
| ); | |
| const flakyCount = tests.filter((t) => t.isFlaky).length; | |
| // Move test-level artifacts from attempts to test object | |
| tests.forEach((t) => { | |
| const firstAttempt = t.attempts[0]; | |
| if (firstAttempt && (firstAttempt as any)._testArtifacts) { | |
| t.artifacts = (firstAttempt as any)._testArtifacts; | |
| delete (firstAttempt as any)._testArtifacts; | |
| } | |
| _testArtifacts: testArtifacts, | |
| errors, | |
| error: errors[0], | |
| }; | |
| }) | |
| ), | |
| }; | |
| } | |
| ) | |
| ); | |
| const flakyCount = tests.filter((t) => t.isFlaky).length; | |
| // Move test-level artifacts from attempts to test object | |
| tests.forEach((t) => { | |
| const collected = t.attempts.flatMap( | |
| (a) => ((a as any)._testArtifacts as any[]) ?? [] | |
| ); | |
| if (collected.length > 0) { | |
| t.artifacts = collected; | |
| } | |
| t.attempts.forEach((a) => delete (a as any)._testArtifacts); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/jest/src/reporter.ts` around lines 363 - 383, The code currently
only sets `_testArtifacts` when `index === 0` and later promotes artifacts from
`t.attempts[0]`, which drops artifacts emitted on retries; remove the `index ===
0` gating so each attempt can set `_testArtifacts`, and change the promotion
logic in the `tests.forEach` block to locate the first (or last) attempt that
has a `_testArtifacts` property (e.g., find(attempt => attempt._testArtifacts)
or scan from last to first) and assign that attempt's `_testArtifacts` to
`t.artifacts`, then delete the `_testArtifacts` from that attempt; update
references to `_testArtifacts`, `tests.forEach`, and `t.attempts` accordingly.
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
3 participants
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.
Add Artifact Support for JUnit / Vitest and Improve Jest Artifacts
Summary
This branch extends Currents artifact support to JUnit-based flows and adds a concrete Vitest example. It also finalizes artifact handling in the Jest reporter.
@currents/jestnow generates and uploads artifacts for stdout/stderr and media files (screenshots, videos).currents convert --input-format=junitcan now turn JUnit logs and attachment markers into Currents artifacts.JUnit Converter Changes
Location:
packages/cmd/src/services/convert/index.tsWrites
stdoutto a text file in.currents/.../artifacts/and adds astdoutartifact.Writes
stderrto a text file and also adds astdoutartifact (API only supportsstdoutfor log artifacts).Scans
stdoutandstderrfor markers:[[ATTACHMENT|/absolute/or/relative/path/to/file.ext]]For each marker:
.currents/.../artifacts/<hash>.<ext>.png,jpg,jpeg,bmp) →type: screenshotmp4,webm) →type: videotype: attachmentAll of these appear under
instances[].results.tests[].attempts[].artifacts[]and are compatible with the generic artifact API.Jest Reporter Changes
Location:
packages/jest/src/reporter.tsCollects console output per test file, including special markers:
[[ATTACHMENT|/absolute/or/relative/path/to/file.ext]]For each test case:
.currents/.../artifacts/directory.stdoutartifacts pointing to those log files.[[ATTACHMENT|...]]markers, infers type/content type from extension (same rules as above), copies the files intoartifacts/, and attaches them asscreenshot/video/attachmentartifacts to the correct test attempt.Example usage lives under
examples/jest/src/artifacts, where tests:[[ATTACHMENT|...]]markers so those files appear as artifacts in Currents.Vitest JUnit Example
Location:
examples/vitestvitest.config.tsenables the built-in JUnit reporter:["default", ["junit", { outputFile: "./junit.xml" }]]artifacts.test.ts:Creates
./artifacts/vitest-screenshot.bmpprogrammatically.Logs a marker from inside the test:
console.log([[ATTACHMENT|<absolute path to vitest-screenshot.bmp>]])Asserts that the file exists.
Flow to reproduce:
The generated
.currents/...directory will contain:type: stdout, text files).type: screenshot) for the BMP file.How Users Should Use This (JUnit-based Flows)
Ensure the test framework emits JUnit XML and includes console output per
<testcase>in<system-out>.When a test generates an artifact file, log a marker from that test:
[[ATTACHMENT|/path/to/file.ext]]Run:
The converter will:
stdoutartifacts.artifacts/directory.screenshot,video, orattachment.Summary by CodeRabbit
Release Notes
New Features
Documentation