diff --git a/crates/hm-dsl-engine/harmont-py/harmont/keygen.py b/crates/hm-dsl-engine/harmont-py/harmont/keygen.py index f7285c33..dcfc78bc 100644 --- a/crates/hm-dsl-engine/harmont-py/harmont/keygen.py +++ b/crates/hm-dsl-engine/harmont-py/harmont/keygen.py @@ -1,13 +1,19 @@ """Cache-key resolver. Direct port of cidsl/lisp/src/harmont_macros.scm (resolve-cache-key -and helpers). Output bytes MUST match the Scheme version so cached -snapshots persisted before the Scheme removal remain reachable. +and helpers). + +NOTE: the key format changed in v0.1 — the step's base ``image`` is now +folded into the outer pre-image so that the same command on two different +base images no longer collides on one cache entry. Old cached snapshots +are intentionally unreachable after this change (a one-time invalidation). Algorithm (pre-image of the outer sha256): pipeline_org NUL pipeline_slug NUL step_key NUL - parent_resolved_key NUL policy_resolution + image NUL parent_resolved_key NUL policy_resolution + +``image`` is the step's ``image`` field, or the empty string when absent. policy_resolution branches: none -> "none" (no key emitted) @@ -62,6 +68,7 @@ def resolve_pipeline_keys( if not cache or cache["policy"] == "none": continue cmd = step.get("cmd", "") + image = step.get("image") or "" parent = parent_key_map.get(step["key"]) parent_resolved = _lookup_parent(parent, resolved) policy_res = _resolve_policy(cache, cmd, now, base_path, env) @@ -72,6 +79,8 @@ def resolve_pipeline_keys( + NUL + step["key"] + NUL + + image + + NUL + parent_resolved + NUL + policy_res diff --git a/crates/hm-dsl-engine/harmont-py/tests/test_keygen.py b/crates/hm-dsl-engine/harmont-py/tests/test_keygen.py index 6675277b..466d8e42 100644 --- a/crates/hm-dsl-engine/harmont-py/tests/test_keygen.py +++ b/crates/hm-dsl-engine/harmont-py/tests/test_keygen.py @@ -1,6 +1,9 @@ """Cache-key resolver -- direct ports of the Scheme algorithm in -harmont_macros.scm. Keys must be byte-identical to what harmont-eval -produced pre-removal, so existing cached snapshots remain reachable.""" +harmont_macros.scm. + +NOTE: the key format changed in v0.1 — base image is now folded into the +outer pre-image. Old cached snapshots are intentionally unreachable after +this change (a one-time cache invalidation).""" from __future__ import annotations @@ -74,7 +77,7 @@ def test_forever_policy_key_matches_scheme_formula(): inner = _sha256_hex("echo hi" + NUL + "") policy_res = "forever-" + inner expected = _sha256_hex( - "default" + NUL + "default" + NUL + "a" + NUL + "scratch" + NUL + policy_res + "default" + NUL + "default" + NUL + "a" + NUL + "" + NUL + "scratch" + NUL + policy_res ) assert out["nodes"][0]["step"]["cache"]["key"] == expected @@ -103,7 +106,7 @@ def test_ttl_policy_key_includes_bucket(): inner = _sha256_hex("x" + NUL + "") policy_res = "ttl-2-" + inner expected = _sha256_hex( - "default" + NUL + "default" + NUL + "a" + NUL + "scratch" + NUL + policy_res + "default" + NUL + "default" + NUL + "a" + NUL + "" + NUL + "scratch" + NUL + policy_res ) assert out["nodes"][0]["step"]["cache"]["key"] == expected @@ -136,7 +139,7 @@ def test_on_change_reads_file_contents(): inner = _sha256_hex(file_hash + NUL) policy_res = "sha-" + inner expected = _sha256_hex( - "default" + NUL + "default" + NUL + "a" + NUL + "scratch" + NUL + policy_res + "default" + NUL + "default" + NUL + "a" + NUL + "" + NUL + "scratch" + NUL + policy_res ) assert out["nodes"][0]["step"]["cache"]["key"] == expected @@ -273,7 +276,7 @@ def test_env_keys_are_sorted_and_picked_up(): inner = _sha256_hex("echo" + NUL + env_str) policy_res = "forever-" + inner expected = _sha256_hex( - "default" + NUL + "default" + NUL + "a" + NUL + "scratch" + NUL + policy_res + "default" + NUL + "default" + NUL + "a" + NUL + "" + NUL + "scratch" + NUL + policy_res ) assert out["nodes"][0]["step"]["cache"]["key"] == expected @@ -312,7 +315,7 @@ def test_parent_key_chains_through_resolved_cache_keys(): inner_b = _sha256_hex("y" + NUL + "") policy_res = "forever-" + inner_b expected_b = _sha256_hex( - "default" + NUL + "default" + NUL + "b" + NUL + parent_key + NUL + policy_res + "default" + NUL + "default" + NUL + "b" + NUL + "" + NUL + parent_key + NUL + policy_res ) assert out["nodes"][1]["step"]["cache"]["key"] == expected_b @@ -350,7 +353,7 @@ def test_compose_concatenates_subpolicies(): inner = _sha256_hex(sub1 + sub2) policy_res = "compose-" + inner expected = _sha256_hex( - "default" + NUL + "default" + NUL + "a" + NUL + "scratch" + NUL + policy_res + "default" + NUL + "default" + NUL + "a" + NUL + "" + NUL + "scratch" + NUL + policy_res ) assert out["nodes"][0]["step"]["cache"]["key"] == expected @@ -382,3 +385,42 @@ def test_parent_without_cache_is_planerror(): base_path=Path("/tmp"), # noqa: S108 env={}, ) + + +def _make_graph_with_image(cmd: str, image: str | None, policy: str) -> dict: + """Minimal graph for image-keying tests.""" + step: dict = {"key": "s", "cmd": cmd, "cache": {"policy": policy, "env_keys": []}} + if image is not None: + step["image"] = image + return _make_graph([{"step": step, "env": {}}]) + + +def _resolved_key(graph: dict) -> str: + return graph["nodes"][0]["step"]["cache"]["key"] + + +def _rk(graph: dict) -> str: + """Resolve pipeline keys and return the resolved cache key.""" + return _resolved_key( + resolve_pipeline_keys( + graph, + pipeline_org="o", + pipeline_slug="p", + now=0, + base_path=Path("."), + env={}, + ) + ) + + +def test_key_changes_with_base_image(): + # same command + policy, different base image => different key + g1 = _make_graph_with_image(cmd="cargo build", image="rust:1.79", policy="forever") + g2 = _make_graph_with_image(cmd="cargo build", image="rust:1.80", policy="forever") + assert _rk(g1) != _rk(g2) + + +def test_key_stable_for_same_image(): + g1 = _make_graph_with_image(cmd="cargo build", image="rust:1.80", policy="forever") + g2 = _make_graph_with_image(cmd="cargo build", image="rust:1.80", policy="forever") + assert _rk(g1) == _rk(g2) diff --git a/crates/hm-dsl-engine/harmont-ts/src/keygen.ts b/crates/hm-dsl-engine/harmont-ts/src/keygen.ts index 2867b3c5..b539d0cb 100644 --- a/crates/hm-dsl-engine/harmont-ts/src/keygen.ts +++ b/crates/hm-dsl-engine/harmont-ts/src/keygen.ts @@ -45,6 +45,7 @@ export function resolvePipelineCacheKeys( if (!cache || cache.policy === "none") continue; const cmd = (step.cmd as string) ?? ""; + const image = (step.image as string | null | undefined) || ""; const stepKey = step.key as string; const parentStepKey = parentKeyMap.get(stepKey); const parentResolved = lookupParent(parentStepKey, resolved); @@ -57,6 +58,8 @@ export function resolvePipelineCacheKeys( NUL + stepKey + NUL + + image + + NUL + parentResolved + NUL + policyRes, diff --git a/crates/hm-dsl-engine/harmont-ts/tests/keygen.test.ts b/crates/hm-dsl-engine/harmont-ts/tests/keygen.test.ts index 9cfa1c1c..a05f162e 100644 --- a/crates/hm-dsl-engine/harmont-ts/tests/keygen.test.ts +++ b/crates/hm-dsl-engine/harmont-ts/tests/keygen.test.ts @@ -144,15 +144,42 @@ describe("resolvePipelineCacheKeys", () => { const stepKey = ir.graph.nodes[0].step.key as string; const cmd = "echo hi"; + const image = ""; const policyRes = "forever-" + sha256(cmd + NUL + ""); const expected = sha256( - "myorg" + NUL + "myslug" + NUL + stepKey + NUL + "scratch" + NUL + policyRes, + "myorg" + NUL + "myslug" + NUL + stepKey + NUL + image + NUL + "scratch" + NUL + policyRes, ); const cache = ir.graph.nodes[0].step.cache as Record; expect(cache.key).toBe(expected); }); + it("key changes with different base image", () => { + const ir1 = pipeline([sh("cargo build", { label: "build", cache: forever(), image: "rust:1.79" })]); + const ir2 = pipeline([sh("cargo build", { label: "build", cache: forever(), image: "rust:1.80" })]); + const opts = makeOpts({ pipelineOrg: "o", pipelineSlug: "p" }); + + resolvePipelineCacheKeys(ir1.graph, opts); + resolvePipelineCacheKeys(ir2.graph, opts); + + const k1 = (ir1.graph.nodes[0].step.cache as Record).key; + const k2 = (ir2.graph.nodes[0].step.cache as Record).key; + expect(k1).not.toBe(k2); + }); + + it("key is stable for the same base image", () => { + const ir1 = pipeline([sh("cargo build", { label: "build", cache: forever(), image: "rust:1.80" })]); + const ir2 = pipeline([sh("cargo build", { label: "build", cache: forever(), image: "rust:1.80" })]); + const opts = makeOpts({ pipelineOrg: "o", pipelineSlug: "p" }); + + resolvePipelineCacheKeys(ir1.graph, opts); + resolvePipelineCacheKeys(ir2.graph, opts); + + const k1 = (ir1.graph.nodes[0].step.cache as Record).key; + const k2 = (ir2.graph.nodes[0].step.cache as Record).key; + expect(k1).toBe(k2); + }); + it("child step uses parent resolved key", () => { const base = sh("apt-get install", { label: "apt", cache: forever() }); const child = base.sh("make", { label: "build", cache: forever() }); diff --git a/crates/hm-exec/src/local/cache.rs b/crates/hm-exec/src/local/cache.rs index 3d609db7..f8be3ab0 100644 --- a/crates/hm-exec/src/local/cache.rs +++ b/crates/hm-exec/src/local/cache.rs @@ -32,8 +32,7 @@ pub(crate) fn stable_cache_tag(step: &CommandStep) -> Option { } let key = cache.key.as_deref()?; let safe = sanitize_for_tag(&step.key); - let short = &key[..key.len().min(16)]; - Some(format!("harmont-cache/{safe}:{short}")) + Some(format!("harmont-cache/{safe}:{key}")) } #[cfg(test)] @@ -64,15 +63,14 @@ mod tests { } #[test] - fn stable_cache_tag_for_cacheable_step() { + fn stable_cache_tag_uses_full_key() { let s = step(Some(Cache { policy: "ttl".into(), key: Some("0123456789abcdef0000".into()), })); - let tag = stable_cache_tag(&s); assert_eq!( - tag, - Some("harmont-cache/build:0123456789abcdef".to_string()) + stable_cache_tag(&s), + Some("harmont-cache/build:0123456789abcdef0000".to_string()) ); }