Skip to content

ci: reduce actions storage churn - #673

Merged
aram-devdocs merged 1 commit into
mainfrom
codex/actions-storage-cleanup
Apr 8, 2026
Merged

ci: reduce actions storage churn#673
aram-devdocs merged 1 commit into
mainfrom
codex/actions-storage-cleanup

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Overview

Type: ci

Summary:
Reduce GitHub Actions storage churn by shortening artifact retention where artifacts are short-lived, stopping unnecessary PR artifact uploads, and narrowing PR remote sccache usage to the primary feedback lanes.

Related Issues: None


Changes Made

Engine Core (goud_engine/src/)

No changes

FFI Layer (goud_engine/src/ffi/)

No changes

C# SDK (sdks/csharp/)

No changes

Python SDK (sdks/python/)

No changes

TypeScript SDK (sdks/typescript/)

  • Node napi binding changes
  • Web WASM binding changes
  • Type definition changes
  • Tests updated
    No SDK code changes. CI now skips PR uploads for ts-native-* and ts-wasm, and keeps shorter retention for generated coverage artifacts.

Codegen Pipeline (codegen/)

  • Schema changes
  • Generator changes
  • Validator changes
  • ffi_mapping changes
    No changes

Proc Macros (goud_engine_macros/)

  • #[goud_api] attribute changes
    No changes

Tools (tools/)

  • lint-layers changes
    No changes

WASM (goud_engine/src/wasm/)

  • wasm-bindgen exports
  • Sprite renderer changes
  • Texture loader changes
    No changes

Examples (examples/)

No changes

Documentation

No changes


Architectural Compliance

  • Rust-first: All logic lives in Rust; SDKs are thin wrappers
  • FFI boundary: New exports use #[no_mangle] extern "C" and #[repr(C)] where needed
  • Dependency flow: Imports follow layer hierarchy (down only)
  • SDK parity: Changes exposed via FFI are wrapped in C#, Python, AND TypeScript SDKs
  • Unsafe discipline: No unsafe block without a // SAFETY: comment
  • File size: No file exceeds 500 lines

Testing

  • cargo test passes
  • cargo clippy -- -D warnings is clean
  • cargo fmt --all -- --check passes
  • Python SDK tests pass (python3 sdks/python/test_bindings.py) — if SDK changed
  • C# SDK tests pass (dotnet test sdks/csharp.tests/) — if SDK changed
  • TypeScript SDK tests pass (cd sdks/typescript && npm test) — if TS SDK changed
  • Codegen produces consistent output (python3 codegen/validate.py && python3 codegen/validate_coverage.py)
  • Pre-commit hooks pass

Validation run for this PR:

  • git diff --check -- .github/workflows/ci.yml .github/workflows/security.yml .github/workflows/benchmarks.yml .github/workflows/release.yml
  • Ruby YAML parse for the touched workflow files after replacing GitHub expression blocks
  • One reviewer pass focused on workflow regressions

actionlint is not installed in this environment, so I could not run it.


Code Quality

  • No todo!() or unimplemented!() in production code
  • No #[allow(unused)] without justification comment
  • Error handling uses Result, not unwrap()/expect() in library code
  • Public items have doc comments

Documentation

  • Updated relevant AGENTS.md files (if architecture changed)
  • Updated README.md (if user-facing behavior changed)
  • Added or updated doc comments on new public APIs

No documentation changes were needed.


Breaking Changes

None

  • API changes:
  • FFI signature changes:
  • SDK interface changes:

Version Bump

Bump type: none
Justification: CI/workflow-only cleanup


Security

  • No new unsafe blocks — or each one has a // SAFETY: comment and is necessary
  • No new FFI pointer parameters without null checks
  • No new dependencies with known advisories (cargo deny check)
  • No secrets or credentials in committed files

Performance

Build performance on main is unchanged.

For PRs, remote sccache is now kept only on the primary Ubuntu Rust lane and the Ubuntu TypeScript native lane. Secondary PR lanes may rebuild more often, but this avoids the broad PR cache churn that was driving Actions storage growth.


Deployment

  • NuGet package version updated (if SDK changed)
  • Python package version updated (if SDK changed)
  • npm package version updated (if TS SDK changed)
  • Native library builds on all targets (macOS, Linux, Windows)

No package publishing changes.


Reviewer Notes

Main tradeoffs:

  • native-release-*, ts-native-*, ts-wasm, the merge-group coverage bundle, and Android screenshots now expire faster.
  • PR runs no longer upload ts-native-* or ts-wasm artifacts for manual download.
  • Release workflow intermediate artifacts stay at 3 days because those jobs can sit behind approval gates.

@github-actions github-actions Bot added the size/S label Apr 8, 2026
@claude

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aram-devdocs's task in 2m 31s —— View job


PR Review: ci: reduce actions storage churn

  • Gather context and read changed files
  • Phase 1: Context gathering
  • Phase 2: Architecture validation
  • Phase 3: Anti-pattern scan
  • Phase 4: Quality assessment
  • Phase 5: Security review
  • Phase 6: Scope verification
  • Phase 7: Review output

Verdict: APPROVED

CI/workflow-only change. No Rust, FFI, or SDK code touched. All three phases of the PR description are accurately reflected in the diff.


BLOCKERS

None.


WARNINGS (non-blocking)

# Location Issue
W1 ci.yml:770-776 ts-native-* artifacts are no longer uploaded on PRs at all (previously ubuntu-latest was still uploaded). This matches the description but means developers cannot manually download a compiled .node addon from a PR run. Worth a note in the PR for contributors who relied on that workflow.
W2 ci.yml:648-655 sccache is still enabled for ts-native jobs on PR/ubuntu-latest via (github.event_name != 'pull_request' || matrix.os == 'ubuntu-latest'). This is intentional but creates an asymmetry: the ubuntu-latest ts-native lane gets cache on PRs while all other PR ts-native lanes do cold builds. Fine as documented, just ensure the tradeoff is understood.
W3 benchmarks.yml:39-45 The criterion-reports upload still uses if: always(). With retention now at 7 days (down from 30), this is fine — just noting that failed benchmark runs still upload their partial output.

Positive Callouts

android-smoke-screenshot condition (ci.yml:1227): Changing if: always() to if: failure() is the right call. Screenshots are only useful for debugging failures; uploading them on success was pure waste.

matrix.lane expression (ci.yml:112): The sccache condition (matrix.lane == 'full') is valid — the matrix uses a fromJSON include with explicit lane values ("full", "smoke", "clippy-only"). Expression is correct and the primary feedback lane (ubuntu-latest / stable / full) keeps its cache on PRs.

Explicit retention on previously-unbounded artifacts: python-coverage, dotnet-test-results, dotnet-coverage, ts-coverage, ts-web-coverage, and rust-outdated-report all lacked retention-days before — meaning they defaulted to 90 days. Adding explicit 3-day retention on these is good hygiene and directly addresses the storage churn goal.

ts-wasm artifact skip is safe: No downstream job in ci.yml downloads ts-wasm or ts-native-* by artifact name, so skipping the PR upload does not break any job dependencies.


Summary

Three files changed, all CI workflow YAML. The changes are coherent, the tradeoffs are clearly explained in the PR body, and no regressions are introduced. The matrix.lane sccache condition is valid. The native-release-* 1-day retention is sufficient since all consumers run within the same workflow run.

@aram-devdocs
aram-devdocs merged commit eec5f6b into main Apr 8, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant