Scion/harness journey p1

[ptone]

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

• Tests pass
• Appropriate changes to documentation are included in the PR

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request introduces the ability to re-import and update harness configurations and templates from their remote source URLs. It adds a new harness-config update CLI command, a POST /api/v1/harness-configs/{id}/reimport API endpoint, database schema updates to store the source_url, and a "Refresh from Source" button in the frontend UI. The review feedback highlights several safety and robustness improvements, including using cmd.Context() to respect CLI cancellation signals, avoiding the r.ContentLength anti-pattern when parsing HTTP request bodies, and adding nil checks for database and API client responses to prevent potential nil pointer dereferences.

[gemini-code-assist]

Checking r.ContentLength > 0 to decide whether to parse the request body is an anti-pattern. In HTTP/1.1 with chunked transfer encoding or in HTTP/2, r.ContentLength can be -1 (indicating unknown length), which would cause the request body to be silently ignored. Instead, check if r.Body is not nil and not http.NoBody before decoding.

Suggested change

	var req ReimportHarnessConfigRequest
	if r.ContentLength > 0 {
	if err := readJSON(r, &req); err != nil {
	BadRequest(w, "Invalid request body: "+err.Error())
	return
	}
	}
	var req ReimportHarnessConfigRequest
	if r.Body != nil && r.Body != http.NoBody {
	if err := readJSON(r, &req); err != nil {
	BadRequest(w, "Invalid request body: "+err.Error())
	return
	}
	}

[gemini-code-assist]

Add a nil check for hc to prevent a potential nil pointer dereference if GetHarnessConfig returns nil, nil.

	hc, err := s.store.GetHarnessConfig(ctx, id)
	if err != nil {
		writeErrorFromErr(w, err, "")
		return
	}
	if hc == nil {
		writeError(w, http.StatusNotFound, "not_found", "Harness config not found", nil)
		return
	}

[gemini-code-assist]

Use cmd.Context() instead of context.Background() to respect cancellation signals (such as Ctrl+C) handled by Cobra.

Suggested change

	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
	defer cancel()
	ctx, cancel := context.WithTimeout(cmd.Context(), 120*time.Second)
	defer cancel()

[gemini-code-assist]

Add a nil check for resp to prevent a potential nil pointer dereference if the API client returns a nil response without an error.

	if err != nil {
		return fmt.Errorf("failed to search Hub: %w", err)
	}
	if resp == nil {
		return fmt.Errorf("failed to search Hub: received empty response")
	}

[gemini-code-assist]

Add a nil check for resp to prevent a potential nil pointer dereference if the API client returns a nil response without an error.

Suggested change

	if err != nil {
	return fmt.Errorf("failed to list harness-configs: %w", err)
	}
	if err != nil {
	return fmt.Errorf("failed to list harness-configs: %w", err)
	}
	if resp == nil {
	return fmt.Errorf("failed to list harness-configs: received empty response")
	}

[ptone]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces the ability to re-import and update harness configurations from their remote source URLs. It adds a new harness-config update CLI command, a backend API endpoint /api/v1/harness-configs/{id}/reimport, and a frontend "Refresh from Source" button. Additionally, the source_url field is integrated into the HarnessConfig and Template database schemas. The review feedback highlights two main improvements in cmd/harness_config_update.go: handling errors when resolving the project directory instead of silently ignoring them, and adding a nil check on the re-import result to prevent a potential nil pointer dereference.

[gemini-code-assist]

The Reimport call can potentially return a nil result even if err is nil. To prevent a potential nil pointer dereference panic when accessing result.Count or result.HarnessConfigs, add a nil check for result.

result, err := hubCtx.Client.HarnessConfigs().Reimport(ctx, match.ID, urlOverride)
	if err != nil {
		return fmt.Errorf("reimport failed: %w", err)
	}
	if result == nil {
		return fmt.Errorf("reimport failed: received empty response from Hub")
	}

[gemini-code-assist]

If projectPath is explicitly provided by the user, any error encountered while resolving the project directory should be handled and returned rather than silently ignored. Silently ignoring the error can lead to unexpected behavior or confusing error messages later in the execution.

Suggested change

	var gp string
	if projectPath != "" {
	resolved, err := config.GetResolvedProjectDir(projectPath)
	if err == nil {
	gp = resolved
	}
	} else if projectDir, err := config.GetResolvedProjectDir(""); err == nil {
	gp = projectDir
	}
	var gp string
	if projectPath != "" {
	resolved, err := config.GetResolvedProjectDir(projectPath)
	if err != nil {
	return fmt.Errorf("failed to resolve project directory %q: %w", projectPath, err)
	}
	gp = resolved
	} else if projectDir, err := config.GetResolvedProjectDir(""); err == nil {
	gp = projectDir
	}