feat(version): warn when project charts drift from the talm binary#216
feat(version): warn when project charts drift from the talm binary#216Aleksei Sviridkin (lexfrei) wants to merge 17 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds Chart.yaml normalization, embedded talm library export, and deterministic hashing; implements vendored-vs-embedded chart drift checks and preset lock pinning; wires checks into root command with a --strict-charts flag and Config.StrictCharts to escalate warnings to hard errors; updates init to write/prune/update preset locks and adds tests/docs. ChangesChart drift detection and strict-mode enforcement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a content-drift detection mechanism to verify that the project's vendored charts/talm/ library matches the copy embedded in the talm binary, ignoring pure version stamp changes. It also adds a strict mode (--strict-charts or strictCharts: true) to treat drift as a hard error. The review feedback highlights two important issues: a regular expression in charts/charts.go that could incorrectly match nested keys or comments in Chart.yaml leading to formatting errors, and a version prefix check in main.go that might prematurely skip the drift check on non-standard release builds.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Chart.yaml. Both are rewritten to a %s placeholder so the init flow can | ||
| // substitute the real project name/version, and so the drift check can | ||
| // compare two chart trees without a version stamp counting as content. | ||
| var chartMetaRegex = regexp.MustCompile(`(name|version): \S+`) |
There was a problem hiding this comment.
The chartMetaRegex regular expression matches any occurrence of name: or version: globally in the Chart.yaml file.
If the Chart.yaml contains nested fields with these keys (for example, under dependencies, maintainers, or annotations), or if these words appear in descriptions or comments, they will also be replaced with %s placeholders. When talm init subsequently formats this content using fmt.Sprintf with only two arguments (project name and version), the extra placeholders will result in formatting errors or %!s(MISSING) being written to the output.
To prevent this, anchor the regular expression to the start of the line using the multi-line flag (?m)^ so that only top-level name: and version: fields are matched.
| var chartMetaRegex = regexp.MustCompile(`(name|version): \S+`) | |
| var chartMetaRegex = regexp.MustCompile(`(?m)^(name|version): \S+`) |
There was a problem hiding this comment.
Anchored in 6af1a7b: (?m)^(name|version): \S+. TestNormalizeChartMeta_PreservesNestedNameVersion pins that nested dependencies/maintainers entries survive and exactly two placeholders remain for the init-time template.
| version, ok := strings.CutPrefix(Version, "v") | ||
| if !ok { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
The drift check is silently skipped if the binary Version does not start with a v prefix (e.g., if a package manager or custom build script injects a version like 0.30.0 instead of v0.30.0).
Instead of using strings.CutPrefix and skipping when ok is false, consider explicitly checking if Version == "dev" to identify development builds, and then trimming the v prefix if present.
| version, ok := strings.CutPrefix(Version, "v") | |
| if !ok { | |
| return nil | |
| } | |
| if Version == "dev" { | |
| return nil | |
| } | |
| version := strings.TrimPrefix(Version, "v") |
There was a problem hiding this comment.
The current code accepts both forms: releaseVersion in main.go strips an optional v prefix and gates only on the dev sentinel, so goreleaser's 0.30.0 and the Makefile's v0.30.0 are both recognized (pinned by TestReleaseVersion). a44d621 additionally rejects git-describe and -dirty shapes so WIP builds never pose as releases.
Expose the binary's built-in talm library chart as a normalized path->content map (TalmLibraryFiles) plus a deterministic, order- independent digest over a chart tree (HashChartFiles). Chart.yaml name/version lines are folded to %s placeholders through a shared NormalizeChartMeta helper — also adopted by PresetFiles — so a version stamp is not counted as content. This is the comparison primitive for detecting when a project's vendored charts/talm/ has drifted from the binary that renders it: equal digests mean identical library bytes, independent of the stamped version. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
CheckChartDrift compares a project's vendored charts/talm/ against the binary's built-in copy via the chart-tree digest. The vendored tree is read through an os.Root scoped to charts/talm/, confining traversal to that subtree so a symlink inside it cannot redirect a read elsewhere. The match is by content, with the Chart.yaml version stamp normalized away on both sides: a binary version bump that left the library byte- identical is not reported as drift. A project with no vendored library stays silent rather than erroring, and a charts/talm that is a file rather than a directory surfaces as an error (callers degrade it to a non-fatal warning) instead of a crash. The drift message points operators at `talm init --update --preset <preset>` — the bare form cannot resolve the preset from an init'd project's Chart.yaml and would not re-vendor. Config gains an opt-in StrictCharts field (Chart.yaml strictCharts) for turning drift from a warning into a hard error; absent keeps today's behavior. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
talm renders from the project's vendored charts/talm/, never the binary's built-in charts, so upgrading the binary leaves the library frozen at the version that last ran init — drift that was previously invisible. On every config-loading command a release build now compares the two and, by default, prints a non-fatal warning to stderr; stdout and the exit code are unchanged. A project (strictCharts: true in Chart.yaml) or an operator (--strict-charts) can promote the warning to a hard error raised before the command body runs. The build version is interpreted through releaseVersion, which accepts both release forms — goreleaser injects it without a leading "v" (0.30.0), the Makefile's `git describe --tags` keeps it (v0.30.0) — and treats only "dev"/empty as a non-release. Gating on the "v" prefix alone would disable the check on every downloaded release and stamp the dev sentinel 0.1.0 into the vendored charts at init time; releaseVersion is used in both places. The decision logic is factored into a pure evaluateChartDrift for direct test coverage of the warn/strict/silent branches. The --strict-charts usage string carries no backticks, so pflag does not misrender the bool flag as taking a value. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add a README section explaining the embed -> vendor -> render model: how init vendors charts/talm/, why a binary upgrade does not touch it, the `talm init --update --preset <preset>` re-sync workflow (the preset is required because an init'd Chart.yaml does not record it), and the content-based drift warning with its strictCharts / --strict-charts enforcement, including that strict mode also blocks read-only commands. Rework manual-test-plan B5 into a runnable drift-detection script: default warning is stderr-only with the exit code unchanged, the re-vendor step that clears it, strict mode short-circuiting before the command body, and the no-false-alarm cases (version-only difference, dev build, freshly synced project) that must stay silent. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/chart-drift-detection
branch
from
21065e3 to
ad56f0e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/charts.go`:
- Around line 31-35: The regex in chartMetaRegex currently matches any
`name:`/`version:` anywhere, including indented dependency entries; update
chartMetaRegex to only match top-level keys by anchoring to the start of a line
and enabling multiline mode (e.g., use a pattern like
`(?m)^(name|version):\s+\S+`) so only top-level Chart.yaml `name` and `version`
lines are normalized; update the declaration of chartMetaRegex accordingly and
run tests that use the rewrite/drift logic to confirm dependency entries are no
longer rewritten.
In `@main.go`:
- Around line 140-145: The help text for the Bool flag created via
cmd.PersistentFlags().BoolVar(&strictChartsFlag, "strict-charts", ...) points
users to "talm init --update" which is inconsistent with the actual remediation
that requires specifying a preset; update the usage string to reference the
correct recovery command (for example "run 'talm init --update --preset
<preset>'" or otherwise instruct to supply --preset <preset>) so the guidance
aligns with the rest of the codebase; modify the third argument (the usage
string) passed to BoolVar where strictChartsFlag is declared to contain the
corrected remediation text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b52ff39-6fbd-43e1-893d-d9903a898ef7
⛔ Files ignored due to path filters (1)
pkg/generated/presets.gois excluded by!**/generated/**
📒 Files selected for processing (9)
README.mdcharts/charts.gocharts/library_test.godocs/manual-test-plan.mdmain.gomain_test.gopkg/commands/chart_drift.gopkg/commands/chart_drift_test.gopkg/commands/root.go
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/charts.go`:
- Around line 35-47: The current NormalizeChartMeta and chartMetaRegex replace
any occurrences of "name:"/ "version:" including indented/nested ones; update
chartMetaRegex to only match top-level fields by using a multiline-anchored
pattern (e.g. use ^ in multiline mode) so it matches only lines that start with
name/version with no indentation, then keep NormalizeChartMeta (function) using
that regex to ReplaceAllString with "$1: %s"; reference chartMetaRegex,
NormalizeChartMeta, and chartYamlName when making the change.
In `@docs/manual-test-plan.md`:
- Around line 239-241: The drift-clear step hardcodes "--preset generic" in the
"talm init --update --preset generic --force" and should use the same preset as
the baseline project created earlier; change the hardcoded preset to the
consistent preset used in setup (e.g., replace "--preset generic" with "--preset
cozystack" or a reusable variable like "--preset ${PRESET}") and ensure the same
preset value is used in the "talm template -f nodes/node0.yaml --offline" flow
so the "drift cleared" assertion is valid.
In `@main.go`:
- Around line 140-145: Update the --strict-charts flag help text set in
cmd.PersistentFlags().BoolVar(&strictChartsFlag, "strict-charts", ...) so it
references the correct remediation (--preset <preset>) instead of suggesting
"talm init --update"; modify the message string to mention using the --preset
<preset> option to re-sync/remediate vendored charts/talm to match the binary,
keeping the rest of the explanatory text intact for context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc549dde-ff97-422c-8522-52bfc581ee2f
⛔ Files ignored due to path filters (1)
pkg/generated/presets.gois excluded by!**/generated/**
📒 Files selected for processing (9)
README.mdcharts/charts.gocharts/library_test.godocs/manual-test-plan.mdmain.gomain_test.gopkg/commands/chart_drift.gopkg/commands/chart_drift_test.gopkg/commands/root.go
The vendored library check (charts/talm/) only covers code the operator never edits. The preset templates (templates/) ARE operator-editable, so content-comparing them would warn on every legitimate customization. Pin the pristine preset hash into .talm-preset.lock at init / init --update time and compare the binary's current preset hash against that baseline — never against the edited templates/ — so operator edits are never reported as drift. A newer binary shipping changed preset defaults no longer matches the baseline and warns (or hard-errors under --strict-charts), pointing at `talm init --update --preset <preset>`. init --update advances the baseline, clearing the warning even when individual file diffs are declined. Projects with no lock (generated before pinning) stay silent. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/commands/init.go (1)
1335-1346:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate inferred preset before any
init --updatewrites.When
presetNameis inferred from Chart.yaml (Line 1335) and not embedded in the current binary,updateTalmLibraryChartcan still mutatecharts/talmand only fail later at Line 1432 inWritePresetLock(unknown preset). That returns an error after partial writes.Fail fast by validating inferred
presetNameagainstgenerated.AvailablePresets()immediately afterreadChartYamlPreset()and before Step 1 writes.Suggested fix
@@ } else { // Try to read from Chart.yaml var err error presetName, err = readChartYamlPreset() if err != nil { @@ return err } + + availablePresets, err := generated.AvailablePresets() + if err != nil { + return errors.Wrap(err, "failed to get available presets") + } + if !isValidPreset(presetName, availablePresets) { + return errors.WithHint( + errors.Newf("preset %q from Chart.yaml is not embedded in this talm binary. Valid presets are: %v", presetName, availablePresets), + "pass --preset with one of the embedded presets, or use a talm version that ships your project preset", + ) + } }Also applies to: 1428-1434
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/commands/init.go` around lines 1335 - 1346, The code currently infers presetName via readChartYamlPreset() but does not validate it before performing writes in updateTalmLibraryChart, which can produce partial changes and later fail in WritePresetLock; after readChartYamlPreset() returns (where presetName is set), immediately check that presetName exists in generated.AvailablePresets() and return an error if not found so init --update fails fast; apply the same validation before any code path that calls updateTalmLibraryChart or reaches WritePresetLock to avoid partial writes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pkg/commands/init.go`:
- Around line 1335-1346: The code currently infers presetName via
readChartYamlPreset() but does not validate it before performing writes in
updateTalmLibraryChart, which can produce partial changes and later fail in
WritePresetLock; after readChartYamlPreset() returns (where presetName is set),
immediately check that presetName exists in generated.AvailablePresets() and
return an error if not found so init --update fails fast; apply the same
validation before any code path that calls updateTalmLibraryChart or reaches
WritePresetLock to avoid partial writes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65a2d9f2-8698-482e-b3cc-1e50a3c37067
📒 Files selected for processing (7)
README.mddocs/manual-test-plan.mdmain.gomain_test.gopkg/commands/chart_drift.gopkg/commands/chart_drift_test.gopkg/commands/init.go
✅ Files skipped from review due to trivial changes (1)
- docs/manual-test-plan.md
🚧 Files skipped from review as they are similar to previous changes (1)
- main_test.go
myasnikovdaniil
left a comment
myasnikovdaniil
left a comment
There was a problem hiding this comment.
NOT LGTM — the implementation and tests are solid, but one manual-test-plan step re-vendors the wrong preset and the assertion it backs validates the wrong content.
Business context: a talm init project vendors the library chart and the preset from the binary; both go stale silently on a binary upgrade. This PR surfaces that drift (content digest for charts/talm/, a pinned baseline hash in .talm-preset.lock for the preset) as a non-fatal stderr WARN, with opt-in --strict-charts / strictCharts: true escalation to exit 1.
Blocker (inline at docs/manual-test-plan.md:239): B5's --preset generic overwrites the cozystack project's preset and makes the drift-clear assertion meaningless.
Non-blocking (inline): unanchored chartMetaRegex (charts/charts.go:35); --strict-charts help omits --preset (main.go:144); no test covers a malformed .talm-preset.lock.
Ran go test ./... locally — green, including the 11 new drift tests.
| Expected (default): a single line on **stderr** — `WARN: project's vendored charts/talm/ library differs from the copy built into talm <version>; run talm init --update --preset <preset> to re-sync ...` — while stdout (the rendered config) and the exit code are unchanged. Clear it and re-confirm silence: | ||
|
|
||
| ```bash | ||
| talm init --update --preset generic --force |
There was a problem hiding this comment.
This runs --preset generic, but the project under test is provisioned with --preset cozystack. init --update re-vendors charts/talm/ (clearing library drift regardless of preset) and rewrites the preset templates/ for the named preset — so --preset generic --force writes the generic preset's files over the cozystack project and pins a generic baseline into .talm-preset.lock. That corrupts the project's preset (the cozystack sysctl/etcd/DRBD opinions B9 documents) and makes the following OK: drift cleared check meaningless, since it re-vendored a different preset than the one whose drift was simulated. Use --preset cozystack --force (or a consistent --preset <your-preset> across the simulate/clear pair).
There was a problem hiding this comment.
Fixed in 9669a55 — the clear step now uses --preset cozystack, so the drift-clear assertion validates the same preset the scenario simulated and B9's cozystack opinions survive.
| // Chart.yaml. Both are rewritten to a %s placeholder so the init flow can | ||
| // substitute the real project name/version, and so the drift check can | ||
| // compare two chart trees without a version stamp counting as content. | ||
| var chartMetaRegex = regexp.MustCompile(`(name|version): \S+`) |
There was a problem hiding this comment.
chartMetaRegex is unanchored. Safe today (no nested lowercase name:/version: in the embedded Chart.yamls), but a future dependencies: block or maintainers: [{name: …}] would be rewritten too — masking real dependency-version drift, and since NormalizeChartMeta doubles as the fmt.Appendf template in init.go, injecting %!s(MISSING) into the written Chart.yaml. Anchor to (?m)^(name|version): \S+.
There was a problem hiding this comment.
Fixed in 6af1a7b with exactly that anchor; TestNormalizeChartMeta_PreservesNestedNameVersion pins both failure modes you named (masked dependency drift and %!s(MISSING) injection through the init-time fmt template).
| // first backtick-quoted word as the flag's value-placeholder name, which | ||
| // on a bool flag misrenders --help as `--strict-charts talm init --update` | ||
| // (as if it took an argument). | ||
| cmd.PersistentFlags().BoolVar(&strictChartsFlag, "strict-charts", false, "fail if the project's vendored charts/talm/ differs from the talm binary's built-in copy (run talm init --update to re-sync)") |
There was a problem hiding this comment.
Help says run talm init --update to re-sync, but bare talm init --update errors with "preset not found in Chart.yaml dependencies" on an init'd project. The WARN/strict messages already carry --preset; align this help with talm init --update --preset <preset>.
There was a problem hiding this comment.
Fixed in 7d564d6 — the flag help now carries --preset like the WARN and strict messages.
The unanchored pattern also rewrote nested name:/version: lines (dependencies, maintainers), which would mask real dependency drift in the content comparison and inject extra %s verbs into the two-argument fmt template the init flow writes Chart.yaml with. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Bare 'talm init --update' errors on an init'd project (the preset is not recorded in Chart.yaml dependencies), so the help now carries the --preset argument like every other drift remediation hint. Also name the preset baseline, which the flag has gated alongside the vendored library since the .talm-preset.lock check landed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…preset The B5 drift-clear step re-vendored the generic preset over a project bootstrapped with cozystack, overwriting its templates and pinning a generic baseline into .talm-preset.lock — which both corrupts the project the later sections rely on and re-vendors a different preset than the one whose drift was simulated, voiding the assertion. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Cover the three corrupt-lock shapes — unparseable YAML, missing preset/presetHash fields, and a lock naming a preset the binary does not ship. All must surface as an error with drift=false so the caller downgrades to a non-fatal warning instead of fabricating a verdict. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
When the preset comes from Chart.yaml rather than --preset, an unknown name previously surfaced only in WritePresetLock — after Step 1 had already rewritten charts/talm/, leaving the project half-updated. Validate against the embedded presets up front, mirroring the --preset flag path, so the failure happens before any file is touched. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
A Windows clone with core.autocrlf=true (the Git for Windows default) materializes the vendored charts/talm/ with CRLF while the embedded library is LF-only. The byte-level comparison then reported drift on every command for a content-identical tree, and strictCharts: true hard-failed it. Normalize CRLF to LF at the vendored-read boundary. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…elines decideDrift downgraded every check failure to a non-fatal warning even with strictCharts enforcement on, and a missing baseline passed even more quietly: deleting .talm-preset.lock or charts/talm/ was the cheapest bypass of the check. Under strict, a corrupted or unreadable baseline now aborts with a repair hint, and a missing one aborts with a pin-the-baseline hint (via an ErrNoBaseline sentinel the checks return); without strict the contracts are unchanged — failures degrade to a neutral "could not check drift" warning and a missing baseline stays silent, so pre-pinning projects are not nagged. The strict drift error no longer carries the warning's "(or ignore if this is intentional)" suffix, which contradicted the refusal to run. Touched not-exist checks moved to the unwrap-tolerant errors.Is idiom. Documented in README and the manual test plan. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
… paths An extraneous file in charts/talm/ (.DS_Store, editor backup, a file a newer library release dropped) kept the content-drift warning — or the strictCharts hard error — permanently on: the update flow only wrote embedded files and never removed leftovers, so the documented remediation could not clear the drift it pointed at. init --update now prunes files the embedded library does not ship, including directories the prune leaves empty (the tree is talm-owned and never operator-edited), reporting each removal. The drift message carries a capped sample of differing paths (modified / extra / missing), so the cause is locatable without a manual tree diff. The LF-only invariant of the embedded tree — which the vendored-side CRLF normalization relies on — is now pinned by a test instead of resting silently on .gitattributes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
git describe --tags on a non-tag commit yields v0.29.0-5-gabc1234, and on a dirty tree at an exact tag --dirty yields v0.29.0-dirty; both shapes were classified as releases, so contributor builds — possibly with edited embedded charts — raised false chart-drift warnings in any real project and hard-failed strictCharts: true ones. releaseVersion now rejects both shapes, while the Makefile describes with --dirty so `talm --version` still carries full provenance for bug reports. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…rce OR Config.StrictCharts had no coverage from the Chart.yaml side: a yaml-tag typo would have silently disabled the committed team/CI enforcement while the per-invocation flag kept working. Pin the parse and the OR in surfaceChartDrift — either source alone blocks on drift, neither only warns. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The init flow writes the lock and the preset-drift section asks to commit it, but the tree diagram omitted it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…g diff The B5a clear step recommends --force on a stand where an operator edit was made two steps earlier; the interactive-diff preservation promise holds only without --force, so the step now says so and points real projects at the interactive form. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
myasnikovdaniil All four points are addressed. The B5 blocker: the clear step now re-vendors the project's own preset (9669a55). Non-blocking: the normalizer is anchored to top-level keys (6af1a7b), the flag help carries A few adjacent gaps surfaced while testing the fixes and are also in: CRLF clones no longer false-positive (e28d34e), strict mode now blocks on an unverifiable or missing baseline instead of silently passing (3dc254a), |
|
CodeRabbit (@coderabbitai) The out-of-diff finding on |
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
2 participants
What
A
talm initproject vendors two things from the binary, and both silently fall behind when you upgradetalm:charts/talm/— the library chart. Render commands read this local copy, never the binary's embedded charts, so a binary upgrade leaves it frozen at whatever version last raninit. You render with stale chart logic and nothing tells you.templates/— the preset (the machine-config opinions: sysctls, etcd args, the cozystack defaults). Same staleness on a binary upgrade, but with a twist: you are expected to edit these, so they cannot be content-compared without flagging every legitimate customization.Previously both were invisible — operators had to eyeball
talm versionagainst the project. This surfaces both, each with the comparison that fits what it is.How
Library (
charts/talm/) — content comparison. On every config-loading command, compare the vendored tree against the binary's built-in copy by content digest. TheChart.yamlversion stamp is normalized away on both sides, so a binary bump that left the library byte-identical is never flagged — no false positives on version-only differences.Preset (
templates/) — pinned baseline. Because the preset is operator-editable, comparing live content would warn on every customization. Insteadtalm init(andinit --update) pin the hash of the pristine preset — as shipped in the binary — into.talm-preset.lock:A release build compares the binary's current preset hash against that pinned baseline — never against your edited
templates/. So operator customizations are never drift; a newer binary shipping changed preset defaults is.init --updaterewrites the baseline, clearing the warning even when you decline individual file diffs to keep your edits. Projects with no lock (generated before pinning) stay silent.Shared contract for both:
WARN:on stderr pointing attalm init --update --preset <preset>. Rendered output (stdout) and the exit code are unchanged.strictCharts: trueinChart.yaml(committed, so a team/CI inherits it) or--strict-chartsturns either mismatch into a hard error (exit 1), raised before the command body runs.dev/source builds (embedded charts are a moving target the developer controls), projects with nothing vendored / no lock, and matching content.The remediation carries
--presetbecausetalm init --updateresolves the preset fromChart.yaml, which an init'd project does not record — pass the preset you rantalm initwith.Testing
templates/false-positive guard, the no-lock silent case, and the warn-vs-fail decision for both checks.-ldflagsbuilds: matching → silent; older-vendored library under a newer binary with identical content → silent; library content edit → warn (exit 0);initwrites.talm-preset.lock; matching preset → silent; operator edit totemplates/→ silent (no false positive); corrupted preset baseline → warn; strict via flag and viaChart.yaml→ exit 1; dev build → silent.Docs
talm init --updateworkflow, and a dedicated preset-drift section.docs/manual-test-plan.md: B5 (library drift) and B5a (preset drift, incl. the false-positive guard) as runnable scripts.Supersedes #214: same goal, but that approach compared the project's top-level
Chart.yamlversion:(the wrong file, owned by the operator) and matched on version number rather than content.Summary by CodeRabbit
New Features
.talm-preset.lock) and enhancedinit --update --presetflow to re-sync vendored charts, prune extraneous files, and offer interactive diff/merge.Documentation