pkl read?("env:...") results are cached, but the environment is not part of the cache key
#1150
Replies: 2 comments
|
Disclosure: this report was researched and written by Claude Code (Opus 5), directed and reviewed by me before posting. Everything above comes from an actual run rather than from reading the source alone — the repro, the both-directions failure, the Flagging it in case you weigh AI-assisted reports differently, and happy to run further experiments against this if any claim needs checking. |
|
this isn't that important to me, but since I own the pkl library it sounds reasonable if you want to make a PR that adds env var tracking to that and then consume it in hk |
Uh oh!
There was an error while loading. Please reload this page.
Summary
hk caches the resolved pkl config, but the cache key does not include anything about the environment. A config that branches on
read?("env:...")therefore has its result frozen by whichever process evaluates it first, and every later run reuses that answer regardless of its own environment.The evaluation itself is correct. Only the invalidation is wrong —
HK_CACHE=0produces the right answer every time.Reproduction
hk 1.53.0, Windows 11, default
pklrbackend.It fails in both directions. In a fresh directory with the same config, seeding the cache the other way sticks just as hard:
Confirming that only caching is at fault:
This is not limited to
config explain. Withprofiles = List("agent")on a step, the wrongly-cached value decides whether the step actually runs in a realgit commit.Where it comes from
The config cache key is assembled from:
BASE_CACHE_KEYS(src/cache.rs:30-35) — the hk version only.pkl-backend:{HK_PKL_BACKEND}(src/config.rs:254).Fresh-file keys hash file bytes. Nothing observes environment variables, so a
read?("env:...")result is invisible to invalidation. pkl already reports what a module read viaanalyze imports-style dependency output, and hk already has the machinery to persist a dependency set —analyze_importscaches anImportAnalysisin a sidecar (src/config.rs:222-230). Env reads look like they could ride the same path: record the names read, then addname=valuepairs to the cache key.Why this is easy to hit and hard to notice
The symptom is silent and non-deterministic across machines. Nothing errors; steps simply run or do not run.
The pattern that triggers it is a natural one — gate expensive steps on whether an AI coding agent is driving the commit:
In a project doing exactly this, the main checkout and a second worktree disagreed while holding byte-identical
hk.pkland the same hk version. Whichever environment ran hk first after the lasthk.pkledit won, permanently. Agents silently skippedtypecheck/build/test, or human commits paid for all three, depending only on that accident of ordering.Interaction with #1034
#1034 proposes dropping the absolute path from the cache key so byte-identical configs share one entry. That is a good idea on its own terms, but it makes this bug strictly worse.
Today the path component accidentally limits the blast radius: each checkout freezes its own wrong answer. Fully content-address the key and every checkout, worktree, and CI job sharing that config also shares one frozen env-derived answer — so a single cold evaluation in the wrong environment can poison an entire fleet.
Note the shared
resolved-config.jsonslot already does something close to this whenhas_untracked_importsis false (src/config.rs:248-258). Configs amending apackage://URL take the path-keyed branch, which is why the divergence above is per-directory rather than global.Whatever key scheme wins, env reads need to be in it. Worth resolving these two together.
What I would expect
Either of:
Silently caching a value that depends on unrecorded inputs is the part that bites.
Workarounds
For anyone finding this later, both work:
HK_CACHE=0— correct, but gives up config caching entirely.HK_PROFILE/HK_PROFILESsit above the pkl layer in the precedence chain (hk config explain profiles) and are read at runtime, outside the cache. SettingHK_PROFILEfrom the environment — via mise's[env]with a template, for instance — resolves per run and is immune to this. That is what the project above ended up doing.The second is arguably better config design anyway, but the failure mode still seems worth fixing: the pkl form looks correct, is documented-adjacent, and fails silently.
Environment
HK_PKL_BACKENDunset (defaultpklr). Not tested against thepklCLI backend.All reactions