From 5497929f30f7d784c0dd2a27f339077884203a9d Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Tue, 14 Jul 2026 11:16:12 +0200 Subject: [PATCH 1/3] fix(deploy,api): interactive project picker + detect shell-mangled api paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CLI failure buckets surfaced by telemetry (last 30 days): deploy — "No project specified" (52 events, 47 users): resolveDeployProject already auto-selects a lone project, but multi-project accounts hard-failed even for interactive users. Add a promptui picker for the multi-project case, mirroring the existing server picker. Agents / piped / --non-interactive callers keep the structured error+list so automation can retry with --project. api — "unsupported protocol scheme \"c\"" (Windows/Git-Bash): MSYS POSIX path conversion rewrites a leading "/projects/..." into "C:/Program Files/Git/...". Detect a drive-letter path (never a valid API path) and return an actionable error pointing to MSYS_NO_PATHCONV=1 or dropping the leading slash — rather than guessing the intended endpoint, which on a raw escape hatch could fire a destructive call. Note: the other two telemetry buckets (422/404 -> internal misclassification, literal "undefined" error_message) are stale-client artifacts (v0.16.3 / v0.14.1) already fixed on current versions; no code change needed. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/commands/api.go | 29 +++++++++++++++++++++++++ internal/commands/api_test.go | 37 ++++++++++++++++++++++++++++++++ internal/commands/deploy.go | 15 +++++++++++++ internal/commands/deploy_test.go | 5 ++++- 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 internal/commands/api_test.go diff --git a/internal/commands/api.go b/internal/commands/api.go index a07dc57..40cd74d 100644 --- a/internal/commands/api.go +++ b/internal/commands/api.go @@ -42,6 +42,20 @@ func newAPICmd() *cobra.Command { } } + // Detect a path that a Git Bash / MSYS shell has rewritten into a + // Windows filesystem path (POSIX path conversion turns a leading "/" + // into "C:/Program Files/Git/..."). We refuse rather than guess the + // intended endpoint — this is a raw escape hatch and reconstructing + // the wrong path could fire a destructive call. + if isShellMangledPath(path) { + return &output.UserError{ + Message: "API path was rewritten by your shell (Git Bash/MSYS turned it into a Windows path)", + Hint: "Your shell converted the leading '/' into a filesystem path. Re-run either way:\n" + + fmt.Sprintf(" MSYS_NO_PATHCONV=1 dhq api %s / (disable path conversion)\n", method) + + fmt.Sprintf(" dhq api %s (omit the leading slash, e.g. projects/my-app)", method), + } + } + client, err := cliCtx.Client() if err != nil { return err @@ -74,3 +88,18 @@ func newAPICmd() *cobra.Command { cmd.Flags().StringVar(&jsonBody, "body", "", "JSON request body") return cmd } + +// isShellMangledPath reports whether p looks like an API path that a Git Bash / +// MSYS shell has rewritten into a Windows filesystem path via POSIX path +// conversion. A real API path is relative ("projects/x") or root-absolute +// ("/projects/x") and never begins with a drive letter, so a leading "C:/" or +// "C:\" is a reliable signal that the argument was mangled before the CLI saw +// it. +func isShellMangledPath(p string) bool { + if len(p) < 3 { + return false + } + c := p[0] + isLetter := (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') + return isLetter && p[1] == ':' && (p[2] == '/' || p[2] == '\\') +} diff --git a/internal/commands/api_test.go b/internal/commands/api_test.go new file mode 100644 index 0000000..78bf398 --- /dev/null +++ b/internal/commands/api_test.go @@ -0,0 +1,37 @@ +package commands + +import "testing" + +func TestIsShellMangledPath(t *testing.T) { + cases := []struct { + name string + path string + want bool + }{ + // Mangled by Git Bash / MSYS POSIX path conversion. + {"drive lowercase forward slash", "c:/Program Files/Git/projects/x", true}, + {"drive uppercase forward slash", "C:/Program Files/Git/projects/x", true}, + {"drive backslash", `C:\Program Files\Git\projects\x`, true}, + {"other drive letter", "d:/msys/projects/x", true}, + + // Legitimate API paths. + {"root-absolute", "/projects/my-app", false}, + {"relative", "projects/my-app", false}, + {"nested relative", "projects/my-app/deployments", false}, + {"root only", "/", false}, + + // Edge cases that must not false-positive. + {"empty", "", false}, + {"single char", "c", false}, + {"two chars", "c:", false}, + {"colon but not drive", "ab:/x", false}, + {"digit prefix", "1:/x", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isShellMangledPath(tc.path); got != tc.want { + t.Errorf("isShellMangledPath(%q) = %v, want %v", tc.path, got, tc.want) + } + }) + } +} diff --git a/internal/commands/deploy.go b/internal/commands/deploy.go index 1a3b303..29991c3 100644 --- a/internal/commands/deploy.go +++ b/internal/commands/deploy.go @@ -199,6 +199,21 @@ func resolveDeployProject(ctx context.Context, client *sdk.Client, configured st for _, p := range projects { items = append(items, fmt.Sprintf("%s (%s)", p.Identifier, p.Name)) } + // Interactive: let the user pick rather than hard-failing. Agents and + // piped/--non-interactive callers still get the structured error+list + // so automation can retry with a concrete --project. + if !env.NonInteractive { + prompt := promptui.Select{ + Label: "Select project to deploy", + Items: items, + } + idx, _, perr := prompt.Run() + if perr != nil { + return "", &output.UserError{Message: "Project selection cancelled"} + } + env.Status("Selected project: %s", projects[idx].Name) + return projects[idx].Identifier, nil + } return "", &output.UserError{ Message: "No project specified", Hint: fmt.Sprintf("Account has %d projects — pass --project . Available: %s", diff --git a/internal/commands/deploy_test.go b/internal/commands/deploy_test.go index 149076d..8dec4eb 100644 --- a/internal/commands/deploy_test.go +++ b/internal/commands/deploy_test.go @@ -602,7 +602,10 @@ func TestResolveDeployProject_MultipleProjectsListsThem(t *testing.T) { })) defer srv.Close() client := newTestSDKClient(t, srv) - env, _ := testEnvelope() + // Non-interactive (agents, piped output): the multi-project case must fail + // with the structured list rather than prompt. Interactive callers instead + // get a picker (see resolveDeployProject), which isn't exercised here. + env := &output.Envelope{Stdout: io.Discard, Stderr: io.Discard, NonInteractive: true} id, err := resolveDeployProject(t.Context(), client, "", env) require.Error(t, err) From 46def73f2269f21d91f3ab00310e918b350300e5 Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Tue, 14 Jul 2026 11:26:51 +0200 Subject: [PATCH 2/3] fix(deploy): skip interactive pickers in JSON mode Review-council finding (Codex, verified): the project and server pickers gated only on !env.NonInteractive, but --json on a TTY leaves NonInteractive false. promptui writes terminal control sequences to os.Stdout (select.go:558), so `dhq deploy --json` in a terminal would interleave non-JSON bytes into stdout, breaking the stdout=data contract. Gate both pickers on `!env.NonInteractive && !env.WantsJSON()` so JSON mode takes the structured-error path (itself valid JSON). Also closes the same pre-existing gap in the server picker. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/commands/deploy.go | 10 +++++++--- internal/commands/deploy_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/internal/commands/deploy.go b/internal/commands/deploy.go index 29991c3..e97fc3c 100644 --- a/internal/commands/deploy.go +++ b/internal/commands/deploy.go @@ -201,8 +201,10 @@ func resolveDeployProject(ctx context.Context, client *sdk.Client, configured st } // Interactive: let the user pick rather than hard-failing. Agents and // piped/--non-interactive callers still get the structured error+list - // so automation can retry with a concrete --project. - if !env.NonInteractive { + // so automation can retry with a concrete --project. JSON mode also + // takes the structured path: promptui renders to stdout, which would + // corrupt the machine-readable output contract. + if !env.NonInteractive && !env.WantsJSON() { prompt := promptui.Select{ Label: "Select project to deploy", Items: items, @@ -428,7 +430,9 @@ func newDeployCmd() *cobra.Command { resolvedServer = &servers[0] env.Status("Auto-selected server: %s", servers[0].Name) } else if err == nil && len(servers) > 1 { - if !env.NonInteractive { + // JSON mode joins the non-interactive path: promptui renders + // to stdout and would corrupt machine-readable output. + if !env.NonInteractive && !env.WantsJSON() { // Interactive picker items := make([]string, len(servers)) for i, s := range servers { diff --git a/internal/commands/deploy_test.go b/internal/commands/deploy_test.go index 8dec4eb..eb4058b 100644 --- a/internal/commands/deploy_test.go +++ b/internal/commands/deploy_test.go @@ -621,6 +621,31 @@ func TestResolveDeployProject_MultipleProjectsListsThem(t *testing.T) { assert.Contains(t, msg, "c-id (Gamma)") } +// TestResolveDeployProject_JSONModeSkipsPicker guards the stdout=data contract: +// --json on a TTY leaves NonInteractive false, but the picker must NOT run +// because promptui renders to stdout and would corrupt JSON output. JSON mode +// must take the structured-error path instead. +func TestResolveDeployProject_JSONModeSkipsPicker(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode([]sdk.Project{ + {Identifier: "a-id", Name: "Alpha"}, + {Identifier: "b-id", Name: "Beta"}, + }) + })) + defer srv.Close() + client := newTestSDKClient(t, srv) + // Interactive TTY (NonInteractive false) but JSON output requested. WantsJSON() + // is true via JSONMode, so the picker must be skipped. + env := &output.Envelope{Stdout: io.Discard, Stderr: io.Discard, IsTTY: true, JSONMode: true} + require.True(t, env.WantsJSON()) + require.False(t, env.NonInteractive) + + id, err := resolveDeployProject(t.Context(), client, "", env) + require.Error(t, err) + assert.Empty(t, id) + assert.Equal(t, "No project specified", strings.SplitN(err.Error(), "\n", 2)[0]) +} + func TestResolveDeployProject_ZeroProjects(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _ = json.NewEncoder(w).Encode([]sdk.Project{}) From 7760453e5e2f7d7e14502db7cc09a6fc5ff2cc9f Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Tue, 14 Jul 2026 11:31:22 +0200 Subject: [PATCH 3/3] fix(api): make shell-mangled-path hint copy-paste safe CodeRabbit review: the recovery hint printed literal "" placeholders, which Bash parses as input redirection (from a file named "path") if a user copies the command. Use concrete examples ("/projects/my-app") in both recovery lines instead. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/commands/api.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/commands/api.go b/internal/commands/api.go index 40cd74d..f3534ba 100644 --- a/internal/commands/api.go +++ b/internal/commands/api.go @@ -50,9 +50,9 @@ func newAPICmd() *cobra.Command { if isShellMangledPath(path) { return &output.UserError{ Message: "API path was rewritten by your shell (Git Bash/MSYS turned it into a Windows path)", - Hint: "Your shell converted the leading '/' into a filesystem path. Re-run either way:\n" + - fmt.Sprintf(" MSYS_NO_PATHCONV=1 dhq api %s / (disable path conversion)\n", method) + - fmt.Sprintf(" dhq api %s (omit the leading slash, e.g. projects/my-app)", method), + Hint: "Your shell converted the leading '/' into a filesystem path. Re-run either way (substitute your real path):\n" + + fmt.Sprintf(" MSYS_NO_PATHCONV=1 dhq api %s /projects/my-app (disable path conversion)\n", method) + + fmt.Sprintf(" dhq api %s projects/my-app (omit the leading slash)", method), } }