fix(plugin): regenerate local plugin create_files when their content changes#2867
Open
mikeland73 wants to merge 3 commits into
Open
fix(plugin): regenerate local plugin create_files when their content changes#2867mikeland73 wants to merge 3 commits into
mikeland73 wants to merge 3 commits into
Conversation
…changes Local plugins' files in the virtenv were never recreated after editing the source files referenced by create_files. Devbox decides whether to recompute the environment (and rewrite plugin files) by comparing a state hash that is derived, in part, from each included plugin config's hash. That hash only covered plugin.json, not the content files that create_files references, so editing a local plugin's source file left the state hash unchanged and the stale virtenv copy in place. Override Config.Hash() to fold the contents of create_files source files into the hash for local plugins. Built-in plugin files are embedded in the binary (covered by the Devbox version) and remote plugins are addressed by an immutable ref, so those are left to the config-file hash alone. Fixes #2755
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes cache invalidation for local (path:) plugins so that edits to files referenced by a plugin’s create_files trigger environment regeneration and the .devbox/virtenv/... copies get rewritten, addressing #2755.
Changes:
- Adds
(*plugin.Config).Hash()to incorporate localcreate_filessource file contents into the plugin config hash. - Adds unit tests ensuring the local plugin hash changes when a referenced
create_filesfile changes, and remains stable otherwise.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
internal/plugin/plugin.go |
Introduces Config.Hash() override to include local create_files content in the cache key used for state up-to-date checks. |
internal/plugin/local_test.go |
Adds unit tests validating the new hashing behavior for local plugins. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+88
to
+99
| configHash, err := c.ConfigFile.Hash() | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| local, ok := c.Source.(*LocalPlugin) | ||
| if !ok { | ||
| return configHash, nil | ||
| } | ||
|
|
||
| buf := bytes.Buffer{} | ||
| buf.WriteString(configHash) |
Comment on lines
+110
to
+116
| for _, contentPath := range contentPaths { | ||
| content, err := local.FileContent(contentPath) | ||
| if err != nil { | ||
| return "", errors.WithStack(err) | ||
| } | ||
| buf.Write(content) | ||
| } |
…lugins Address review feedback on the local plugin cache-invalidation fix: - configfile.ConfigFile.Hash() only covers ConfigFile fields, so the previous seed omitted plugin-only fields (create_files destinations, version, readme, __remove_trigger_package). Fold these into the hash explicitly so changing them also invalidates Devbox's cached state. - Hash each create_files source file independently, keyed by its path, instead of concatenating raw bytes. Concatenation is ambiguous (different file/content splits can yield identical byte streams) and buffered every file at once.
Collaborator
Author
|
The one red check (
I re-triggered CI once already (the flake recurred only on the 2.19.2 job). I don't have permission to re-run individual jobs via the API, so a maintainer re-run of that single job should clear it. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2755.
When developing a local plugin (included via
path:), editing a file referenced by the plugin'screate_filesdoes not update the copy in.devbox/virtenv/.... Even deleting the generated file and re-entering the shell leaves it missing/stale, which makes local plugin development confusing.Root cause
Devbox skips recomputing the environment (and rewriting plugin files) when its state hash is unchanged (
internal/lock/statehash.go). That state hash is derived in part fromDevbox.ConfigHash(), which hashes each included plugin config viaplugin.Config.Hash()— and that resolved to the embeddedconfigfile.ConfigFile.Hash(), which only hashesplugin.json.The files referenced by
create_filesare read from local disk separately and are not part of any hash. So editing a local plugin's source file (e.g.plugin/test/test.txt) changes nothing the state hash tracks:devbox.json— unchangedplugin.json— unchangeddevbox.lock, nix manifest, print-dev-env cache — unchangedisStateUpToDatetherefore returnstrue,CreateFilesForConfigis never called, and the stale virtenv copy stays in place.(Note:
shouldCreateFilealready returnstruefor files under.devbox/, so once recomputation is triggered the file is rewritten correctly — the gap was purely in cache invalidation.)Fix
Add a
Hash()method onplugin.Configthat shadows the embeddedConfigFile.Hash(). For local plugins, it folds the contents of the files referenced bycreate_filesinto the hash (sorted for determinism), so editing them invalidates Devbox's cached state and regenerates the virtenv files on the nextdevbox shell/run.This is scoped to local plugins on purpose:
DevboxVersionfield in the state hash.The only caller of
plugin.Config.Hash()isDevbox.ConfigHash(), so the override is tightly scoped to the cache-invalidation path.How was it tested?
go build ./internal/...,go vet ./internal/plugin/, andgofumpt— all clean.internal/plugin/local_test.go:TestLocalPluginHashIncludesCreateFilesContent— hash changes when acreate_filessource file is edited (plugin.json untouched). Fails before this change, passes after.TestLocalPluginHashStableWithoutContentChange— hash is stable across calls when nothing changes.cc @tomtaylor (issue reporter) — thanks for the clear repro repo, it made this easy to pin down.
Generated by Claude Code