feat: rebuild the action on the effected Effect v4 suite - #206
Conversation
Complete greenfield rebuild of the internals on @effected/github-actions, npm, package-json and semver, replacing @savvy-web/github-action-effects. The public interface is unchanged: same inputs, outputs, devEngines contract, job summary and log output — consuming repositories need no changes. Package managers are now provisioned directly into the tool cache without corepack, and the dependency cache key gains a CPU-architecture segment and a two-rung restore ladder. The embedded Turbo remote-cache server gets a per-run random auth token that fails closed, graceful SIGTERM teardown and EADDRINUSE handling. Lifecycle scripts in consuming repos now see the installed runtimes and package manager on PATH, and Windows package-manager shims are invoked safely (CVE-2024-27980 mitigation). Ships with a rebuilt unit suite (448 tests), a hardened fixture harness, reconciled design and context docs, and a rewritten README. Signed-off-by: C. Spencer Beggs <spencer@savvyweb.systems> Claude-Session: https://claude.ai/code/session_01YLYULzeqGaeTYSe4tWoNUv
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughChangesThe action was reorganized around typed Effect schemas and dedicated steps for configuration loading, runtime and package-manager installation, dependency caching, Biome installation, Turbo caching, output emission, and job summaries. Application and post-action layers now use the new services and state formats. Workflows and unit tests cover the new behavior. Runtime and cache architecture
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Action
participant Program
participant Installers
participant Cache
participant Turbo
participant Summary
Action->>Program: load inputs and configuration
Program->>Installers: install runtimes and package manager
Program->>Cache: restore dependency cache
Program->>Turbo: start Turbo cache
Program->>Summary: emit outputs and write summary
Action->>Cache: save cache during post phase
Action->>Turbo: reap embedded server
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 26
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__test__/unit/layers.test.ts`:
- Around line 39-49: Update the MainLive test to import and request
PackageManagerInstaller alongside ActionCache and ToolInstaller in the
Effect.all service list, then assert all three services are provided so the test
verifies MainLive's required service contract.
In `@__test__/unit/program.test.ts`:
- Around line 405-427: Replace the manual process.env.PATH try/finally
restoration in the three tests around the current test and the adjacent
PATH-mutation tests with the existing withEnvRestored helper. Wrap each test
body using the helper’s established API so PATH is restored exactly, including
deleting it when it was initially unset, while preserving each test’s existing
assertions and behavior.
- Around line 61-74: Update childProcessSpawnerTest to construct the
ChildProcessSpawner service through ChildProcessSpawnerNS.make, matching the
pattern in layers.test.ts. Pass the existing spawn, exitCode, and quietHandle
behavior to the factory, and remove the manually defined streamString,
streamLines, lines, and string members.
In `@__test__/unit/schema/domain.test.ts`:
- Around line 23-38: Extend the invalid-version cases in the bad-input list of
the domain schema test to include a string using “<”, a bare “=”, and uppercase
“X”. Keep these additions alongside the existing rejected range-character cases.
In `@__test__/unit/schema/outputs.test.ts`:
- Around line 54-75: Update the emitted helper and the “writes every action.yml
output exactly once” test to record every publication in an array alongside the
existing Map, then assert the publication count equals OUTPUT_NAMES.length. Keep
the Map assertions for final values and ensure duplicate calls remain visible to
the count.
In `@__test__/unit/state.test.ts`:
- Around line 54-72: The tests invoking acrossPhases and makeTempDirectoryScoped
must run with scoped test support. Update the relevant `@effect/vitest` test
declarations from it.effect to it.scoped, preserving the existing CacheState
round-trip assertions and test logic.
In `@__test__/unit/steps/install-dependencies.test.ts`:
- Around line 377-397: Update the test around run to use the platform-resolved
pathKey consistently: capture the existing live value through pathKey before
modifying the environment, restore that same key in finally, and assert the
spawned environment using pathKey rather than separately reading or deleting
PATH. Preserve the test’s case-insensitive Windows behavior while avoiding reads
and writes through different spellings of the same environment key.
In `@__test__/unit/turbo-cache/activation.test.ts`:
- Around line 8-28: Extract the duplicated baseInputs fixture and
inputs(overrides) builder into a shared test helper module, then replace the
local definitions in activation.test.ts, restore-cache.test.ts, and
turbo-cache.test.ts with imports from that helper. Preserve the existing fixture
values and builder behavior across all three suites.
In `@__test__/unit/turbo-cache/meta.test.ts`:
- Around line 53-63: Annotate the tuple array passed to the duration-clamping
it.each test so each row is typed as [string, number, number]. Keep the existing
test cases and clampDurationMs assertions unchanged, ensuring input is inferred
as number rather than string | number.
In `@__test__/unit/turbo-cache/server-config.test.ts`:
- Around line 174-199: The test should exercise an actual S3 store operation
while console.log is being captured, rather than relying only on build’s typeof
probes. In the “never writes the s3 credentials” test, invoke an appropriate
operation on the built store inside the existing capture window, or otherwise
verify that layer construction performs declassification, while preserving the
assertions that neither credential nor “add-mask” appears in captured output.
In @.github/workflows/test-turbo-cache.yml:
- Around line 194-220: Add cleanup for the run-scoped cache objects created by
the workflow’s turbo-cache-prefix under ci/. Prefer configuring an S3 lifecycle
rule to expire objects with the ci/ prefix after a short retention period;
alternatively, add an always-running cleanup step that deletes the current run’s
ci/${{ github.run_id }}-${{ github.run_attempt }}/ prefix using the configured
bucket and endpoint credentials.
In @.github/workflows/test.yml:
- Line 47: Add a bun entry to the test-node-restore-cache matrix alongside npm,
pnpm, yarn, and multi, setting expected-cache-hit to "true" so it restores the
cache created by test-node-create-cache.
In @.gitmodules:
- Around line 1-4: Update the .repos/effect submodule configuration so shallow
initialization fetches a branch or tag containing the recorded commit, using
appropriate branch/shallow-branch settings; alternatively remove shallow = true
to allow the commit to be fetched directly. Preserve the existing submodule path
and URL.
In `@package.json`:
- Around line 34-48: Remove the unused `@effected/`* entries from the dependencies
section of package.json: `@effected/commands`, `@effected/git`, `@effected/github`,
`@effected/glob`, `@effected/lockfiles`, `@effected/markdown`, `@effected/package-json`,
`@effected/runtimes`, `@effected/sbom`, `@effected/workspaces`, and `@effected/yaml`.
Preserve the existing `@effected/github-actions`, `@effected/jsonc`, `@effected/npm`,
and `@effected/semver` dependencies.
In `@README.md`:
- Line 4: Update the Node.js badge in the README so its Markdown alt text uses
the readable “>=” characters rather than the percent-encoded “%3E%3D”; keep the
percent-encoding unchanged in the badge URL.
In `@src/descriptors/biome.ts`:
- Around line 63-69: Validate and normalize resolved Biome versions before they
reach the URL construction in the plan method: apply SemVer.ExactVersionString
to biome-version overrides and validate the config-file $schema capture,
rejecting values that are not safe exact versions or contain path traversal such
as "/" or "..". Preserve normal supported-platform handling while ensuring only
validated versions are interpolated into the GitHub release URL.
In `@src/descriptors/node.ts`:
- Around line 39-45: Update the Node tarball descriptor’s tarFlags in the
version URL configuration to use the BSD-compatible strip-components option
instead of --strip=1, while preserving the existing extraction flags and
behavior on other platforms.
In `@src/steps/cache-config.ts`:
- Around line 151-177: Update storePaths for the pnpm and deno cases to add
darwin-specific defaults: use ~/Library/pnpm/store for pnpm and
~/Library/Caches/deno for deno, while preserving the existing Windows and Linux
paths.
In `@src/steps/install-dependencies.ts`:
- Around line 75-81: Update anyLockfile’s Effect.reduce call to use false
directly as the initial boolean accumulator instead of () => false, preserving
the existing short-circuiting lockfile checks and installDependencies selection
behavior.
In `@src/steps/install-runtimes.ts`:
- Line 82: Remove the redundant Error check at line 82 because Error instances
are already handled by the preceding object branch; retain the existing handling
for empty-message Errors without adding unrelated changes.
In `@src/steps/setup-package-manager.ts`:
- Around line 9-12: Update the docstring for the setup-package-manager error to
remove the stale “activated via corepack” wording. Describe failure to install,
activate through the current addPath-based flow, or verify the package manager,
consistent with PackageManagerInstaller and the activate behavior.
In `@src/steps/turbo-cache.ts`:
- Around line 254-256: The fixed-port startup flow must not accept a stale
server as ready when readinessProbe checks the unauthenticated status endpoint.
Update the readinessProbe/spawn logic around DEFAULT_TURBO_SERVER_PORT and
credential so readiness is tied to this spawn’s token, or ensure an EADDRINUSE
listen failure rejects startup instead of proceeding with the existing server;
preserve authenticated Turbo cache requests using the new credential.
In `@src/turbo-cache/handler.ts`:
- Around line 89-93: Update artifactHash to reject "." and ".." in addition to
empty values and segments containing "/", before constructing any artifact key.
Ensure the BlobStore backends also enforce artifact-key containment, preserving
valid single-segment hashes.
In `@src/turbo-server.ts`:
- Around line 121-129: Update the SIGTERM handler to call
server.closeIdleConnections() immediately after server.close(), ensuring idle
keep-alive sockets are closed so the close callback reaches runtime.dispose()
before the shutdown deadline.
- Around line 71-74: Add an error listener to the request stream alongside the
existing data and end listeners in the request handling flow. On request-stream
errors, discard the accumulated chunks and send the appropriate response only
when the socket remains writable, preventing aborted uploads from becoming
uncaught exceptions while preserving normal end handling.
In `@vitest.config.ts`:
- Around line 25-26: Update the coverage configuration in vitest.config.ts to
exclude the detached src/turbo-server.ts entry point from coverage, while
continuing to include other src/**/*.ts files so unimported source files are
scored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6c4577f2-9e9d-4325-9480-0ca169c44877
⛔ Files ignored due to path filters (22)
.changeset/happy-trees-rest.mdis excluded by!.changeset/**.changeset/sturdy-engines-ignite.mdis excluded by!.changeset/**.claude/design/refs.jsonis excluded by!.claude/**.claude/design/silk-runtime-action/INDEX.mdis excluded by!.claude/**.claude/design/silk-runtime-action/architecture.mdis excluded by!.claude/**.claude/design/silk-runtime-action/build-and-distribution.mdis excluded by!.claude/**.claude/design/silk-runtime-action/caching-strategy.mdis excluded by!.claude/**.claude/design/silk-runtime-action/effect-service-model.mdis excluded by!.claude/**.claude/design/silk-runtime-action/runtime-installation.mdis excluded by!.claude/**.claude/design/silk-runtime-action/testing-strategy.mdis excluded by!.claude/**.claude/design/silk-runtime-action/turbo-remote-cache.mdis excluded by!.claude/**.github/actions/local/dist/main.jsis excluded by!**/dist/**.github/actions/local/dist/post.jsis excluded by!**/dist/**.github/actions/local/dist/turbo-server.jsis excluded by!**/dist/**CLAUDE.mdis excluded by!**/CLAUDE.md__fixtures__/CLAUDE.mdis excluded by!**/CLAUDE.md__fixtures__/bun-bun/bun.lockis excluded by!**/*.lockdist/main.jsis excluded by!**/dist/**,!dist/**dist/post.jsis excluded by!**/dist/**,!dist/**dist/turbo-server.jsis excluded by!**/dist/**,!dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/CLAUDE.mdis excluded by!**/CLAUDE.md
📒 Files selected for processing (95)
.github/actions/test-fixture/action.yml.github/workflows/test-turbo-cache.yml.github/workflows/test.yml.gitignore.gitmodules.repos/config.json.repos/effect.repos/effect-smolLICENSEREADME.md__fixtures__/bun-bun/package.json__test__/unit/descriptors.test.ts__test__/unit/layers.test.ts__test__/unit/post.test.ts__test__/unit/program.test.ts__test__/unit/schema/domain.test.ts__test__/unit/schema/inputs.test.ts__test__/unit/schema/outputs.test.ts__test__/unit/state.test.ts__test__/unit/steps/cache-config.test.ts__test__/unit/steps/detect-biome.test.ts__test__/unit/steps/detect-turbo.test.ts__test__/unit/steps/install-biome.test.ts__test__/unit/steps/install-dependencies.test.ts__test__/unit/steps/install-runtimes.test.ts__test__/unit/steps/load-config.test.ts__test__/unit/steps/restore-cache.test.ts__test__/unit/steps/setup-package-manager.test.ts__test__/unit/steps/steps.test.ts__test__/unit/steps/summary.test.ts__test__/unit/steps/turbo-cache.test.ts__test__/unit/summary/format.test.ts__test__/unit/turbo-cache/activation.test.ts__test__/unit/turbo-cache/handler.test.ts__test__/unit/turbo-cache/meta.test.ts__test__/unit/turbo-cache/server-config.test.tsaction.config.tspackage.jsonsrc/descriptors/biome.tssrc/descriptors/bun.tssrc/descriptors/deno.tssrc/descriptors/descriptor.tssrc/descriptors/descriptors.test.tssrc/descriptors/node.tssrc/errors/errors.test.tssrc/errors/errors.tssrc/layers/app.tssrc/main.tssrc/post.test.tssrc/post.tssrc/program.test.tssrc/program.tssrc/schema/domain.tssrc/schema/inputs.tssrc/schema/outputs.tssrc/schemas/domain.test.tssrc/schemas/domain.tssrc/services/cache.test.tssrc/services/cache.tssrc/services/config-loader.test.tssrc/services/config-loader.tssrc/services/runtime-installer.test.tssrc/services/runtime-installer.tssrc/services/summary.test.tssrc/services/summary.tssrc/services/turbo-cache/activation.test.tssrc/services/turbo-cache/activation.tssrc/services/turbo-cache/apply.test.tssrc/services/turbo-cache/apply.tssrc/services/turbo-cache/codec.test.tssrc/services/turbo-cache/codec.tssrc/services/turbo-cache/handler.test.tssrc/services/turbo-cache/handler.tssrc/services/turbo-cache/lifecycle.test.tssrc/services/turbo-cache/lifecycle.tssrc/state.test.tssrc/state.tssrc/steps/cache-config.tssrc/steps/detect-biome.tssrc/steps/detect-turbo.tssrc/steps/install-biome.tssrc/steps/install-dependencies.tssrc/steps/install-runtimes.tssrc/steps/load-config.tssrc/steps/restore-cache.tssrc/steps/setup-package-manager.tssrc/steps/summary.tssrc/steps/turbo-cache.tssrc/summary/format.tssrc/turbo-cache/activation.tssrc/turbo-cache/handler.tssrc/turbo-cache/meta.tssrc/turbo-cache/server-config.tssrc/turbo-server.tsvitest.config.ts
💤 Files with no reviewable changes (27)
- src/errors/errors.test.ts
- src/state.test.ts
- src/services/config-loader.test.ts
- .repos/effect-smol
- src/services/runtime-installer.test.ts
- src/descriptors/descriptors.test.ts
- src/services/turbo-cache/apply.ts
- src/services/turbo-cache/apply.test.ts
- src/services/turbo-cache/handler.test.ts
- src/services/summary.ts
- src/services/cache.test.ts
- src/services/runtime-installer.ts
- src/post.test.ts
- src/services/turbo-cache/lifecycle.test.ts
- src/services/config-loader.ts
- src/services/turbo-cache/activation.test.ts
- src/services/turbo-cache/codec.test.ts
- src/services/turbo-cache/activation.ts
- src/errors/errors.ts
- src/schemas/domain.test.ts
- src/services/summary.test.ts
- src/services/turbo-cache/handler.ts
- src/schemas/domain.ts
- src/services/cache.ts
- src/services/turbo-cache/lifecycle.ts
- src/services/turbo-cache/codec.ts
- src/program.test.ts
Hardens the detached turbo server (request-stream error listener so a client abort cannot kill the process, idle keep-alive sockets closed on SIGTERM so graceful shutdown beats the exit deadline) and refuses relative-segment artifact hashes. Adds a path-safety guard on resolved Biome versions so neither the biome-version input nor a poisoned $schema can redirect the binary download, while npm-style tags keep working. Removes an unreachable error-formatting branch, fixes a stale corepack docstring and the README badge alt text, strengthens three unit assertions, retries the fixed-port bind in the probe test against transient ephemeral-port collisions, and adds the missing bun restore-cache workflow row. Signed-off-by: C. Spencer Beggs <spencer@savvyweb.systems> Claude-Session: https://claude.ai/code/session_01YLYULzeqGaeTYSe4tWoNUv
Signed-off-by: C. Spencer Beggs <spencer@savvyweb.systems> Claude-Session: https://claude.ai/code/session_01YLYULzeqGaeTYSe4tWoNUv
…port This repository's own CI is set up by this very action, so a real turbo cache server holds the default port 41230 for the whole job and the probe test's fixed-port bind failed with EADDRINUSE. The step gains a port seam alongside its existing detached and serverEntry seams, and the test now binds port 0 first and points the step at whatever the OS handed out — no fixed-port bind, no retry loop, deterministic under a busy runner. Verified by running the suite with a listener deliberately holding 41230. Signed-off-by: C. Spencer Beggs <spencer@savvyweb.systems> Claude-Session: https://claude.ai/code/session_01YLYULzeqGaeTYSe4tWoNUv
Complete greenfield rebuild of the internals on @effected/github-actions, npm, package-json and semver, replacing @savvy-web/github-action-effects.
The public interface is unchanged: same inputs, outputs, devEngines contract, job summary and log output — consuming repositories need no changes.
Package managers are now provisioned directly into the tool cache without corepack, and the dependency cache key gains a CPU-architecture segment and a two-rung restore ladder.
The embedded Turbo remote-cache server gets a per-run random auth token that fails closed, graceful SIGTERM teardown and EADDRINUSE handling.
Lifecycle scripts in consuming repos now see the installed runtimes and package manager on PATH, and Windows package-manager shims are invoked safely (CVE-2024-27980 mitigation).
Validated end to end at HEAD: 448 unit tests, the 35-job fixture matrix and the 5-job turbo-cache workflow including real S3, all green.
Ships a minor changeset (1.2.3 → 1.3.0) plus the dependency table, reconciled design and context docs, a rewritten README and the MIT license file.
Signed-off-by: C. Spencer Beggs spencer@savvyweb.systems
https://claude.ai/code/session_01YLYULzeqGaeTYSe4tWoNUv
Rebuild the action internals with
@effected/github-actionsand direct package-manager provisioning. Preserve the public interface,devEnginescontract, outputs, summaries, and logs.Add architecture-aware dependency cache keys with two-level restore. Run lifecycle scripts with installed runtimes and package managers on
PATH.Secure and harden the embedded Turbo cache server with per-run tokens, graceful shutdown, and
EADDRINUSEhandling. Add comprehensive unit and workflow coverage, documentation, an MIT license, and a1.3.0changeset.