feat(version): warn when project charts drift from the talm binary version#214
feat(version): warn when project charts drift from the talm binary version#214myasnikovdaniil wants to merge 1 commit into
Conversation
…rsion talm vendors its preset and library charts into the project at `talm init` time; render commands (template/apply/upgrade) read that local copy, never the binary's embedded charts. Upgrading the talm binary therefore leaves charts/talm/ frozen at the version that last ran init, and the drift was invisible — operators had to eyeball `talm version` against Chart.yaml. Compare the binary version against the `version:` stamped in the project's Chart.yaml on every config-loading command. On mismatch, warn on stderr by default (output and exit code unchanged) and point at `talm init --update`. Teams can opt into enforcement via `strictVersion: true` in Chart.yaml or the `--strict-version` flag, turning drift into a hard error. The check stays silent for dev builds, the 0.1.0 dev sentinel, empty or unparseable versions, and matching versions, so it never raises a false alarm. The strictVersion field is optional; absent means today's behavior. Documents the vendoring model in the README and extends manual-test-plan B5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
📝 WalkthroughWalkthroughThis PR introduces chart drift detection for ChangesChart Drift Detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to detect and handle drift between the talm binary version and the project's vendored chart version. It adds a drift warning on stderr when they differ, and a strict mode (via the --strict-version flag or strictVersion: true in Chart.yaml) that turns this warning into a hard error. The implementation includes documentation, CLI flag integration, version comparison logic using semver, and unit tests. There are no review comments to address.
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.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/manual-test-plan.md (1)
225-238: 💤 Low valueOptional: Add language hint to fenced code block at line 231.
The test plan additions are comprehensive and clearly document the expected behavior. The fenced code block showing the warning message (line 231-233) could optionally use
```textfor consistent markdown style, matching the static analysis hint.🤖 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 `@docs/manual-test-plan.md` around lines 225 - 238, In the "B5. Render with stale chart preset" section update the fenced code block that shows the warning message so it includes a language hint (e.g., change ``` to ```text) to match the repo's markdown style; locate the block under the "Drift warning." paragraph in that section (the fenced block that contains the WARN: talm binary... message) and add the language token.README.md (1)
229-268: 💤 Low valueConsider adding language hints to fenced code blocks.
The documentation is clear and comprehensive. As a minor style improvement, consider adding language specifiers to the fenced code blocks:
- Line 233: Directory tree could use
```text- Line 255: Warning message could use
```textThis improves rendering in some markdown viewers. As per coding guidelines, this is flagged by markdownlint but is optional.
🤖 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 `@README.md` around lines 229 - 268, Add language hints to the two fenced code blocks in the README: the directory tree block (the block showing "your-project/... templates/ charts/talm/") and the warning message block (the block starting with "WARN: talm binary is ..."); update their opening backticks to include a language specifier such as ```text so markdown linters/renderers treat them as plain text (leave other blocks like ```bash and ```yaml unchanged).
🤖 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.
Nitpick comments:
In `@docs/manual-test-plan.md`:
- Around line 225-238: In the "B5. Render with stale chart preset" section
update the fenced code block that shows the warning message so it includes a
language hint (e.g., change ``` to ```text) to match the repo's markdown style;
locate the block under the "Drift warning." paragraph in that section (the
fenced block that contains the WARN: talm binary... message) and add the
language token.
In `@README.md`:
- Around line 229-268: Add language hints to the two fenced code blocks in the
README: the directory tree block (the block showing "your-project/... templates/
charts/talm/") and the warning message block (the block starting with "WARN:
talm binary is ..."); update their opening backticks to include a language
specifier such as ```text so markdown linters/renderers treat them as plain text
(leave other blocks like ```bash and ```yaml unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62fa366b-b263-4f32-8e4b-95dbeef4bb13
📒 Files selected for processing (6)
README.mddocs/manual-test-plan.mdmain.gopkg/commands/root.gopkg/commands/version_check.gopkg/commands/version_check_test.go
|
Thanks for digging into this — the underlying problem is real: I'm closing this in favor of a different implementation, because comparing the binary version against the project's top-level
The replacement detects drift by comparing the vendored Appreciate the contribution — it surfaced a genuine gap. |
What
talm vendors its preset and library charts into the project directory at
talm inittime. Render commands (template/apply/upgrade) load thatlocal copy — never the binary's embedded charts. So upgrading the talm
binary leaves
charts/talm/frozen at whatever version last ran init, andthat drift was invisible: operators had to manually eyeball
talm versionagainst
Chart.yaml.This PR surfaces the drift.
How
version:stamped in the project'sChart.yamlon every config-loading command.WARN:on stderr pointing attalm init --update.Rendered output (stdout) and exit code are unchanged — pipelines/CI keep working.
strictVersion: trueinChart.yaml(committed, so awhole team/CI inherits it) or the
--strict-versionflag turns a mismatchinto a hard error (exit 1).
devbuilds, the0.1.0dev sentinel, empty/unparseableversions, and matching versions — never a false alarm.
Reuses the existing
version:stamp written byinit/init --update; no newrequired field.
strictVersionis optional and absent means today's behavior —fully backward-compatible.
Docs
explaining the embed→vendor→render model and the
init --updateworkflow.Testing
go test ./...green;go vet ./...clean.version_check_test.go, 9 cases) cover every warn/skip path.-ldflagsbuilds: warn / strict-via-Chart.yaml/ strict-via-flag → exit 1, dev-build → silent, matched → silent.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
--strict-versionflag and configuration option to enforce strict version matching with hard errorsDocumentation