Fix plugin cache policy handling, add producer-derived TTLs, declare generated grammar as a build output - #1068
Conversation
…s from produced values Three independent fixes, split out of the aws MFA work so they can land on their own. Plugins install during finishInit, before the @cache root decorator is applied, so a cache accessor bound to whatever store existed at install time silently ignored the policy - every plugin cacheTtl fell through to a no-op store under @cache=memory or @cache=disabled. The accessor now reads the store per call and the final store is re-propagated after the policy resolves. getOrSet needed a TTL up front, but for anything whose lifetime the source decides - an STS session, an OAuth token, a lease - that is only known once the producer has run. The ttl argument now also accepts a callback given the produced value, called once and only when the producer produced. Returning zero or less skips the write, and that skip does not share across processes, which the docs now say plainly. Existing callers pass a literal and are unaffected. The parser's generated src/grammar.js was not a declared build output, so a turbo cache hit restored dist/ without it and anything importing the parser from source failed to resolve. grammar.peggy is now an input too, since the shared src/** glob never covered it.
|
The changes in this PR will be included in the next version bump.
|
📦 Bundle size
dist/ only; native binaries are versioned separately and not counted here. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The parser build cache still omits an environment input that changes the generated grammar, so Turbo can restore the wrong build variant.
Reviewed changes in 405dc1fa, covering plugin cache-store propagation, producer-derived TTL handling across every store, tests, and parser build-cache declarations.
- Plugin cache policy: Accessors now resolve their backing store lazily, and
EnvGraphpropagates the store selected by the final@cachepolicy. - Producer-derived TTLs:
getOrSetaccepts value-based TTL callbacks, normalizes plugin duration strings, skips non-positive writes, and retains producer coalescing behavior. - Build outputs: The parser task now tracks
grammar.peggyand restores generatedsrc/grammar.js, with nested Turbo configs covered by JSONC linting.
⚠️ Parser build cache ignores trace mode
peggy.config.cjs uses the documented PEGGY_TRACE variable to change src/grammar.js, but the package build task does not include that variable in its environment inputs. Turbo therefore treats traced and untraced builds as interchangeable and can restore the wrong generated parser.
Technical details
# Include the grammar trace mode in the build hash
## Affected sites
- `packages/env-spec-parser/turbo.json:5` - the build task has no `env` entry
- `packages/env-spec-parser/peggy.config.cjs:5` - `process.env.PEGGY_TRACE` changes Peggy's `trace` option
## Required outcome
- Builds with `PEGGY_TRACE` enabled and disabled must have different Turbo cache keys.
## Suggested approach
- Add `"env": ["PEGGY_TRACE"]` to the package build task.
## Verification
- `turbo run build --filter=@env-spec/parser --dry=json` reports hash `e3ec243272b9b092` both with `PEGGY_TRACE=1` and with it unset.
- Direct generation produced different grammar hashes: `b7b456fb...` with tracing and `fbf21eeb...` without tracing.
@env-spec/parser
varlock
@varlock/native-helper-darwin
@varlock/native-helper-linux-arm64
@varlock/native-helper-linux-x64
@varlock/native-helper-win32-x64
@varlock/aws-sigv4-plugin
commit: |
peggy.config.cjs turns on tracing from PEGGY_TRACE, which changes the generated grammar, but the build task did not declare it. That was harmless while the grammar was regenerated by executing the task; declaring src/grammar.js as a restorable output made it a hazard, since a traced and an untraced build shared a cache key and either could be restored over the other. Verified: the task hash was identical with and without the variable and now differs, and the two grammars really are different files.
|
Confirmed and fixed in 1609aa7 by adding Verified both directions rather than taking the report on faith: the task hash was byte-identical with and without the variable ( Worth noting this hazard was introduced by this PR: while |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focusing on the parser build-cache correction in 1609aa7c.
- Declared trace-mode input: Added
PEGGY_TRACEto the parser build task's environment inputs, so traced and untraced generated grammars no longer share a Turbo cache key. - Verified effective hashing: Confirmed Turbo resolves the environment declaration and produces distinct build hashes with
PEGGY_TRACEunset and enabled.
azure/gpt-5.6-sol ｜ 𝕏
Rebuilt on current main with the non-AWS changes removed - the @cache policy fix, the getOrSet TTL callback, and the parser grammar build output now live in #1068. The duplicate varlock changeset went with them. DEPENDS ON #1068. The session cache passes getOrSet a TTL callback, which that PR adds, so this does not typecheck until #1068 is in main: plugin.ts: Argument of type '(session: StoredSession) => number' is not assignable to parameter of type 'string | number' That is the only outstanding error. Rebase on main once #1068 lands. Parked pending a real-world test - see #1047 for the known open gap in cross-process coordination on the failure path.
1 participant
Three independent fixes, split out of #1044 so they can land without waiting on the AWS MFA feature. None of them are AWS-specific.
Plugin caches ignored the
@cachepolicyPlugins install during
finishInit, before the@cacheroot decorator is applied infinishLoad. A cache accessor bound to whatever store existed at install time therefore never saw the policy, so under@cache=memoryor@cache=disabledevery plugin'scacheTtlfell through to a no-op store. That affects all 11 plugins that cache, not just one.The accessor now looks up the backing store per call, and the final store is re-propagated to plugins once the policy resolves.
Covered by a new test using a fixture plugin that resolves through
plugin.cache: two items sharing a key must produce one value. Without the fix the producer runs twice.getOrSetcan derive its TTL from the produced valuegetOrSettook its TTL up front, but for anything whose lifetime the source decides (an STS session, an OAuth token, a lease) that is only known once the producer has run. Callers had to guess a lower bound and correct it afterwards, leaving a window where the entry outlived what it described.The
ttlargument now also accepts a callback receiving the produced value, called once and only when the producer actually produced. Returning zero or less skips the write. All existing callers pass a literal and are unaffected.Note the guarantee is scoped: cross-process sharing covers persisted results only. A skipped or failed write leaves later processes to run the producer themselves, so a caller guarding single-use work needs a positive TTL. The docstring says so.
Generated parser grammar was not a declared build output
packages/env-spec-parser/src/grammar.jsis produced by peggy and gitignored, but the build task only declareddist/**. On a cache hit turbo restoreddist/without it, so a fresh checkout that hit the cache had no grammar and anything importing the parser from source failed withERR_MODULE_NOT_FOUND. This surfaced whenever a downstream test task cache-missed while the parser build cache-hit.Also adds
grammar.peggyto the build inputs. It sits at the package root, so the sharedsrc/**glob never covered it and editing the grammar would not have invalidated the cache.The eslint jsonc glob now matches package-level
turbo.jsonfiles, not just the root one, so they can carry comments the way the root config does.