Skip to content

feat(artifact): add direct log artifact upload to action runtime#396

Merged
marcusrbrown merged 1 commit intomainfrom
feat/direct-artifact-upload
Mar 28, 2026
Merged

feat(artifact): add direct log artifact upload to action runtime#396
marcusrbrown merged 1 commit intomainfrom
feat/direct-artifact-upload

Conversation

@marcusrbrown
Copy link
Copy Markdown
Collaborator

Summary

  • Move OpenCode log artifact upload from workflow YAML steps (actions/upload-artifact) into the action's TypeScript runtime
  • When OPENCODE_PROMPT_ARTIFACT is enabled, the action automatically uploads ~/.local/share/opencode/log as a GitHub artifact — consumers no longer need a separate upload-artifact workflow step
  • Artifact naming follows the existing pattern: opencode-logs-{runId}-{runAttempt}

Changes

New: src/services/artifact/ (Layer 1)

  • upload.tsuploadLogArtifact() wrapping @actions/artifact DefaultArtifactClient. Collects files via fs.readdir({recursive: true}), uploads with 7-day retention and max compression. Non-fatal on failure.
  • index.ts — Re-exports

Modified: Harness (Layer 3)

  • cleanup.ts — Uploads artifacts after cache save when OPENCODE_PROMPT_ARTIFACT is set; writes ARTIFACT_UPLOADED state on success
  • post.ts — Fallback artifact upload in post-action hook if cleanup didn't complete it (mirrors the cache save double-write pattern)
  • state-keys.ts — Added ARTIFACT_UPLOADED state key for main↔post handoff

Modified: Shared (Layer 0)

  • env.ts — Added getGitHubRunAttempt() with Number.isFinite() guard against NaN/negative values

Modified: Workflows

  • fro-bot.yaml / ci.yaml — Removed actions/upload-artifact steps (replaced by in-action upload)

New dependency

  • @actions/artifact@6.2.1 — bundled by tsdown via existing @actions/* rule

How it works

  1. execution.ts writes prompt artifact to log dir when OPENCODE_PROMPT_ARTIFACT=true (existing, unchanged)
  2. Cleanup phase (main action finally block) uploads the log directory as a GitHub artifact
  3. Post-action hook checks ARTIFACT_UPLOADED state — uploads only if cleanup didn't complete it
  4. Both paths are non-fatal: upload failures log warnings but never crash the action

Test coverage

  • upload.test.ts (6 tests) — dir missing, empty dir, upload success/failure, custom options, directory entry filtering
  • env.test.ts (6 new) — getGitHubRunAttempt with valid, missing, empty, non-numeric, zero, negative values
  • post.test.ts (4 new) — artifact upload enabled+not-yet-uploaded, already-uploaded skip, disabled skip, non-fatal failure
  • state-keys.test.ts (2 updated) — new key + count

1085 tests pass, 0 lint errors, type-check clean, dist in sync.

@marcusrbrown marcusrbrown requested a review from fro-bot as a code owner March 28, 2026 19:37
Move OpenCode log artifact upload from workflow YAML steps into the
action's TypeScript runtime. When OPENCODE_PROMPT_ARTIFACT is enabled,
the action now uploads ~/.local/share/opencode/log as a GitHub artifact
automatically — no separate upload-artifact step needed.

- Add @actions/artifact service with DefaultArtifactClient wrapper
- Upload in cleanup phase (primary) and post hook (fallback)
- Track upload state via ARTIFACT_UPLOADED to prevent duplicates
- Add getGitHubRunAttempt() with NaN/negative guard
- Remove upload-artifact steps from fro-bot.yaml and ci.yaml
- Add unit tests for upload service, env accessor, and post branching
@marcusrbrown marcusrbrown force-pushed the feat/direct-artifact-upload branch from e396018 to 1b6c248 Compare March 28, 2026 19:42
Copy link
Copy Markdown
Owner

@fro-bot fro-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This PR demonstrates excellent adherence to established codebase patterns. The implementation is well-structured with comprehensive test coverage.

Architecture & Design ✓

  • Layer placement: New services/artifact/ module correctly positioned as Layer 1 (services layer)
  • Double-write pattern: Post-action fallback mirrors the existing cache save pattern (STATE_KEYS.CACHE_SAVEDSTATE_KEYS.ARTIFACT_UPLOADED)
  • Non-fatal error handling: Both upload paths gracefully degrade with warnings, never crashing the action

Code Quality ✓

  • Logger injection: All functions properly inject logger
  • Functional style: No classes, only pure functions
  • ESM imports: .js extensions correctly used
  • Strict booleans: Explicit comparisons (=== 'true', !== 'true') not implicit falsy
  • Guard conditions: getGitHubRunAttempt() uses Number.isFinite() and guards against zero/negative values

Test Coverage ✓

6 new tests in upload.test.ts + 4 new tests in post.test.ts + 6 new tests in env.test.ts:

  • Missing directory handling
  • Empty directory handling
  • Upload success/failure paths
  • Custom options passthrough
  • Directory entry filtering (files vs directories)
  • State key verification

Implementation Details ✓

  1. upload.ts: Clean implementation with graceful handling of missing/empty directories
  2. cleanup.ts: Correctly uploads after cache save when OPENCODE_PROMPT_ARTIFACT is enabled
  3. post.ts: Fallback upload via state check mirrors cache save pattern
  4. getGitHubRunAttempt(): Returns 1 for invalid/missing/zero/negative values (correct default)

Bundling ✓

@actions/artifact@6.2.1 correctly bundled via existing noExternal rule for @actions/* packages (line 172 in tsdown.config.ts).

Verification ✓

  • 1085 tests pass
  • Type check clean
  • Lint clean (34 warnings are pre-existing, none in new code)
  • Dist in sync

No blocking issues found. The implementation is production-ready.

Run Summary
Field Value
Event pull_request
Repository fro-bot/agent
Run ID 23692874135
Cache hit
Session ses_2ca067d11ffe5emqJuH27wRBgb

@marcusrbrown marcusrbrown merged commit 65ae2e9 into main Mar 28, 2026
10 checks passed
@fro-bot fro-bot mentioned this pull request Mar 29, 2026
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants