From bf1ccb64f4a36b19982db7f9386533d0c87fd404 Mon Sep 17 00:00:00 2001 From: Tamish Mhatre Date: Thu, 30 Jul 2026 17:29:29 +0000 Subject: [PATCH 1/2] display: show version in Banner and WelcomeBanner 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 #1 --- cmd/root.go | 1 + internal/display/display.go | 17 ++++++- internal/display/display_test.go | 84 ++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 internal/display/display_test.go diff --git a/cmd/root.go b/cmd/root.go index 27afa00..05e5a1a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -72,6 +72,7 @@ func FormatExecuteError(err error) string { func NewRootCmd(version string) *cobra.Command { ver := cleanVersion(version) + display.SetVersion(ver) root := &cobra.Command{ Use: "archcore", Short: "Archcore — Git-native context for AI coding agents", diff --git a/internal/display/display.go b/internal/display/display.go index 2a1c74a..7196683 100644 --- a/internal/display/display.go +++ b/internal/display/display.go @@ -16,8 +16,21 @@ var ( Logo = lipgloss.NewStyle().Foreground(lipgloss.Color("12")) ) +var version = "" + +func SetVersion(v string) { + version = v +} + +func versionSuffix() string { + if version == "" || version == "dev" { + return "" + } + return " " + version +} + func Banner() string { - return Title.Render("Archcore") + Dim.Render(" — Git-native context for AI coding agents") + return Title.Render("Archcore"+versionSuffix()) + Dim.Render(" — Git-native context for AI coding agents") } func WelcomeBanner() string { @@ -31,7 +44,7 @@ func WelcomeBanner() string { logo := Logo.Render(strings.Join(logoLines, "\n")) textLines := []string{ - Title.Render("Archcore — Git-native context for AI coding agents"), + Title.Render("Archcore" + versionSuffix() + " — Git-native context for AI coding agents"), Dim.Render("Context engineering for repositories"), Dim.Render("https://archcore.ai"), } diff --git a/internal/display/display_test.go b/internal/display/display_test.go new file mode 100644 index 0000000..e42a785 --- /dev/null +++ b/internal/display/display_test.go @@ -0,0 +1,84 @@ +package display + +import ( + "strings" + "testing" +) + +func TestBannerWithoutVersion(t *testing.T) { + SetVersion("") + b := Banner() + if !strings.Contains(b, "Archcore") { + t.Fatalf("Banner missing 'Archcore': %q", b) + } + if strings.Contains(b, "v0.5.4") { + t.Fatalf("Banner should not contain version when unset: %q", b) + } +} + +func TestBannerWithVersion(t *testing.T) { + SetVersion("v0.5.4") + b := Banner() + if !strings.Contains(b, "v0.5.4") { + t.Fatalf("Banner missing version 'v0.5.4': %q", b) + } +} + +func TestBannerWithDevVersion(t *testing.T) { + SetVersion("dev") + b := Banner() + if strings.Contains(b, "dev") { + t.Fatalf("Banner should not contain 'dev' version: %q", b) + } +} + +func TestWelcomeBannerWithoutVersion(t *testing.T) { + SetVersion("") + wb := WelcomeBanner() + if !strings.Contains(wb, "Archcore") { + t.Fatalf("WelcomeBanner missing 'Archcore': %q", wb) + } + if strings.Contains(wb, "v0.5.4") { + t.Fatalf("WelcomeBanner should not contain version when unset: %q", wb) + } +} + +func TestWelcomeBannerWithVersion(t *testing.T) { + SetVersion("v0.5.4") + wb := WelcomeBanner() + if !strings.Contains(wb, "v0.5.4") { + t.Fatalf("WelcomeBanner missing version 'v0.5.4': %q", wb) + } +} + +func TestWelcomeBannerWithDevVersion(t *testing.T) { + SetVersion("dev") + wb := WelcomeBanner() + if strings.Contains(wb, "dev") { + t.Fatalf("WelcomeBanner should not contain 'dev' version: %q", wb) + } +} + +func TestVersionSuffixEmpty(t *testing.T) { + SetVersion("") + got := versionSuffix() + if got != "" { + t.Fatalf("expected empty suffix, got %q", got) + } +} + +func TestVersionSuffixDev(t *testing.T) { + SetVersion("dev") + got := versionSuffix() + if got != "" { + t.Fatalf("expected empty suffix for dev, got %q", got) + } +} + +func TestVersionSuffixReal(t *testing.T) { + SetVersion("v1.2.3") + got := versionSuffix() + if got != " v1.2.3" { + t.Fatalf("expected ' v1.2.3', got %q", got) + } +} From 5cb79b81f6a68a23f42aabc3eead62c536a1dc4f Mon Sep 17 00:00:00 2001 From: Tamish Mhatre Date: Sun, 2 Aug 2026 09:23:18 +0000 Subject: [PATCH 2/2] fix: replace global version setter with parameter passing 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 --- cmd/doctor.go | 4 +- cmd/init.go | 10 +-- cmd/init_agent_flag_spec_test.go | 2 +- cmd/mcp.go | 2 +- cmd/root.go | 11 ++- cmd/sync.go | 6 +- cmd/sync_test.go | 26 +++---- cmd/update.go | 2 +- internal/display/display.go | 16 ++-- internal/display/display_test.go | 123 ++++++++++++++----------------- 10 files changed, 93 insertions(+), 109 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 4bc85b6..5f7da6a 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -14,7 +14,7 @@ import ( "github.com/spf13/cobra" ) -func newDoctorCmd() *cobra.Command { +func newDoctorCmd(version string) *cobra.Command { var ( fix bool fixAgents []string @@ -31,7 +31,7 @@ func newDoctorCmd() *cobra.Command { return errors.New("--agent requires --fix") } - fmt.Println(display.Banner()) + fmt.Println(display.Banner(version)) fmt.Println() cwd, err := resolveProjectRoot(projectFlag, os.Getenv("ARCHCORE_PROJECT_ROOT")) diff --git a/cmd/init.go b/cmd/init.go index 865c1c8..b03ebae 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -87,7 +87,7 @@ func runInit(ctx context.Context, baseDir string, settings *config.Settings) (*i return result, nil } -func newInitCmd() *cobra.Command { +func newInitCmd(version string) *cobra.Command { var ( agentFlags []string projectFlag string @@ -108,10 +108,10 @@ func newInitCmd() *cobra.Command { if err != nil { return err } - return runInitForAgents(baseDir, agentFlags) + return runInitForAgents(baseDir, agentFlags, version) } - fmt.Println(display.WelcomeBanner()) + fmt.Println(display.WelcomeBanner(version)) fmt.Println() cwd, err := resolveProjectRoot(projectFlag, os.Getenv("ARCHCORE_PROJECT_ROOT")) @@ -186,7 +186,7 @@ func newInitCmd() *cobra.Command { // never open a TTY prompt; write all artifacts under baseDir regardless of // process cwd; keep existing .archcore/ settings untouched (idempotent pass); // explicit --agent implies consent for the usage-hint instructions. -func runInitForAgents(baseDir string, agentIDs []string) error { +func runInitForAgents(baseDir string, agentIDs []string, version string) error { list := make([]*agents.Agent, 0, len(agentIDs)) for _, id := range agentIDs { agent := agents.ByID(agents.AgentID(id)) @@ -196,7 +196,7 @@ func runInitForAgents(baseDir string, agentIDs []string) error { list = append(list, agent) } - fmt.Println(display.WelcomeBanner()) + fmt.Println(display.WelcomeBanner(version)) fmt.Println() created, err := wiring.EnsureProjectInitialized(baseDir) diff --git a/cmd/init_agent_flag_spec_test.go b/cmd/init_agent_flag_spec_test.go index 03e3625..ff620a1 100644 --- a/cmd/init_agent_flag_spec_test.go +++ b/cmd/init_agent_flag_spec_test.go @@ -28,7 +28,7 @@ import ( // implementation routes output via cmd.OutOrStdout(). func runInitCmdForSpec(t *testing.T, args ...string) (*bytes.Buffer, error) { t.Helper() - cmd := newInitCmd() + cmd := newInitCmd("") out := &bytes.Buffer{} cmd.SetOut(out) cmd.SetErr(out) diff --git a/cmd/mcp.go b/cmd/mcp.go index e690094..23ad849 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -30,7 +30,7 @@ func newMCPCmd(version string) *cobra.Command { return err } - fmt.Fprintln(os.Stderr, display.WelcomeBanner()) + fmt.Fprintln(os.Stderr, display.WelcomeBanner(version)) fmt.Fprintln(os.Stderr) if !config.DirExists(baseDir) { fmt.Fprintln(os.Stderr, display.Dim.Render(" MCP server running on stdio (uninitialized project — only init_project tool is useful until the agent initializes .archcore/)...")) diff --git a/cmd/root.go b/cmd/root.go index 05e5a1a..721bd52 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -72,7 +72,6 @@ func FormatExecuteError(err error) string { func NewRootCmd(version string) *cobra.Command { ver := cleanVersion(version) - display.SetVersion(ver) root := &cobra.Command{ Use: "archcore", Short: "Archcore — Git-native context for AI coding agents", @@ -85,7 +84,7 @@ func NewRootCmd(version string) *cobra.Command { // here would be dead — unlike the non-root commands below, which DO need // it because legacyArgs lets a subcommand's stray positional through. RunE: func(cmd *cobra.Command, args []string) error { - fmt.Fprintln(cmd.OutOrStdout(), display.WelcomeBanner()) + fmt.Fprintln(cmd.OutOrStdout(), display.WelcomeBanner(ver)) fmt.Fprintln(cmd.OutOrStdout()) _ = cmd.Usage() return nil @@ -100,7 +99,7 @@ func NewRootCmd(version string) *cobra.Command { defaultHelp := root.HelpFunc() root.SetHelpFunc(func(cmd *cobra.Command, args []string) { if cmd == root { - fmt.Fprintln(cmd.OutOrStdout(), display.WelcomeBanner()) + fmt.Fprintln(cmd.OutOrStdout(), display.WelcomeBanner(ver)) fmt.Fprintln(cmd.OutOrStdout()) _ = cmd.Usage() return @@ -109,14 +108,14 @@ func NewRootCmd(version string) *cobra.Command { }) root.AddCommand( - newInitCmd(), + newInitCmd(ver), newConfigCmd(), - newDoctorCmd(), + newDoctorCmd(ver), newStatusCmd(), newHooksCmd(ver), newMCPCmd(version), newInstructionsCmd(), - newSyncCmd(), + newSyncCmd(ver), newUpdateCmd(ver), ) diff --git a/cmd/sync.go b/cmd/sync.go index 9788b0d..8a6d4bc 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -65,7 +65,7 @@ type syncFlags struct { CI bool } -func newSyncCmd() *cobra.Command { +func newSyncCmd(version string) *cobra.Command { flags := &syncFlags{} cmd := &cobra.Command{ @@ -88,7 +88,7 @@ func newSyncCmd() *cobra.Command { // doSync contains the core sync logic, separated from cobra and os.Getwd for // testability. When the sync gate in newSyncCmd is lifted, its RunE becomes: // checkSyncPreconditions(cwd) → api.NewSyncClient(pre.ServerURL) → doSync. -func doSync(ctx context.Context, baseDir string, flags *syncFlags, pre *syncPreconditions, client syncClient) error { +func doSync(ctx context.Context, baseDir string, flags *syncFlags, pre *syncPreconditions, client syncClient, version string) error { // 2. Load manifest and scan files. manifest, err := archsync.LoadManifest(baseDir) if err != nil { @@ -136,7 +136,7 @@ func doSync(ctx context.Context, baseDir string, flags *syncFlags, pre *syncPrec return nil } - fmt.Println(display.Banner()) + fmt.Println(display.Banner(version)) fmt.Println() if len(created) > 0 { fmt.Println(display.CheckLine(fmt.Sprintf("%d new file(s)", len(created)))) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 25cb030..5c7d61c 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -221,7 +221,7 @@ func TestRunSync_DryRun_DoesNotUpdateManifest(t *testing.T) { mock := &mockSyncClient{} flags := &syncFlags{DryRun: true, CI: true} - err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -264,7 +264,7 @@ func TestRunSync_Force_ResyncsUnchangedFiles(t *testing.T) { } flags := &syncFlags{Force: true, CI: true} - err = doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err = doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -298,7 +298,7 @@ func TestRunSync_Force_DetectsDeletions(t *testing.T) { } flags := &syncFlags{Force: true, CI: true} - err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -351,7 +351,7 @@ func TestRunSync_NoChanges_ShortCircuit(t *testing.T) { mock := &mockSyncClient{} flags := &syncFlags{CI: true} - err = doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err = doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -377,7 +377,7 @@ func TestRunSync_EndToEnd(t *testing.T) { } flags := &syncFlags{CI: true} - err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -428,7 +428,7 @@ func TestRunSync_AutoCreateProject(t *testing.T) { pre := testPreconditionsNoPID(baseDir) pre.Settings = &config.Settings{Sync: config.SyncTypeCloud} - err := doSync(context.Background(), baseDir, flags, pre, mock) + err := doSync(context.Background(), baseDir, flags, pre, mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -468,7 +468,7 @@ func TestRunSync_PayloadHasFrontmatter(t *testing.T) { } flags := &syncFlags{CI: true} - err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -511,7 +511,7 @@ func TestRunSync_AutoCreate_NoGitRepo_RepoURLNil(t *testing.T) { pre := testPreconditionsNoPID(baseDir) pre.Settings = &config.Settings{Sync: config.SyncTypeCloud} - err := doSync(context.Background(), baseDir, flags, pre, mock) + err := doSync(context.Background(), baseDir, flags, pre, mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -548,7 +548,7 @@ func TestRunSync_AutoCreate_WithGitRepo_RepoURLPopulated(t *testing.T) { pre := testPreconditionsNoPID(baseDir) pre.Settings = &config.Settings{Sync: config.SyncTypeCloud} - err := doSync(context.Background(), baseDir, flags, pre, mock) + err := doSync(context.Background(), baseDir, flags, pre, mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -585,7 +585,7 @@ func TestRunSync_ExistingProject_NoRepoURL(t *testing.T) { } flags := &syncFlags{CI: true} - err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err != nil { t.Fatalf("doSync: %v", err) } @@ -604,7 +604,7 @@ func TestDoSync_ClientError_CI(t *testing.T) { mock := &mockSyncClient{err: context.DeadlineExceeded} flags := &syncFlags{CI: true} - err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock) + err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, "") if err == nil { t.Fatal("CI mode must propagate a sync failure as an error") } @@ -645,7 +645,7 @@ func TestDoSync_PartialFailure_RejectedFilesNotRecorded(t *testing.T) { }, } flags := &syncFlags{CI: true} - if err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock); err != nil { + if err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, ""); err != nil { t.Fatalf("doSync: %v", err) } @@ -685,7 +685,7 @@ func TestDoSync_AcceptedDeletionRemoved(t *testing.T) { }, } flags := &syncFlags{CI: true} - if err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock); err != nil { + if err := doSync(context.Background(), baseDir, flags, testPreconditions(baseDir), mock, ""); err != nil { t.Fatalf("doSync: %v", err) } diff --git a/cmd/update.go b/cmd/update.go index 7ae868a..5e9033f 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -136,7 +136,7 @@ func buildUpdateCmd(version string, u *update.Updater) *cobra.Command { return nil } - fmt.Println(display.Banner()) + fmt.Println(display.Banner(version)) fmt.Println() fmt.Println(display.Dim.Render(" Checking for updates...")) diff --git a/internal/display/display.go b/internal/display/display.go index 7196683..e603851 100644 --- a/internal/display/display.go +++ b/internal/display/display.go @@ -16,24 +16,18 @@ var ( Logo = lipgloss.NewStyle().Foreground(lipgloss.Color("12")) ) -var version = "" - -func SetVersion(v string) { - version = v -} - -func versionSuffix() string { +func versionSuffix(version string) string { if version == "" || version == "dev" { return "" } return " " + version } -func Banner() string { - return Title.Render("Archcore"+versionSuffix()) + Dim.Render(" — Git-native context for AI coding agents") +func Banner(version string) string { + return Title.Render("Archcore"+versionSuffix(version)) + Dim.Render(" — Git-native context for AI coding agents") } -func WelcomeBanner() string { +func WelcomeBanner(version string) string { logoLines := []string{ "╔══════╗", "║ ║", @@ -44,7 +38,7 @@ func WelcomeBanner() string { logo := Logo.Render(strings.Join(logoLines, "\n")) textLines := []string{ - Title.Render("Archcore" + versionSuffix() + " — Git-native context for AI coding agents"), + Title.Render("Archcore" + versionSuffix(version) + " — Git-native context for AI coding agents"), Dim.Render("Context engineering for repositories"), Dim.Render("https://archcore.ai"), } diff --git a/internal/display/display_test.go b/internal/display/display_test.go index e42a785..8754ce4 100644 --- a/internal/display/display_test.go +++ b/internal/display/display_test.go @@ -5,80 +5,71 @@ import ( "testing" ) -func TestBannerWithoutVersion(t *testing.T) { - SetVersion("") - b := Banner() - if !strings.Contains(b, "Archcore") { - t.Fatalf("Banner missing 'Archcore': %q", b) +func TestVersionSuffix(t *testing.T) { + tests := []struct { + name string + version string + want string + }{ + {"empty", "", ""}, + {"dev", "dev", ""}, + {"real version", "v0.5.4", " v0.5.4"}, + {"another version", "v1.2.3", " v1.2.3"}, } - if strings.Contains(b, "v0.5.4") { - t.Fatalf("Banner should not contain version when unset: %q", b) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := versionSuffix(tt.version) + if got != tt.want { + t.Fatalf("versionSuffix(%q) = %q, want %q", tt.version, got, tt.want) + } + }) } } -func TestBannerWithVersion(t *testing.T) { - SetVersion("v0.5.4") - b := Banner() - if !strings.Contains(b, "v0.5.4") { - t.Fatalf("Banner missing version 'v0.5.4': %q", b) +func TestBanner(t *testing.T) { + tests := []struct { + name string + version string + wantContain string + notContain string + }{ + {"no version", "", "Archcore", "v0.5.4"}, + {"dev version", "dev", "Archcore", "dev"}, + {"real version", "v0.5.4", "v0.5.4", ""}, } -} - -func TestBannerWithDevVersion(t *testing.T) { - SetVersion("dev") - b := Banner() - if strings.Contains(b, "dev") { - t.Fatalf("Banner should not contain 'dev' version: %q", b) - } -} - -func TestWelcomeBannerWithoutVersion(t *testing.T) { - SetVersion("") - wb := WelcomeBanner() - if !strings.Contains(wb, "Archcore") { - t.Fatalf("WelcomeBanner missing 'Archcore': %q", wb) - } - if strings.Contains(wb, "v0.5.4") { - t.Fatalf("WelcomeBanner should not contain version when unset: %q", wb) - } -} - -func TestWelcomeBannerWithVersion(t *testing.T) { - SetVersion("v0.5.4") - wb := WelcomeBanner() - if !strings.Contains(wb, "v0.5.4") { - t.Fatalf("WelcomeBanner missing version 'v0.5.4': %q", wb) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := Banner(tt.version) + if !strings.Contains(b, tt.wantContain) { + t.Fatalf("Banner(%q) missing %q: %q", tt.version, tt.wantContain, b) + } + if tt.notContain != "" && strings.Contains(b, tt.notContain) { + t.Fatalf("Banner(%q) should not contain %q: %q", tt.version, tt.notContain, b) + } + }) } } -func TestWelcomeBannerWithDevVersion(t *testing.T) { - SetVersion("dev") - wb := WelcomeBanner() - if strings.Contains(wb, "dev") { - t.Fatalf("WelcomeBanner should not contain 'dev' version: %q", wb) +func TestWelcomeBanner(t *testing.T) { + tests := []struct { + name string + version string + wantContain string + notContain string + }{ + {"no version", "", "Archcore", "v0.5.4"}, + {"dev version", "dev", "Archcore", "dev"}, + {"real version", "v0.5.4", "v0.5.4", ""}, } -} - -func TestVersionSuffixEmpty(t *testing.T) { - SetVersion("") - got := versionSuffix() - if got != "" { - t.Fatalf("expected empty suffix, got %q", got) - } -} - -func TestVersionSuffixDev(t *testing.T) { - SetVersion("dev") - got := versionSuffix() - if got != "" { - t.Fatalf("expected empty suffix for dev, got %q", got) - } -} - -func TestVersionSuffixReal(t *testing.T) { - SetVersion("v1.2.3") - got := versionSuffix() - if got != " v1.2.3" { - t.Fatalf("expected ' v1.2.3', got %q", got) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wb := WelcomeBanner(tt.version) + if !strings.Contains(wb, tt.wantContain) { + t.Fatalf("WelcomeBanner(%q) missing %q: %q", tt.version, tt.wantContain, wb) + } + if tt.notContain != "" && strings.Contains(wb, tt.notContain) { + t.Fatalf("WelcomeBanner(%q) should not contain %q: %q", tt.version, tt.notContain, wb) + } + }) } }