Skip to content

Cached outputs are restored onto disk on a cache hit - #150

Merged
calvin-archastro merged 2 commits into
mainfrom
factory/tsk_0RQ4O3dUaMcr4ntlduQypa-cebcfc5c-d225-4561-8c63-
Oct 15, 2026
Merged

calvin-archastro merged 2 commits into
mainfrom
factory/tsk_0RQ4O3dUaMcr4ntlduQypa-cebcfc5c-d225-4561-8c63-

Conversation

@calvin-archastro

@calvin-archastro calvin-archastro commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Epic: Aster restores real build artifacts from a shared cache, including the GitHub Actions cache
Task: Cached outputs are restored onto disk on a cache hit

Local git hooks skipped: pre-push on the submission push and the publish push. CI is the gate for what those hooks check.

Problem and author intent

Aster’s cache could pack declared target outputs into the local content-addressed store, but a hit could not put those artifacts back on disk. The store only recorded hash, timestamp, and success, so stale workspace files were never replaced from blobs.

Pack also rewrote a relative symlink that stayed inside the project into a collapsed project-relative path. A link at dist/links whose text was ../app.js was stored as dist/app.js. Manifest decode rejected any target that was not a strictly safe relative path, so a .. component failed even when resolving it from the link’s parent never left the project. Restoring that collapsed text as the link would point at the wrong place relative to the link parent.

The intent of this change is to materialize a packed entry list under the project root: validate every path before the first delete, replace each declared output (file, directory, or symlink) from the local store, keep original in-project relative link text, and on a missing or corrupt blob remove the declared paths again so the caller can treat the hit as a miss.

What changed

Before, pack collapsed in-project relative symlink targets, and Manifest validation refused those .. targets. There was no restore.

After:

  • src/cache/outputs.rs adds restore(project_root, entries, store). It checks every entry before deleting anything. Absolute paths, empty paths, backslashes, drive-style :, a leading /, or any .. in a declared output path are refused and nothing is removed. Duplicate paths are refused. Entries are then ordered parents-before-children.
  • Each declared path is removed with symlink_metadata. A symlink or non-directory is remove_file (so a directory symlink is not followed); a real directory is remove_dir_all. Removal walks deepest-first.
  • Directories are created and, on Unix, mode bits in 0o777 are applied. File bytes are read from the store into a same-directory temp (.{name}.aster-restore.{pid}.{seq}, create_new), sync_all’d, mode-set, then renamed over the destination. Symlinks are recreated with the stored target text (symlink on Unix, symlink_file on Windows).
  • If a blob is missing or corrupt, or any write fails after a delete, the declared paths are removed again and restore returns Err. A failed write also deletes its temp file.
  • Pack now stores the original relative link text when that text stays inside the project (../app.js stays ../app.js). An absolute link that names a path inside the project is still stored as that project-relative path. Empty targets and targets containing \ or : are rejected.
  • src/cache/artifact.rs accepts a symlink target with .. only when lexical resolution from the link parent does not climb out of the project. Absolute targets, prefixes, and escapes are still rejected.
  • src/cache/mod.rs re-exports restore next to pack.

Scope

Three files: src/cache/artifact.rs, src/cache/mod.rs, and src/cache/outputs.rs (16 hunks, +516 / −13). No omitted hunks, paths, or patch bytes in the supplied diff. Executor cache-hit wiring and remote backends are not in this revision. Advisory pipeline results list three checks (format, lint, test), all succeeded with exit code 0; those log excerpts are truncated, so they are not treated as complete evidence of what ran.

Risk assessment

High. Restore deletes and rewrites paths in the workspace. A bad path or a followed directory symlink could remove the wrong tree.

Mitigations in this change: all entries are validated before the first delete; declared paths cannot contain .. or leave project_root; symlink targets are accepted only when resolution from the link parent stays inside the project; removal uses symlink metadata and does not follow links; file writes are temp-plus-rename; failure deletes the declared roots again. Undeclared siblings are not in the removal list.

Residual risk: path checks are lexical (component walk), not a canonicalized filesystem walk, so unusual link layouts outside the declared entries are not re-resolved at delete time. Cleanup errors on the failure path are ignored. Mode bits are applied only on Unix. Windows restore always uses symlink_file.

User impact

restore is available to cache callers, but this revision does not wire it into the executor. A cache hit still does not restore artifacts until that caller lands.

When restore is invoked, each declared output root is replaced from the local store, including Unix modes and symlink text. Files outside the declared set are left in place. A path that escapes the project fails closed with no deletes. A missing or corrupt blob fails and leaves none of the declared outputs behind, rather than a partial tree.

Testing

New tests are present in the diff; the patch is not evidence that they passed.

In src/cache/outputs.rs:

  • restore_round_trip_matches_packed_tree_including_modes
  • restore_aborts_on_corrupt_blob_and_leaves_no_partial_output
  • restore_refuses_path_that_escapes_project_root
  • restore_same_directory_symlink_stays_inside_the_project
  • restore_removes_a_directory_symlink_without_following_it

In src/cache/artifact.rs:

  • decode_accepts_parent_relative_symlink_that_stays_inside

An existing pack assertion now expects ../app.js instead of the collapsed dist/app.js.

The advisory test check is reported succeeded (exit code 0), as are format and lint. Excerpts are truncated and do not name these functions, so this review cannot confirm from the logs that these tests ran or that the cited CI command covered them. The workflow file is not in this diff. Author notes say the Test job runs cargo test --locked --workspace --all-targets --all-features and that the five restore tests passed; that is a claim, not log evidence here. Windows symlink creation is cfg-gated and is not shown as executed.

Follow-ups and known issues

  • Executor cache-hit wiring and remote CacheBackend support are out of scope; restore is unused by the runner in this revision.
  • Symlink “stays inside” checks are duplicated in artifact.rs and outputs.rs, and they differ (to_str vs to_string_lossy).
  • Non-Unix builds ignore mode bits. Windows restore creates file symlinks only.
  • Failure-path cleanup ignores removal errors, so a second delete failure can leave declared paths after Err.
  • CI excerpts are truncated; do not treat the succeeded test outcome as proof these new cases ran.

@calvin-archastro
calvin-archastro requested a review from a team October 15, 2026 14:54
@calvin-archastro

calvin-archastro commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

ArchDev is watching this pull request. State: settled (cycle 2 of 2 for head d68104d).

when trigger decision result
2026-10-15T15:03:23.326Z review feedback enqueue job_b7188c58952b6235eecff956 published d68104d
2026-10-15T15:18:00.099Z verify head enqueue job_2e4fa30df71dbbca82fbbdec no change

Paused by the no-autofix label or a draft; never resolves reviewer threads. Jobs: job_b7188c58952b6235eecff956, job_2e4fa30df71dbbca82fbbdec.

@calvin-archastro
calvin-archastro merged commit 2338686 into main Oct 15, 2026
12 checks passed
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.

1 participant