Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.

LLM-1387: only update harness from now on #57

Merged
Zilv1nas merged 1 commit into from
May 6, 2026
Merged

LLM-1387: only update harness from now on #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions cmd/harness.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,18 @@ func runHarness(cmd *cobra.Command, args []string) error {
return launchHarness(cmd, harnessPath, args)
}

// Reuse the update command logic for CLI + harness updates.
// Uses cache (skipCache=false) since harness launches happen frequently.
if _, err := runUpdate(cmd, false, false, false); err != nil {
result, err := runUpdate(cmd, false, false, false)
Copy link
Copy Markdown

@kimchi-review kimchi-review Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️⚠️ Error Handling

runUpdate now propagates errors from runHarnessUpdate as fatal, whereas previously runHarnessUpdate logged warnings and continued. If the update check fails (e.g., network error, GitHub API unavailable), the entire kimchi harness command fails and prevents launching the existing harness, degrading user experience.

💡 Suggestion: Change the error handling to log a warning and continue instead of returning: if err != nil { fmt.Fprintf(cmd.ErrOrStderr(), "Warning: harness update check failed: %v\n", err) } else if result != nil && result.Updated { ... }

if err != nil {
return err
}

// If the harness was updated, exit and ask the user to re-run kimchi.
if result != nil && result.Updated {
_, _ = fmt.Fprintln(cmd.OutOrStdout(), "\n✓ Harness was updated. Please re-run the 'kimchi' command to use the new version.")
return nil
}

// If the harness is not installed (e.g. user declined, or fetch failed), exit
// gracefully instead of treating it as a fatal error.
if !update.HarnessInstalled(harnessPath) {
Expand Down
57 changes: 6 additions & 51 deletions cmd/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"k8s.io/klog/v2"

"github.com/castai/kimchi/internal/update"
"github.com/castai/kimchi/internal/version"
)

type updateDoneMsg struct{ err error }
Expand Down Expand Up @@ -52,10 +51,6 @@ func (m updateModel) View() string {
return m.spinner.View() + " Updating to " + m.version + "...\n"
}

type updateResult struct {
cliUpdated bool
}

func NewUpdateCommand() *cobra.Command {
var (
force bool
Expand All @@ -78,58 +73,17 @@ func NewUpdateCommand() *cobra.Command {
return cmd
Copy link
Copy Markdown

@kimchi-review kimchi-review Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️🔧 Maintainability

The printUpdateResult helper is called with result without checking for nil. While wf.Run() likely never returns nil for success, a defensive nil check would prevent potential panics if the contract changes.

💡 Suggestion: Add a nil check before calling printUpdateResult: if result != nil { printUpdateResult(...) }

}

func runUpdate(cmd *cobra.Command, force, dryRun, skipCache bool) (*updateResult, error) {
func runUpdate(cmd *cobra.Command, force, dryRun, skipCache bool) (*update.WorkflowResult, error) {
ctx := cmd.Context()
res := &updateResult{}

cliResult, err := runCLIUpdate(cmd, ctx, force, dryRun, skipCache)
if err != nil {
return nil, err
}
res.cliUpdated = cliResult.Updated

// CLI update is deprecated - only run harness update
klog.V(1).Info("checking for coding harness updates")
runHarnessUpdate(cmd, ctx, force, dryRun, skipCache)
return res, nil
}

func runCLIUpdate(cmd *cobra.Command, ctx context.Context, force, dryRun, skipCache bool) (*update.WorkflowResult, error) {
var opts []update.WorkflowOpt
if dryRun {
opts = append(opts, update.WithDryRun())
}
if skipCache {
opts = append(opts, update.WithSkipUpdateCache())
}

opts = append(opts,
update.WithCurrentVersionFn(func(ctx context.Context) (*semver.Version, error) {
if version.Version == "dev" {
return semver.MustParse("0.0.0-dev"), nil
}
return semver.NewVersion(version.Version)
}),
update.WithConfirmFn(func(current, latest *semver.Version) (bool, error) {
prompt := fmt.Sprintf("Kimchi update available: %s → %s\nUpdate? [Y/n]: ", current, latest)
return confirmAction(cmd, prompt, "Update skipped.", force), nil
}),
update.WithProgressFn(runUpdateWithSpinner),
)
wf := update.NewCLIWorkflow(opts...)

result, err := wf.Run(ctx)
if err != nil {
return nil, fmt.Errorf("check for kimchi updates: %w", err)
}

printUpdateResult(cmd, "CLI", result, dryRun)

return result, nil
return runHarnessUpdate(cmd, ctx, force, dryRun, skipCache)
}

// runHarnessUpdate checks for the coding harness and installs or updates it.
// Failures are reported as warnings and never block the CLI update.
func runHarnessUpdate(cmd *cobra.Command, ctx context.Context, force, dryRun, skipCache bool) {
func runHarnessUpdate(cmd *cobra.Command, ctx context.Context, force, dryRun, skipCache bool) (*update.WorkflowResult, error) {
var opts []update.WorkflowOpt
if dryRun {
opts = append(opts, update.WithDryRun())
Expand All @@ -153,10 +107,11 @@ func runHarnessUpdate(cmd *cobra.Command, ctx context.Context, force, dryRun, sk
result, err := wf.Run(ctx)
Copy link
Copy Markdown

@kimchi-review kimchi-review Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️🔧 Maintainability

The comment Failures are reported as warnings and never block the CLI update is now stale and misleading. The function currently returns errors to the caller, which blocks execution (e.g., in cmd/harness.go). This contradicts the documented behavior.

💡 Suggestion: Update the comment to reflect the new behavior: Failures are returned as errors to the caller, which should handle them appropriately

if err != nil {
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %v\n", err)
return
return nil, err
}

printUpdateResult(cmd, "Coding harness", result, dryRun)
return result, nil
}

func printUpdateResult(cmd *cobra.Command, label string, result *update.WorkflowResult, dryRun bool) {
Expand Down
Loading
Loading