display: show version in Banner and WelcomeBanner by tamish560 · Pull Request #29 · archcore-ai/cli

tamish560 · 2026-07-30T17:30:27Z

What

Banner() and WelcomeBanner() now render the resolved build version (e.g. Archcore v0.5.4) when one is set, with a graceful fallback that omits it for empty or dev builds.

Why

Closes #1. Users and bug reporters could not identify their version from the first screen they see. The version was already resolved at startup in NewRootCmd(version) and printed in HookConnectedLine, but never surfaced in the banners.

How

Added a package-level version in internal/display with SetVersion(v string) to set it once.
versionSuffix() returns "" for empty or "dev", otherwise " v0.5.4".
Banner() and WelcomeBanner() append versionSuffix() to the title.
cmd/root.go calls display.SetVersion(ver) right after cleanVersion.

Tests

9 new tests in internal/display/display_test.go covering:

Banner with no version (empty string)
Banner with real version (v0.5.4)
Banner with dev version
WelcomeBanner with no version
WelcomeBanner with real version
WelcomeBanner with dev version
versionSuffix for empty, dev, and real version

All pass. go build ./... and go vet clean.

Banner() and WelcomeBanner() now include the resolved version when set, with a graceful fallback for empty or dev builds. SetVersion() is called from NewRootCmd where the version is already known. Tests cover empty, dev, and real version cases. Closes archcore-ai#1

ivklgn

Thanks - the visible behavior is exactly what #1 asked for, and hiding the suffix for empty/dev builds is the right call. But the package-level version + setter shape has to go: it's a real data race (reproducible with go test -race ./cmd/, details inline) and our code-quality rule explicitly says to pass the version as an argument, not a global. Swapping the setter for a version string parameter on both banners resolves most of the inline comments in one move. After that: table-driven tests per our testing guide, and a decision on the separate version line in WelcomeBanner.

Ran the suite on the PR state: green without -race, fails with it.

ivklgn · 2026-08-02T09:04:40Z


 func NewRootCmd(version string) *cobra.Command {
 	ver := cleanVersion(version)
+	display.SetVersion(ver)


There's a data race here. NewRootCmd now writes to display.version, and args_validation_test.go calls NewRootCmd from 12 parallel subtests - go test -race ./cmd/ -run TestCommands_RejectStrayPositionalArgs fails with WARNING: DATA RACE. CI runs tests without -race, so this would land in main unnoticed. Let's drop the global: Banner(version string) / WelcomeBanner(version string), thread the version through the command factories like newHooksCmd(ver) already does. That also takes care of half the other comments here.

ivklgn · 2026-08-02T09:04:40Z


+var version = ""
+
+func SetVersion(v string) {


go-code-quality.rule.md is explicit about this: pass the version as an argument to command factories, not as a package-level global - var Version string is literally the anti-pattern example there. And this very file already has the precedent: HookConnectedLine(version string, docCount int). I know version-in-banners.idea.md sketched a global, but that idea is a draft while the rule is accepted. Going with a parameter means updating the call sites in root.go, init.go, doctor.go, sync.go, update.go and mcp.go.

ivklgn · 2026-08-02T09:04:40Z

+}
+
+func versionSuffix() string {
+	if version == "" || version == "dev" {


versionSuffix silently assumes the input already went through cleanVersion. Pass it "1.2.3" and the banner reads "Archcore 1.2.3", no v. And "dev-abc123" slips past the == "dev" check, so you'd get "vdev-abc123". The dev knowledge now lives in two packages. I'd keep normalization in cmd only: cleanVersion stays the single entry point, display gets an already-clean string (empty for dev builds).

ivklgn · 2026-08-02T09:04:40Z

+	"testing"
+)
+
+func TestBannerWithoutVersion(t *testing.T) {


Nine flat functions that are really a 3x3 table: {Banner, WelcomeBanner, versionSuffix} x {empty, dev, real}. Tests in this project are table-driven (unit-testing-patterns.guide.md), let's collapse these into three table tests. A few cases are missing too: a version without the v prefix ("1.2.3"), "dev-abc123", a whitespace-only string.

ivklgn · 2026-08-02T09:04:40Z

+var version = ""
+
+func SetVersion(v string) {
+	version = v


Exported SetVersion has no doc comment, revive's exported check will flag it. Moot if we switch to a parameter. If the setter stays, document the contract: v is expected to already be cleanVersion'd, empty string and "dev" hide the suffix.

ivklgn · 2026-08-02T09:04:40Z

+)
+
+func TestBannerWithoutVersion(t *testing.T) {
+	SetVersion("")


Every test calls SetVersion(...) and nobody restores it. After the run the package is left with version = "v1.2.3", and any future test that forgets to set its own value inherits someone else's state. At minimum t.Cleanup(func() { version = "" }). With a parameter instead of a global the problem disappears and these tests could run in parallel.

ivklgn · 2026-08-02T09:04:41Z


 	textLines := []string{
-		Title.Render("Archcore — Git-native context for AI coding agents"),
+		Title.Render("Archcore" + versionSuffix() + " — Git-native context for AI coding agents"),


version-in-banners.idea.md sketched a separate dim version line below the title for WelcomeBanner, exactly so a long v0.0.1-alpha.5 wouldn't stretch the title or break alignment with the ASCII logo. Here the version gets inlined into the title. Maybe that's fine, the idea is a draft after all, but let's make the choice explicit - in the PR description and in the idea doc.

ivklgn · 2026-08-02T09:04:41Z

 	Logo    = lipgloss.NewStyle().Foreground(lipgloss.Color("12"))
 )

+var version = ""


Nits: var version = "" can be just var version string, the zero value is already an empty string. Asserting absence via the magic string "v0.5.4" in the tests is brittle - once these go table-driven, compare versionSuffix() for exact equality instead. And the naming guide wants TestBanner_WithoutVersion style.

ivklgn · 2026-08-02T09:06:05Z

@tamish560 Thank you for your activity!

Remove package-level version variable and SetVersion() setter to fix data race reported by reviewer (fails go test -race). Changes: - Banner() and WelcomeBanner() now accept version string as parameter - versionSuffix() is now a pure function taking version as input - All call sites updated to pass version through command constructors - Tests rewritten as table-driven per testing guide - go test -race passes clean

tamish560 · 2026-08-02T09:23:33Z

@ivklgn thanks for the review. Pushed a fix for all three points:

1. Data race + global setter removed

version package variable and SetVersion() are gone. Banner() and WelcomeBanner() now accept version string as a parameter. versionSuffix() is a pure function taking version as input. All call sites in cmd/ pass the version through command constructors (newInitCmd(ver), newDoctorCmd(ver), newSyncCmd(ver), etc.).

go test -race ./cmd/... ./internal/display/... passes clean.

2. Table-driven tests

Rewrote display_test.go with three table-driven tests: TestVersionSuffix, TestBanner, TestWelcomeBanner. Each covers empty, dev, and real version cases in a single []struct with t.Run subtests.

3. WelcomeBanner version line

The version suffix is appended to the title line in WelcomeBanner (same as Banner), not on a separate line. If you prefer it on its own line below the title, let me know and I will move it.

ivklgn · 2026-08-02T20:13:02Z

@tamish560 thank you! 🎉🎉🎉

ivklgn requested changes Aug 2, 2026

View reviewed changes

ivklgn merged commit 17373a2 into archcore-ai:main Aug 2, 2026
1 check passed

Navigation Menu

Uh oh!

display: show version in Banner and WelcomeBanner - #29

display: show version in Banner and WelcomeBanner#29
ivklgn merged 2 commits into
archcore-ai:mainfrom
tamish560:fix/banner-version

tamish560 commented Jul 30, 2026

Uh oh!

ivklgn left a comment

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn Aug 2, 2026

Uh oh!

ivklgn commented Aug 2, 2026

Uh oh!

tamish560 commented Aug 2, 2026

Uh oh!

Uh oh!

ivklgn commented Aug 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tamish560 commented Jul 30, 2026

What

Why

How

Tests

Uh oh!

ivklgn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivklgn commented Aug 2, 2026

Uh oh!

tamish560 commented Aug 2, 2026

Uh oh!

Uh oh!

ivklgn commented Aug 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants