From 9404db41e31a373e6d591139d548918f7335df8e Mon Sep 17 00:00:00 2001 From: Felix Lisczyk <5102728+FelixLisczyk@users.noreply.github.com> Date: Thu, 6 Aug 2026 12:34:57 +0200 Subject: [PATCH 1/3] Fix explicit stdin body handling Make stdin consumption testable and opt-in through the explicit '-' flag value across descriptions, comments, and replies. Align command help and documentation with the safe non-blocking contract and add propagation, edge-case, and help-text regression coverage.\n\nCo-Authored-By: Claude --- README.md | 1 + internal/cli/dependencies.go | 39 ++-- internal/cli/helpers.go | 22 ++- internal/cli/issues.go | 70 +++---- internal/cli/onboard.go | 2 +- internal/cli/projects.go | 22 ++- internal/cli/stdin_commands_test.go | 288 ++++++++++++++++++++++++++++ internal/cli/stdin_help_test.go | 43 +++++ internal/cli/stdin_test.go | 76 ++++++++ internal/skills/linear/SKILL.md | 6 +- plan.md | 145 ++++++++++++++ 11 files changed, 651 insertions(+), 63 deletions(-) create mode 100644 internal/cli/stdin_commands_test.go create mode 100644 internal/cli/stdin_help_test.go create mode 100644 internal/cli/stdin_test.go create mode 100644 plan.md diff --git a/README.md b/README.md index 081cc91..e301844 100644 --- a/README.md +++ b/README.md @@ -534,6 +534,7 @@ linear issues update ENG-123 --attach /tmp/additional-context.png linear issues comment ENG-123 --body "Here's the screenshot:" --attach /tmp/bug.png # Piping content (powerful!) +# Stdin is read only with the explicit - value; surrounding whitespace is trimmed. cat .claude/plans/feature-plan.md | linear issues create "Implementation plan" --team ENG -d - cat prd.md | linear issues create "Feature: OAuth" --team ENG --description - cat bug-report.txt | linear issues comment ENG-123 --body - diff --git a/internal/cli/dependencies.go b/internal/cli/dependencies.go index 32b7bba..ff1d86e 100644 --- a/internal/cli/dependencies.go +++ b/internal/cli/dependencies.go @@ -1,8 +1,11 @@ package cli import ( - "github.com/joa23/linear-cli/pkg/linear" + "io" + "os" + "github.com/joa23/linear-cli/internal/service" + "github.com/joa23/linear-cli/pkg/linear" ) // Dependencies holds all injectable dependencies for CLI commands @@ -11,14 +14,17 @@ type Dependencies struct { // Client is the Linear API client Client *linear.Client + // Stdin is read only when a body or description flag is exactly "-". + Stdin io.Reader + // Services provide business logic and formatting - Issues service.IssueServiceInterface - Cycles service.CycleServiceInterface - Projects service.ProjectServiceInterface - Search service.SearchServiceInterface - Teams service.TeamServiceInterface - Users service.UserServiceInterface - Labels service.LabelServiceInterface + Issues service.IssueServiceInterface + Cycles service.CycleServiceInterface + Projects service.ProjectServiceInterface + Search service.SearchServiceInterface + Teams service.TeamServiceInterface + Users service.UserServiceInterface + Labels service.LabelServiceInterface TaskExport service.TaskExportServiceInterface Attachments service.AttachmentServiceInterface IssueExport service.IssueExportServiceInterface @@ -29,14 +35,15 @@ func NewDependencies(client *linear.Client) *Dependencies { services := service.New(client) return &Dependencies{ - Client: client, - Issues: services.Issues, - Cycles: services.Cycles, - Projects: services.Projects, - Search: services.Search, - Teams: services.Teams, - Users: services.Users, - Labels: services.Labels, + Client: client, + Stdin: os.Stdin, + Issues: services.Issues, + Cycles: services.Cycles, + Projects: services.Projects, + Search: services.Search, + Teams: services.Teams, + Users: services.Users, + Labels: services.Labels, TaskExport: services.TaskExport, Attachments: services.Attachments, IssueExport: services.IssueExport, diff --git a/internal/cli/helpers.go b/internal/cli/helpers.go index ae03d69..274b9cd 100644 --- a/internal/cli/helpers.go +++ b/internal/cli/helpers.go @@ -21,9 +21,14 @@ const ( MaxLimit = 250 ) -// readStdin reads all piped content from stdin +// readStdin reads all content from the process stdin. func readStdin() (string, error) { - reader := bufio.NewReader(os.Stdin) + return readStdinFrom(os.Stdin) +} + +// readStdinFrom reads all content from reader and trims surrounding whitespace. +func readStdinFrom(input io.Reader) (string, error) { + reader := bufio.NewReader(input) var builder strings.Builder for { @@ -60,11 +65,18 @@ func parseCommaSeparated(s string) []string { return result } -// getDescriptionFromFlagOrStdin returns description from flag or stdin -// Use "-" as flagValue to explicitly read from stdin (e.g., -d -) +// getDescriptionFromFlagOrStdin returns the flag value, or reads stdin when the +// flag is exactly "-". Stdin is never read for an ordinary flag value. func getDescriptionFromFlagOrStdin(flagValue string) (string, error) { + return getDescriptionFromFlagOrStdinWithReader(flagValue, os.Stdin) +} + +func getDescriptionFromFlagOrStdinWithReader(flagValue string, input io.Reader) (string, error) { if flagValue == "-" { - return readStdin() + if input == nil { + input = os.Stdin + } + return readStdinFrom(input) } return flagValue, nil diff --git a/internal/cli/issues.go b/internal/cli/issues.go index 9567429..8fd5d92 100644 --- a/internal/cli/issues.go +++ b/internal/cli/issues.go @@ -6,8 +6,8 @@ import ( "strconv" "github.com/joa23/linear-cli/internal/format" - paginationutil "github.com/joa23/linear-cli/pkg/linear/pagination" "github.com/joa23/linear-cli/internal/service" + paginationutil "github.com/joa23/linear-cli/pkg/linear/pagination" "github.com/spf13/cobra" ) @@ -40,21 +40,21 @@ func newIssuesCmd() *cobra.Command { func newIssuesListCmd() *cobra.Command { var ( - teamID string - project string - state string - priority string - assignee string - cycle string - labels string + teamID string + project string + state string + priority string + assignee string + cycle string + labels string excludeLabels string - sortBy string + sortBy string createdSince string createdAfter string createdBefore string - limit int - formatStr string - outputType string + limit int + formatStr string + outputType string ) cmd := &cobra.Command{ @@ -243,7 +243,7 @@ Images in the description (uploads.linear.app/...) require auth — use: # Download a private image from the issue description linear attachments download "https://uploads.linear.app/..."`, - Args: cobra.ExactArgs(1), + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { issueID := args[0] @@ -313,7 +313,10 @@ OUTPUT: On success, prints the new issue's identifier on the FIRST line, then it URL — and nothing else. The description is not echoed back. On failure it exits non-zero. Scripts should use --output json and read .identifier. -TIP: Run 'linear init' first to set default team.`, +TIP: Run 'linear init' first to set default team. + +STDIN: To read a description from a pipe, pass --description -. Stdin is not +read unless the flag value is exactly '-'; surrounding whitespace is trimmed.`, Example: ` # Minimal - create with just title (requires 'linear init') linear issues create "Fix login bug" @@ -373,7 +376,7 @@ TIP: Run 'linear init' first to set default team.`, } // Get description from flag or stdin - desc, err := getDescriptionFromFlagOrStdin(description) + desc, err := getDescriptionFromFlagOrStdinWithReader(description, deps.Stdin) if err != nil { return fmt.Errorf("failed to read description: %w", err) } @@ -452,7 +455,7 @@ TIP: Run 'linear init' first to set default team.`, // Add flags (with short versions for common flags) cmd.Flags().StringVarP(&team, "team", "t", "", TeamFlagDescription) - cmd.Flags().StringVarP(&description, "description", "d", "", "Issue description (or pipe to stdin)") + cmd.Flags().StringVarP(&description, "description", "d", "", "Issue description (use --description - to read from stdin)") cmd.Flags().StringVarP(&state, "state", "s", "", "Workflow state name (e.g., 'In Progress', 'Backlog')") cmd.Flags().StringVarP(&priority, "priority", "p", "", "Priority: 0-4 or none/urgent/high/normal/low") cmd.Flags().Float64VarP(&estimate, "estimate", "e", 0, "Story points estimate") @@ -502,7 +505,10 @@ LABEL MODES: --remove-labels Remove specific labels without affecting others --add-labels and --remove-labels can be used together. - --labels cannot be combined with --add-labels or --remove-labels.`, + --labels cannot be combined with --add-labels or --remove-labels. + +STDIN: To read a description from a pipe, pass --description -. Stdin is not +read unless the flag value is exactly '-'; surrounding whitespace is trimmed.`, Example: ` # Update state and priority linear issues update CEN-123 --state Done --priority 0 @@ -534,7 +540,7 @@ LABEL MODES: return err } -// Get team from flag or config (for cycle resolution) + // Get team from flag or config (for cycle resolution) if team == "" { team = GetDefaultTeam() } @@ -558,7 +564,7 @@ LABEL MODES: } // Get description from flag or stdin - desc, err := getDescriptionFromFlagOrStdin(description) + desc, err := getDescriptionFromFlagOrStdinWithReader(description, deps.Stdin) if err != nil { return fmt.Errorf("failed to read description: %w", err) } @@ -646,7 +652,7 @@ LABEL MODES: // Add flags (with short versions for common flags) cmd.Flags().StringVarP(&title, "title", "T", "", "Update issue title") - cmd.Flags().StringVarP(&description, "description", "d", "", "Update description (or pipe to stdin)") + cmd.Flags().StringVarP(&description, "description", "d", "", "Update description (use --description - to read from stdin)") cmd.Flags().StringVarP(&state, "state", "s", "", "Update workflow state name (e.g., 'In Progress', 'Backlog')") cmd.Flags().StringVarP(&priority, "priority", "p", "", "Priority: 0-4 or none/urgent/high/normal/low") cmd.Flags().StringVarP(&estimate, "estimate", "e", "", "Update story points estimate") @@ -675,15 +681,15 @@ func newIssuesCommentCmd() *cobra.Command { cmd := &cobra.Command{ Use: "comment ", Short: "Add a comment to an issue", - Long: `Add a comment to an issue. Comment body can be provided via --body flag or piped from stdin.`, + Long: `Add a comment to an issue. Provide the body with --body, or pass --body - to read it from stdin. Stdin is not read unless the flag value is exactly '-'. Surrounding whitespace is trimmed.`, Example: ` # Add a simple comment linear issues comment CEN-123 --body "This is a comment" # Comment with screenshot attachment linear issues comment CEN-123 --body "Bug screenshot:" --attach /tmp/screenshot.png - # Pipe content from file - cat notes.md | linear issues comment CEN-123`, + # Pipe content from file (use --body -) + cat notes.md | linear issues comment CEN-123 --body -`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { issueID := args[0] @@ -695,7 +701,7 @@ func newIssuesCommentCmd() *cobra.Command { } // Get body from flag or stdin - commentBody, err := getDescriptionFromFlagOrStdin(body) + commentBody, err := getDescriptionFromFlagOrStdinWithReader(body, deps.Stdin) if err != nil { return fmt.Errorf("failed to read comment body: %w", err) } @@ -709,7 +715,7 @@ func newIssuesCommentCmd() *cobra.Command { } if commentBody == "" { - return fmt.Errorf("comment body is required. Use --body flag or pipe content to stdin") + return fmt.Errorf("comment body is required. Use --body or --body - to read content from stdin") } // Get the issue first to get its ID (comments need issue ID, not identifier) @@ -732,7 +738,7 @@ func newIssuesCommentCmd() *cobra.Command { }, } - cmd.Flags().StringVarP(&body, "body", "b", "", "Comment body (or pipe to stdin)") + cmd.Flags().StringVarP(&body, "body", "b", "", "Comment body (use --body - to read from stdin)") cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Embed file as inline image in body (repeatable); for sidebar cards use: attachments create") return cmd @@ -864,15 +870,15 @@ func newIssuesReplyCmd() *cobra.Command { cmd := &cobra.Command{ Use: "reply ", Short: "Reply to a comment", - Long: `Reply to an existing comment on an issue. Reply body can be provided via --body flag or piped from stdin.`, + Long: `Reply to an existing comment on an issue. Provide the body with --body, or pass --body - to read it from stdin. Stdin is not read unless the flag value is exactly '-'. Surrounding whitespace is trimmed.`, Example: ` # Reply to a comment linear issues reply CEN-123 abc-comment-id --body "Thanks for the feedback!" # Reply with attachment linear issues reply CEN-123 abc-comment-id --body "Here's the fix:" --attach /tmp/screenshot.png - # Pipe content from file - cat response.md | linear issues reply CEN-123 abc-comment-id`, + # Pipe content from file (use --body -) + cat response.md | linear issues reply CEN-123 abc-comment-id --body -`, Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { issueID := args[0] @@ -883,7 +889,7 @@ func newIssuesReplyCmd() *cobra.Command { } // Get body from flag or stdin - replyBody, err := getDescriptionFromFlagOrStdin(body) + replyBody, err := getDescriptionFromFlagOrStdinWithReader(body, deps.Stdin) if err != nil { return fmt.Errorf("failed to read reply body: %w", err) } @@ -897,7 +903,7 @@ func newIssuesReplyCmd() *cobra.Command { } if replyBody == "" { - return fmt.Errorf("reply body is required. Use --body flag or pipe content to stdin") + return fmt.Errorf("reply body is required. Use --body or --body - to read content from stdin") } comment, err := deps.Issues.ReplyToComment(issueID, commentID, replyBody) @@ -912,7 +918,7 @@ func newIssuesReplyCmd() *cobra.Command { }, } - cmd.Flags().StringVarP(&body, "body", "b", "", "Reply body (or pipe to stdin)") + cmd.Flags().StringVarP(&body, "body", "b", "", "Reply body (use --body - to read from stdin)") cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Embed file as inline image in body (repeatable); for sidebar cards use: attachments create") return cmd diff --git a/internal/cli/onboard.go b/internal/cli/onboard.go index cc4466e..af82128 100644 --- a/internal/cli/onboard.go +++ b/internal/cli/onboard.go @@ -112,7 +112,7 @@ func runOnboard() error { fmt.Println(" linear auth status Check login status") fmt.Println() fmt.Println("Create issue (full example):") - fmt.Println(" cat feature.md | linear i create \"Add user authentication\" \\") + fmt.Println(" cat feature.md | linear i create \"Add user authentication\" --description - \\") fmt.Println(" -t CEN \\") fmt.Println(" -s \"In Progress\" \\") fmt.Println(" -p 1 \\") diff --git a/internal/cli/projects.go b/internal/cli/projects.go index 376beed..68436ea 100644 --- a/internal/cli/projects.go +++ b/internal/cli/projects.go @@ -166,12 +166,15 @@ func newProjectsCreateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "create ", Short: "Create a new project", - Long: `Create a new project. States: planned, started, paused, completed, canceled.`, + Long: `Create a new project. States: planned, started, paused, completed, canceled. + +STDIN: To read a description from a pipe, pass --description -. Stdin is not +read unless the flag value is exactly '-'; surrounding whitespace is trimmed.`, Example: ` # Create a simple project linear projects create "Q1 Release" --team CEN # Create with description from stdin - cat project-spec.md | linear projects create "Q1 Release" --team CEN + cat project-spec.md | linear projects create "Q1 Release" --team CEN --description - # Create with all options linear projects create "Q1 Release" --team CEN --state started --lead Stefan`, @@ -192,7 +195,7 @@ func newProjectsCreateCmd() *cobra.Command { } // Get description from flag or stdin - desc, err := getDescriptionFromFlagOrStdin(description) + desc, err := getDescriptionFromFlagOrStdinWithReader(description, deps.Stdin) if err != nil { return fmt.Errorf("failed to read description: %w", err) } @@ -235,7 +238,7 @@ func newProjectsCreateCmd() *cobra.Command { // Add flags (with short versions for common flags) cmd.Flags().StringVarP(&team, "team", "t", "", TeamFlagDescription) - cmd.Flags().StringVarP(&description, "description", "d", "", "Project description (or pipe to stdin)") + cmd.Flags().StringVarP(&description, "description", "d", "", "Project description (use --description - to read from stdin)") cmd.Flags().StringVarP(&state, "state", "s", "", "Project state: planned, started, paused, completed, canceled") cmd.Flags().StringVarP(&lead, "lead", "l", "", "Project lead name (use 'me' for yourself)") cmd.Flags().StringVar(&startDate, "start-date", "", "Start date YYYY-MM-DD") @@ -257,7 +260,10 @@ func newProjectsUpdateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "update ", Short: "Update an existing project", - Long: `Update an existing project. Only provided flags are changed.`, + Long: `Update an existing project. Only provided flags are changed. + +STDIN: To read a description from a pipe, pass --description -. Stdin is not +read unless the flag value is exactly '-'; surrounding whitespace is trimmed.`, Example: ` # Update project state linear projects update PROJ-123 --state completed @@ -265,7 +271,7 @@ func newProjectsUpdateCmd() *cobra.Command { linear projects update PROJ-123 --lead john@example.com # Update description from stdin - cat updated-spec.md | linear projects update PROJ-123`, + cat updated-spec.md | linear projects update PROJ-123 --description -`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { projectID := args[0] @@ -283,7 +289,7 @@ func newProjectsUpdateCmd() *cobra.Command { } // Get description from flag or stdin - desc, err := getDescriptionFromFlagOrStdin(description) + desc, err := getDescriptionFromFlagOrStdinWithReader(description, deps.Stdin) if err != nil { return fmt.Errorf("failed to read description: %w", err) } @@ -327,7 +333,7 @@ func newProjectsUpdateCmd() *cobra.Command { // Add flags (with short versions for common flags) cmd.Flags().StringVarP(&name, "name", "n", "", "Update project name") - cmd.Flags().StringVarP(&description, "description", "d", "", "Update description (or pipe to stdin)") + cmd.Flags().StringVarP(&description, "description", "d", "", "Update description (use --description - to read from stdin)") cmd.Flags().StringVarP(&state, "state", "s", "", "Update state: planned, started, paused, completed, canceled") cmd.Flags().StringVarP(&lead, "lead", "l", "", "Update project lead (use 'me' for yourself)") cmd.Flags().StringVar(&startDate, "start-date", "", "Update start date YYYY-MM-DD") diff --git a/internal/cli/stdin_commands_test.go b/internal/cli/stdin_commands_test.go new file mode 100644 index 0000000..d11ec13 --- /dev/null +++ b/internal/cli/stdin_commands_test.go @@ -0,0 +1,288 @@ +package cli + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/joa23/linear-cli/internal/format" + "github.com/joa23/linear-cli/internal/service" + "github.com/joa23/linear-cli/pkg/linear" + "github.com/joa23/linear-cli/pkg/linear/core" +) + +func TestIssueDescriptionFromExplicitStdinReachesService(t *testing.T) { + issues := &recordingIssueService{} + deps := &Dependencies{Issues: issues, Stdin: strings.NewReader(" issue body\n")} + cmd := NewCmdWithDeps(deps, newIssuesCreateCmd) + cmd.SetArgs([]string{"New issue", "--team", "CEN", "--description", "-"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if issues.created == nil || issues.created.Description != "issue body" { + t.Fatalf("created description = %#v, want %q", issues.created, "issue body") + } +} + +func TestIssueUpdateDescriptionFromExplicitStdinReachesService(t *testing.T) { + issues := &recordingIssueService{} + deps := &Dependencies{Issues: issues, Stdin: strings.NewReader(" updated body\n")} + cmd := NewCmdWithDeps(deps, newIssuesUpdateCmd) + cmd.SetArgs([]string{"CEN-123", "--description", "-"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if issues.updated == nil || issues.updated.Description == nil || *issues.updated.Description != "updated body" { + t.Fatalf("updated description = %#v, want %q", issues.updated, "updated body") + } +} + +func TestProjectDescriptionFromExplicitStdinReachesService(t *testing.T) { + tests := []struct { + name string + create bool + }{ + {name: "create", create: true}, + {name: "update", create: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + projects := &recordingProjectService{} + deps := &Dependencies{Projects: projects, Stdin: strings.NewReader(" project body\n")} + factory := newProjectsUpdateCmd + args := []string{"PROJ-123", "--description", "-"} + if tt.create { + factory = newProjectsCreateCmd + args = []string{"New project", "--team", "CEN", "--description", "-"} + } + + cmd := NewCmdWithDeps(deps, factory) + cmd.SetArgs(args) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tt.create { + if projects.created == nil || projects.created.Description != "project body" { + t.Fatalf("created description = %#v, want %q", projects.created, "project body") + } + } else if projects.updated == nil || projects.updated.Description == nil || *projects.updated.Description != "project body" { + t.Fatalf("updated description = %#v, want %q", projects.updated, "project body") + } + }) + } +} + +func TestReplyBodyFromExplicitStdinReachesService(t *testing.T) { + issues := &recordingIssueService{} + deps := &Dependencies{Issues: issues, Stdin: strings.NewReader(" reply body\n")} + cmd := NewCmdWithDeps(deps, newIssuesReplyCmd) + cmd.SetArgs([]string{"CEN-123", "comment-123", "--body", "-"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if issues.replyBody != "reply body" { + t.Fatalf("reply body = %q, want %q", issues.replyBody, "reply body") + } +} + +func TestCommentBodyFromExplicitStdinReachesAPI(t *testing.T) { + transport := &recordingCommentTransport{} + client := linear.NewClient("test-token") + client.GetBase().SetHTTPClient(&http.Client{Transport: transport}) + deps := &Dependencies{Client: client, Stdin: strings.NewReader(" comment body\n")} + cmd := NewCmdWithDeps(deps, newIssuesCommentCmd) + cmd.SetArgs([]string{"CEN-123", "--body", "-"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if transport.body != "comment body" { + t.Fatalf("comment body = %q, want %q", transport.body, "comment body") + } +} + +func TestOrdinaryDescriptionDoesNotReadStdinAtCommandBoundary(t *testing.T) { + issues := &recordingIssueService{} + deps := &Dependencies{Issues: issues, Stdin: errorReader{err: fmt.Errorf("stdin should not be read")}} + cmd := NewCmdWithDeps(deps, newIssuesCreateCmd) + cmd.SetArgs([]string{"New issue", "--team", "CEN", "--description", "literal body"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if issues.created == nil || issues.created.Description != "literal body" { + t.Fatalf("created description = %#v, want %q", issues.created, "literal body") + } +} + +func TestExplicitEmptyStdinPreservesExistingSemantics(t *testing.T) { + issues := &recordingIssueService{} + deps := &Dependencies{Issues: issues, Stdin: strings.NewReader("")} + cmd := NewCmdWithDeps(deps, newIssuesUpdateCmd) + cmd.SetArgs([]string{"CEN-123", "--description", "-"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if issues.updated == nil || issues.updated.Description != nil { + t.Fatalf("empty update description = %#v, want nil", issues.updated) + } +} + +func TestExplicitStdinReadErrorsAreWrapped(t *testing.T) { + issues := &recordingIssueService{} + deps := &Dependencies{Issues: issues, Stdin: errorReader{err: fmt.Errorf("stdin failed")}} + cmd := NewCmdWithDeps(deps, newIssuesCreateCmd) + cmd.SetArgs([]string{"New issue", "--team", "CEN", "--description", "-"}) + + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "failed to read description: stdin failed") { + t.Fatalf("got error %v, want contextual stdin error", err) + } + if issues.created != nil { + t.Fatal("created issue after stdin read error") + } +} + +func TestEmptyStdinRejectsCommentAndReply(t *testing.T) { + tests := []struct { + name string + args []string + check func(*recordingIssueService) bool + }{ + {name: "comment", args: []string{"CEN-123", "--body", "-"}, check: func(issues *recordingIssueService) bool { + return issues.replyBody == "" + }}, + {name: "reply", args: []string{"CEN-123", "comment-123", "--body", "-"}, check: func(issues *recordingIssueService) bool { + return issues.replyBody == "" + }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + issues := &recordingIssueService{} + client := linear.NewClient("test-token") + deps := &Dependencies{Client: client, Issues: issues, Stdin: strings.NewReader("")} + factory := newIssuesReplyCmd + if tt.name == "comment" { + factory = newIssuesCommentCmd + } + cmd := NewCmdWithDeps(deps, factory) + cmd.SetArgs(tt.args) + if err := cmd.Execute(); err == nil { + t.Fatal("expected empty body error") + } + if !tt.check(issues) { + t.Fatal("empty body reached service") + } + }) + } +} + +type recordingIssueService struct { + created *service.CreateIssueInput + updated *service.UpdateIssueInput + replyBody string +} + +func (s *recordingIssueService) Get(string, format.Format) (string, error) { return "", nil } +func (s *recordingIssueService) GetWithOutput(string, format.Verbosity, format.OutputType) (string, error) { + return "", nil +} +func (s *recordingIssueService) Search(*service.SearchFilters) (string, error) { return "", nil } +func (s *recordingIssueService) SearchWithOutput(*service.SearchFilters, format.Verbosity, format.OutputType) (string, error) { + return "", nil +} +func (s *recordingIssueService) ListAssigned(int, format.Format) (string, error) { return "", nil } +func (s *recordingIssueService) ListAssignedWithPagination(*core.PaginationInput) (string, error) { + return "", nil +} +func (s *recordingIssueService) Create(input *service.CreateIssueInput, _ format.OutputType) (string, error) { + s.created = input + return "created", nil +} +func (s *recordingIssueService) Update(_ string, input *service.UpdateIssueInput) (string, error) { + s.updated = input + return "updated", nil +} +func (s *recordingIssueService) GetComments(string) (string, error) { return "", nil } +func (s *recordingIssueService) AddComment(string, string) (string, error) { return "", nil } +func (s *recordingIssueService) ReplyToComment(_ string, _ string, body string) (*core.Comment, error) { + s.replyBody = body + return &core.Comment{ID: "reply-id", User: core.User{Name: "Test User"}}, nil +} +func (s *recordingIssueService) AddReaction(string, string) error { return nil } +func (s *recordingIssueService) GetIssueID(string) (string, error) { return "", nil } + +type recordingProjectService struct { + created *service.CreateProjectInput + updated *service.UpdateProjectInput +} + +func (s *recordingProjectService) Get(string) (string, error) { return "", nil } +func (s *recordingProjectService) GetWithOutput(string, format.Verbosity, format.OutputType) (string, error) { + return "", nil +} +func (s *recordingProjectService) ListAll(int) (string, error) { return "", nil } +func (s *recordingProjectService) ListAllWithOutput(int, format.Verbosity, format.OutputType) (string, error) { + return "", nil +} +func (s *recordingProjectService) ListByTeam(string, int) (string, error) { return "", nil } +func (s *recordingProjectService) ListByTeamWithOutput(string, int, format.Verbosity, format.OutputType) (string, error) { + return "", nil +} +func (s *recordingProjectService) ListUserProjects(int) (string, error) { return "", nil } +func (s *recordingProjectService) ListUserProjectsWithOutput(int, format.Verbosity, format.OutputType) (string, error) { + return "", nil +} +func (s *recordingProjectService) Create(input *service.CreateProjectInput) (string, error) { + s.created = input + return "created", nil +} +func (s *recordingProjectService) Update(_ string, input *service.UpdateProjectInput) (string, error) { + s.updated = input + return "updated", nil +} + +type recordingCommentTransport struct { + body string +} + +func (t *recordingCommentTransport) RoundTrip(req *http.Request) (*http.Response, error) { + payload, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + var request struct { + Query string `json:"query"` + Variables map[string]interface{} `json:"variables"` + } + if err := json.Unmarshal(payload, &request); err != nil { + return nil, err + } + + response := `{"data":{"issue":{"id":"issue-id","identifier":"CEN-123","title":"Test issue"}}}` + if strings.Contains(request.Query, "commentCreate") { + t.body, _ = request.Variables["body"].(string) + response = `{"data":{"commentCreate":{"success":true,"comment":{"id":"comment-id","body":"comment body","user":{"id":"user-id","name":"Test User","email":"test@example.com"},"issue":{"id":"issue-id","identifier":"CEN-123"}}}}}` + } + + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: io.NopCloser(bytes.NewBufferString(response)), + Request: req, + }, nil +} + +var _ service.IssueServiceInterface = (*recordingIssueService)(nil) +var _ service.ProjectServiceInterface = (*recordingProjectService)(nil) diff --git a/internal/cli/stdin_help_test.go b/internal/cli/stdin_help_test.go new file mode 100644 index 0000000..0a50504 --- /dev/null +++ b/internal/cli/stdin_help_test.go @@ -0,0 +1,43 @@ +package cli + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func TestBodyCommandHelpRequiresExplicitStdinFlag(t *testing.T) { + tests := []struct { + name string + cmd func() *cobra.Command + want string + }{ + {name: "issue create", cmd: newIssuesCreateCmd, want: "--description -"}, + {name: "issue update", cmd: newIssuesUpdateCmd, want: "--description -"}, + {name: "project create", cmd: newProjectsCreateCmd, want: "--description -"}, + {name: "project update", cmd: newProjectsUpdateCmd, want: "--description -"}, + {name: "comment", cmd: newIssuesCommentCmd, want: "--body -"}, + {name: "reply", cmd: newIssuesReplyCmd, want: "--body -"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := tt.cmd() + var output bytes.Buffer + cmd.SetOut(&output) + cmd.SetErr(&output) + cmd.SetArgs([]string{"--help"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(output.String(), tt.want) { + t.Fatalf("help output does not contain %q:\n%s", tt.want, output.String()) + } + if strings.Contains(output.String(), "or pipe to stdin") || strings.Contains(output.String(), "piped from stdin") { + t.Fatalf("help output still advertises implicit stdin:\n%s", output.String()) + } + }) + } +} diff --git a/internal/cli/stdin_test.go b/internal/cli/stdin_test.go new file mode 100644 index 0000000..ca4d3fe --- /dev/null +++ b/internal/cli/stdin_test.go @@ -0,0 +1,76 @@ +package cli + +import ( + "errors" + "io" + "strings" + "testing" +) + +func TestDescriptionFromFlagOrStdin(t *testing.T) { + tests := []struct { + name string + flagValue string + input string + want string + }{ + {name: "literal value bypasses stdin", flagValue: "description", input: "unused", want: "description"}, + {name: "explicit stdin trims whitespace", flagValue: "-", input: " \n description\n\n", want: "description"}, + {name: "explicit empty stdin remains empty", flagValue: "-", input: "", want: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := &trackingReader{Reader: strings.NewReader(tt.input)} + got, err := getDescriptionFromFlagOrStdinWithReader(tt.flagValue, reader) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Fatalf("got %q, want %q", got, tt.want) + } + if tt.flagValue != "-" && reader.read { + t.Fatal("literal flag value consumed stdin") + } + if tt.flagValue == "-" && !reader.read { + t.Fatal("explicit stdin flag did not read stdin") + } + }) + } +} + +func TestReadStdinFromReturnsReaderError(t *testing.T) { + wantErr := errors.New("read failed") + _, err := readStdinFrom(errorReader{err: wantErr}) + if !errors.Is(err, wantErr) { + t.Fatalf("got error %v, want %v", err, wantErr) + } +} + +func TestReadStdinFromReadsFinalLineWithoutNewline(t *testing.T) { + got, err := readStdinFrom(strings.NewReader("first\nsecond")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "first\nsecond" { + t.Fatalf("got %q, want %q", got, "first\nsecond") + } +} + +type trackingReader struct { + io.Reader + read bool +} + +func (r *trackingReader) Read(p []byte) (int, error) { + r.read = true + return r.Reader.Read(p) +} + +type errorReader struct { + err error +} + +func (r errorReader) Read([]byte) (int, error) { + return 0, r.err +} diff --git a/internal/skills/linear/SKILL.md b/internal/skills/linear/SKILL.md index d771f32..31a684b 100644 --- a/internal/skills/linear/SKILL.md +++ b/internal/skills/linear/SKILL.md @@ -226,7 +226,11 @@ cat spec.md | linear i create "Feature title" --team CEN -d - ## Piping Support (Powerful!) -**All description and body flags support stdin via `-`:** +**Description and body flags support stdin only when their value is `-`:** + +Pass `--description -` (or `-d -`) for issue and project descriptions, and +`--body -` (or `-b -`) for comments and replies. An unadorned pipe is not read; +surrounding whitespace is trimmed. ```bash # Pipe Claude plan into ticket description diff --git a/plan.md b/plan.md new file mode 100644 index 0000000..0f54d36 --- /dev/null +++ b/plan.md @@ -0,0 +1,145 @@ +# Implementation Plan + +## Task Overview +- **Source**: TL-561 +- **Title**: Make CLI stdin body handling explicit and regression-tested +- **Description**: `linear issues create/update`, project create/update, and issue comment/reply commands currently describe implicit piped stdin even though stdin is only read when the corresponding flag value is explicitly `-`. The implementation must make that contract unambiguous, preserve the historical non-blocking behavior, and add coverage proving body values reach the service/API inputs. + +## Requirements Analysis +- **Core Functionality**: + - Standardize the documented convention: `--description -` for issue/project descriptions and `--body -` for comments/replies. + - Keep stdin consumption opt-in only; do not restore automatic TTY/non-TTY detection. + - Preserve ordinary flag values and current `strings.TrimSpace` handling of explicit stdin. + - Preserve current empty-input behavior, including the existing update/clearing semantics, unless tests reveal only documentation or helper behavior needs clarification. + - Ensure explicit stdin content reaches issue create/update, project create/update, comment, and reply inputs. +- **Acceptance Criteria**: + - Every affected command's help text and examples explicitly show the `-` value and no longer imply that an unadorned pipe is consumed. + - README and embedded CLI skill documentation contain no stale implicit-pipe examples or wording. + - Tests cover the shared resolver, whitespace trimming, empty input, explicit propagation through all affected command surfaces, and the non-consuming implicit path. + - Existing ordinary `--description `/`--body `, attachments, and service-layer behavior remain intact. +- **Technical Constraints**: + - `readStdin` currently reads `os.Stdin` directly and trims leading/trailing whitespace. + - A prior fix (commit `2a47758`) intentionally removed automatic pipe detection because open stdin could make updates hang in CI, Claude Code, or scripts; this must not regress. + - Tests should not depend on mutating process-global stdin or on a live Linear API. + - No service or GraphQL schema change is required; the defect is at CLI input resolution and documentation. +- **Integration Points**: + - Shared helper in `internal/cli/helpers.go` is used by issue/project descriptions and issue comment/reply bodies. + - Cobra command closures in `internal/cli/issues.go` and `internal/cli/projects.go` assemble service inputs. + - `Dependencies`, `NewCmdWithDeps`, and the service interfaces provide test injection; the raw client path used by comments may require an HTTP test transport or a narrowly scoped test seam. + - README and `internal/skills/linear/SKILL.md` are user-facing documentation surfaces to audit and update. + +## Codebase Analysis +- **Existing Patterns**: + - `readStdin` uses a `bufio.Reader` over `os.Stdin` and returns `strings.TrimSpace(builder.String())` (`internal/cli/helpers.go:24-41`). + - `getDescriptionFromFlagOrStdin` reads only for the exact flag value `"-"` (`helpers.go:63-71`); all affected commands call it before building service inputs. + - Issue create resolves the body before attachment processing and passes it as `CreateIssueInput.Description` (`issues.go:375-394`); issue update does the same for `UpdateIssueInput.Description` (`issues.go:560-582`). + - Project create/update follow the same resolver pattern (`projects.go:194-205`, `projects.go:285-299`). Reply already uses the injectable `IssueServiceInterface`; comment currently resolves the issue and creates the comment through `deps.Client` (`issues.go:697-725`). + - Existing tests are table-driven Go tests, but `helpers_test.go` currently covers only date parsing. `NewCmdWithDeps` injects command dependencies, while service interfaces use hand-written test doubles in existing service tests. +- **Available Infrastructure**: + - `Dependencies` exposes issue/project/user services and the Linear client (`internal/cli/dependencies.go`). + - `pkg/linear/core` supports test HTTP clients/transports, and `pkg/linear/testutil` contains mock transport helpers for API-level assertions. + - `make test` is the repository-wide verification command. +- **Dependencies**: + - Likely changes are confined to CLI helpers, affected Cobra commands, CLI tests, and documentation. Service input types and GraphQL mutations should remain unchanged. +- **Architecture Notes**: + - The explicit `-` check is the safety boundary: the implementation should make this boundary clearer rather than infer intent from stdin state. + - Extract a pure `readStdinFrom(io.Reader)` helper and keep `readStdin()` as the production wrapper over `os.Stdin`; this gives tests isolated readers without a mutable package-global override. + - Use `NewCmdWithDeps` and complete test doubles for the issue/project service interfaces. Tests must pass explicit `--team` values so they do not depend on local config. For comment, preserve the existing output and raw-client path and use a test-local sequential recording `http.RoundTripper` that handles issue resolution and comment mutation responses while capturing the mutation variables; do not broaden public APIs. + +## Implementation Strategy +- **Problem Complexity**: Moderate, cross-cutting CLI contract/documentation bug with a small behavior surface and a significant regression-testing gap. +- **Core Problem**: The code already implements safe explicit stdin handling, but help and examples promise a different interface and lack tests proving the safe path is used consistently. +- **Approach**: Preserve the exact-`-` behavior and trimming policy, make the stdin reader testable, update all affected help/docs/error text to state the explicit convention, then add command-level propagation tests through dependency injection and a bounded non-consumption test. +- **Testing Approach**: Automated unit and CLI-level tests, followed by the full Go test suite. No manual GUI verification is relevant. +- **Phases**: + 1. **Stabilize and test the shared stdin contract** + - **Implementation**: + - Extract a pure `readStdinFrom(io.Reader)` helper and keep the production `readStdin()` wrapper sourced from `os.Stdin`. + - Keep `getDescriptionFromFlagOrStdin` exact-match behavior (`"-"` reads; all other values return unchanged) and retain `strings.TrimSpace`. + - Add helper tests for ordinary flag values, explicit non-empty stdin, leading/trailing whitespace and final newlines, empty stdin, reader errors, and a reader that records whether it was touched. + - **Verification**: + - Run the focused CLI helper tests. Confirm explicit `-` returns trimmed content, empty stdin remains empty, ordinary values bypass the reader, and no implicit path attempts a read. + 2. **Cover propagation through affected commands** + - **Implementation**: + - Add command-level tests using `NewCmdWithDeps`, complete issue/project service test doubles, and explicit `--team` values to capture `CreateIssueInput.Description`, `UpdateIssueInput.Description`, `CreateProjectInput.Description`, and `UpdateProjectInput.Description` for explicit stdin. + - Cover reply body propagation through the issue service fake. + - Cover comment body propagation without changing its current output path: use a test-local sequential recording `http.RoundTripper` that responds to the issue lookup and comment mutation and asserts the exact comment body variable. + - Add bounded non-blocking coverage that executes commands with ordinary flags and a reader that would fail/block if consumed; verify service calls still occur without reading unrelated stdin. The shared helper test is the primary proof of the no-read branch; command coverage should only be added where it exercises a distinct command path. + - Assert explicit stdin read errors are returned with the command's existing contextual error wrapping. + - Define empty-input compatibility precisely: the helper returns `""`; create inputs carry an empty description; update inputs retain the current nil `Description` behavior while `description == "-"` still passes the existing update gate; comment/reply commands reject empty bodies before lookup/mutation. Do not introduce description clearing. + - Assert ordinary literal flag values continue to pass through unchanged and that stdin-resolved text remains the body before attachment appending. + - **Verification**: + - Run all new CLI tests. Confirm each affected command forwards explicit stdin content and the implicit form does not consume stdin or silently claim to have accepted a body. + 3. **Correct user-facing help and documentation** + - **Implementation**: + - Update issue create/update, comment, and reply long descriptions, flag descriptions, examples, and required-body error messages in `internal/cli/issues.go` to say stdin is read only with `--description -` or `--body -`; specifically fix the unadorned `comment` and `reply` examples. + - Update project create/update examples and flag descriptions in `internal/cli/projects.go`, specifically replacing the unadorned project create/update pipe examples with explicit `--description -` examples. + - Audit `internal/cli/onboard.go` for its stale pipe example and correct it as well. + - Audit README piping and command examples for the same surfaces. Preserve examples that already use explicit `-` (including the current README issue/comment/reply forms), changing only stale or ambiguous wording and adding the whitespace policy where the stdin contract is documented. + - Audit `internal/skills/linear/SKILL.md` similarly; retain its already-correct explicit `-` examples unless the audit finds wording that still implies implicit reading. Search for command-shaped unadorned pipelines as well as `pipe to stdin` text before finishing. + - **Verification**: + - Run help/documentation-focused tests if added (including assertions against command help text), then inspect targeted grep results to ensure no affected command advertises implicit stdin. Run `make test` after documentation and command changes. + 4. **Final regression verification** + - **Implementation**: + - Review the diff for unchanged attachment handling, ordinary flag behavior, empty-input semantics, and the historical no-auto-detection constraint. + - Add or adjust comments only where they accurately describe explicit `-` behavior and trimming. + - **Verification**: + - Run `make test` from a clean working state. Confirm all tests pass and the final search/help checks show a single consistent stdin contract. + +**Phase Verification Approach**: Each phase ends with automated verification before the next begins. Tests are added after the corresponding implementation within each phase, using injected readers, service fakes, and deterministic HTTP transport rather than a live API. The final phase requires the complete repository test suite to pass. + +## Quality Assurance Plan +- **Testing Strategy**: + - Table-driven helper tests for the resolver and whitespace/empty-input policy. + - CLI execution tests for issue/project create/update and reply using `NewCmdWithDeps`. + - Deterministic client/transport coverage for comment request body propagation. + - Help-text assertions or focused string checks for explicit flag syntax, plus the full `make test` suite. +- **Edge Cases**: + - Literal body/description values other than `-` must not trigger stdin. + - Explicit stdin with leading/trailing spaces and final newline is trimmed exactly as today. + - Explicit stdin read errors are surfaced with the existing helper/command context. + - Empty stdin returns an empty string; create forwards empty description, update keeps `Description` nil while retaining the `"-"` update gate, and comment/reply fail before issue lookup or mutation. No new clearing behavior is introduced. + - Commands with unrelated open stdin must not block or consume it. + - Attachment appending must still work after body resolution, with the resolved body retained before the attachment markdown is added. + - Issue update's `description == "-"` must still count as an explicitly requested update even when the resulting body is empty, without inventing a new clearing behavior. +- **Regression Prevention**: + - Keep the exact-`-` condition in one shared helper. + - Test both the positive read path and the negative no-read path. + - Search all affected help/docs surfaces for stale implicit-pipe language. + - Preserve service interfaces and GraphQL inputs unless a narrowly scoped testability refactor is proven necessary. +- **Success Verification**: + - `make test` passes. + - Explicit stdin examples work consistently for all six command paths (issue create/update, project create/update, comment, reply). + - Ordinary values and non-body commands do not read stdin. + - Help, README, and embedded skill guidance all require the explicit `-` convention. + +## Development Environment +- **Setup Requirements**: Go toolchain and repository dependencies already defined by the project; no external service credentials or live Linear workspace are needed. +- **Debugging Strategy**: Capture service inputs and GraphQL request payloads in test doubles/transports; use focused test runs while iterating, then `make test`. +- **Iteration Approach**: Implement one phase at a time, run its focused tests, then proceed only after verification; finish with repository-wide tests and targeted documentation search. + +## Risk Assessment +- **Potential Issues**: + - Process-global stdin can make tests flaky or hang if the seam is incomplete. + - Comment command's direct client access may make body capture harder than the service-backed paths. + - Help wording can remain inconsistent in an overlooked README or embedded skill example. + - Changing empty stdin handling accidentally could alter description-clearing behavior. +- **Mitigation Strategies**: + - Use the pure reader helper rather than mutable package-global test state; keep command tests serial only where the existing command globals require it. + - Use a test-local sequential recording `http.RoundTripper` for the comment path so issue lookup and comment mutation receive distinct deterministic responses and the mutation body can be asserted; avoid requiring a live API or changing the production base URL. + - Complete every method on the issue/project service interfaces in test doubles, and pass explicit team flags to avoid machine-local configuration. + - Search all source/docs for `pipe`, `stdin`, `--description`, and `--body`, including unadorned command-shaped pipelines and onboarding examples, and add help assertions for affected commands. + - Treat whitespace, read errors, and empty-input behavior as explicit compatibility cases before editing command assembly. +- **Backup Approaches**: + - If the recording transport is too brittle, keep production comment code unchanged and isolate the GraphQL request sequence in a focused test helper that still asserts the mutation variables. + - If complete interface fakes become noisy, define test-only adapters that delegate unneeded methods to zero-value error implementations while keeping the captured methods explicit; do not weaken production interfaces or use a live API. + +## Files Likely to Change +- `internal/cli/helpers.go` - Add the internal stdin reader seam and clarify the explicit-`-` helper contract without changing production behavior. +- `internal/cli/helpers_test.go` - Add whitespace, empty-input, ordinary-value, explicit-read, and non-read helper tests. +- `internal/cli/issues.go` - Correct issue description/body help, examples, and errors; retain explicit stdin behavior. +- `internal/cli/projects.go` - Correct project description help and examples. +- `internal/cli/onboard.go` - Correct the stale onboarding pipeline example. +- `internal/cli/*_test.go` - Add command-level fakes/transport tests for issue, project, comment, and reply body propagation, read errors, and bounded non-consumption. +- `README.md` - Remove stale implicit-pipe examples and document explicit `--description -`/`--body -` usage and trimming policy. +- `internal/skills/linear/SKILL.md` - Audit and align embedded CLI stdin guidance. From 6bc06de1d3390a1c42bd8665ba9c12cd5ef75407 Mon Sep 17 00:00:00 2001 From: Felix Lisczyk <5102728+FelixLisczyk@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:00:50 +0200 Subject: [PATCH 2/3] Expand explicit stdin verification coverage Add empty-input, attachment propagation, and no-request tests for CLI body handling. Make the injected stdin seam deterministic when unconfigured and document the actual helper entry points. Co-Authored-By: Claude --- internal/cli/dependencies.go | 1 + internal/cli/doc.go | 33 +++--- internal/cli/helpers.go | 14 ++- internal/cli/stdin_commands_test.go | 156 +++++++++++++++++++++------- internal/cli/stdin_test.go | 7 ++ 5 files changed, 155 insertions(+), 56 deletions(-) diff --git a/internal/cli/dependencies.go b/internal/cli/dependencies.go index ff1d86e..2f8f849 100644 --- a/internal/cli/dependencies.go +++ b/internal/cli/dependencies.go @@ -15,6 +15,7 @@ type Dependencies struct { Client *linear.Client // Stdin is read only when a body or description flag is exactly "-". + // Commands return an error if explicit stdin is requested without a reader. Stdin io.Reader // Services provide business logic and formatting diff --git a/internal/cli/doc.go b/internal/cli/doc.go index 413f92e..477a0e2 100644 --- a/internal/cli/doc.go +++ b/internal/cli/doc.go @@ -25,18 +25,18 @@ has its own file: ## Team Resolution Commands that require team context follow a consistent resolution order: - 1. Explicit --team flag - 2. Default team from .linear.yaml (set via 'linear init') - 3. Error if neither is provided + 1. Explicit --team flag + 2. Default team from .linear.yaml (set via 'linear init') + 3. Error if neither is provided Use [GetDefaultTeam] to retrieve the configured default team. ## Project Resolution Commands that accept a --project flag follow a consistent resolution order: - 1. Explicit --project flag - 2. Default project from .linear.yaml (manually configured) - 3. No project filter (all projects shown) + 1. Explicit --project flag + 2. Default project from .linear.yaml (manually configured) + 3. No project filter (all projects shown) Use [GetDefaultProject] to retrieve the configured default project. @@ -64,12 +64,13 @@ Reusable flag descriptions are centralized in flags.go: ## Helpers Common utilities in helpers.go: - - [readStdin] - Read from stdin - - [parseCommaSeparated] - Parse comma-separated values - - [getDescriptionFromFlagOrStdin] - Get text from flag or stdin (use "-" for stdin) - - [uploadAndAppendAttachments] - Upload files and generate markdown - - [validateAndNormalizeLimit] - Validate --limit flags - - [looksLikeCycleNumber] - Detect numeric cycle IDs + - [readStdinFrom] - Read and trim from an injected reader + - [getDescriptionFromFlagOrStdinWithReader] - Resolve a flag or explicit "-" stdin input + - [getDescriptionFromFlagOrStdin] - Resolve a flag or process stdin for standalone callers + - [parseCommaSeparated] - Parse comma-separated values + - [uploadAndAppendAttachments] - Upload files and generate markdown + - [validateAndNormalizeLimit] - Validate --limit flags + - [looksLikeCycleNumber] - Detect numeric cycle IDs # Dependency Management @@ -105,10 +106,10 @@ Services are obtained via helper functions: # State Management The CLI is stateless between invocations. Each command: - 1. Creates a fresh linear.Client from stored OAuth tokens - 2. Constructs service layer instances as needed - 3. Executes the operation - 4. Exits + 1. Creates a fresh linear.Client from stored OAuth tokens + 2. Constructs service layer instances as needed + 3. Executes the operation + 4. Exits State persisted between runs: - OAuth tokens (via token.Storage in ~/.linear/) diff --git a/internal/cli/helpers.go b/internal/cli/helpers.go index 274b9cd..ac25edd 100644 --- a/internal/cli/helpers.go +++ b/internal/cli/helpers.go @@ -22,12 +22,16 @@ const ( ) // readStdin reads all content from the process stdin. +// Commands should use getDescriptionFromFlagOrStdinWithReader so stdin remains injectable. func readStdin() (string, error) { return readStdinFrom(os.Stdin) } // readStdinFrom reads all content from reader and trims surrounding whitespace. func readStdinFrom(input io.Reader) (string, error) { + if input == nil { + return "", fmt.Errorf("stdin reader is not configured") + } reader := bufio.NewReader(input) var builder strings.Builder @@ -65,17 +69,17 @@ func parseCommaSeparated(s string) []string { return result } -// getDescriptionFromFlagOrStdin returns the flag value, or reads stdin when the -// flag is exactly "-". Stdin is never read for an ordinary flag value. +// getDescriptionFromFlagOrStdin returns the flag value, or reads process stdin when +// the flag is exactly "-". Commands use the reader-injected variant below. func getDescriptionFromFlagOrStdin(flagValue string) (string, error) { return getDescriptionFromFlagOrStdinWithReader(flagValue, os.Stdin) } +// getDescriptionFromFlagOrStdinWithReader resolves command body input through an +// injected reader. A nil reader is an invalid dependency and returns an error; +// it never falls back to process stdin. func getDescriptionFromFlagOrStdinWithReader(flagValue string, input io.Reader) (string, error) { if flagValue == "-" { - if input == nil { - input = os.Stdin - } return readStdinFrom(input) } diff --git a/internal/cli/stdin_commands_test.go b/internal/cli/stdin_commands_test.go index d11ec13..5921fa5 100644 --- a/internal/cli/stdin_commands_test.go +++ b/internal/cli/stdin_commands_test.go @@ -6,6 +6,9 @@ import ( "fmt" "io" "net/http" + "net/http/httptest" + "os" + "path/filepath" "strings" "testing" @@ -13,6 +16,7 @@ import ( "github.com/joa23/linear-cli/internal/service" "github.com/joa23/linear-cli/pkg/linear" "github.com/joa23/linear-cli/pkg/linear/core" + "github.com/spf13/cobra" ) func TestIssueDescriptionFromExplicitStdinReachesService(t *testing.T) { @@ -33,7 +37,7 @@ func TestIssueUpdateDescriptionFromExplicitStdinReachesService(t *testing.T) { issues := &recordingIssueService{} deps := &Dependencies{Issues: issues, Stdin: strings.NewReader(" updated body\n")} cmd := NewCmdWithDeps(deps, newIssuesUpdateCmd) - cmd.SetArgs([]string{"CEN-123", "--description", "-"}) + cmd.SetArgs([]string{"CEN-123", "--team", "CEN", "--description", "-"}) if err := cmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -128,7 +132,7 @@ func TestExplicitEmptyStdinPreservesExistingSemantics(t *testing.T) { issues := &recordingIssueService{} deps := &Dependencies{Issues: issues, Stdin: strings.NewReader("")} cmd := NewCmdWithDeps(deps, newIssuesUpdateCmd) - cmd.SetArgs([]string{"CEN-123", "--description", "-"}) + cmd.SetArgs([]string{"CEN-123", "--team", "CEN", "--description", "-"}) if err := cmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -138,6 +142,77 @@ func TestExplicitEmptyStdinPreservesExistingSemantics(t *testing.T) { } } +func TestExplicitEmptyStdinForCreatesForwardsEmptyDescription(t *testing.T) { + tests := []struct { + name string + factory func() *cobra.Command + args []string + description func(*testing.T, *recordingIssueService, *recordingProjectService) + }{ + { + name: "issue", + factory: newIssuesCreateCmd, + args: []string{"New issue", "--team", "CEN", "--description", "-"}, + description: func(t *testing.T, issues *recordingIssueService, _ *recordingProjectService) { + if issues.created == nil || issues.created.Description != "" { + t.Fatalf("created issue description = %#v, want empty string", issues.created) + } + }, + }, + { + name: "project", + factory: newProjectsCreateCmd, + args: []string{"New project", "--team", "CEN", "--description", "-"}, + description: func(t *testing.T, _ *recordingIssueService, projects *recordingProjectService) { + if projects.created == nil || projects.created.Description != "" { + t.Fatalf("created project description = %#v, want empty string", projects.created) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + issues := &recordingIssueService{} + projects := &recordingProjectService{} + deps := &Dependencies{Issues: issues, Projects: projects, Stdin: strings.NewReader("")} + cmd := NewCmdWithDeps(deps, tt.factory) + cmd.SetArgs(tt.args) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + tt.description(t, issues, projects) + }) + } +} + +func TestResolvedDescriptionIsPreservedBeforeAttachmentAppend(t *testing.T) { + filePath := filepath.Join(t.TempDir(), "attachment.txt") + if err := os.WriteFile(filePath, []byte("attachment contents"), 0o600); err != nil { + t.Fatalf("write attachment: %v", err) + } + + issues := &recordingIssueService{} + uploadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer uploadServer.Close() + transport := &recordingCommentTransport{uploadURL: uploadServer.URL + "/attachment"} + client := linear.NewClient("test-token") + client.GetBase().SetHTTPClient(&http.Client{Transport: transport}) + deps := &Dependencies{Client: client, Issues: issues, Stdin: strings.NewReader("resolved body\n")} + cmd := NewCmdWithDeps(deps, newIssuesCreateCmd) + cmd.SetArgs([]string{"New issue", "--team", "CEN", "--description", "-", "--attach", filePath}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := "resolved body\n\n![attachment.txt](https://assets.test/attachment.txt)" + if issues.created == nil || issues.created.Description != want { + t.Fatalf("created description = %#v, want %q", issues.created, want) + } +} + func TestExplicitStdinReadErrorsAreWrapped(t *testing.T) { issues := &recordingIssueService{} deps := &Dependencies{Issues: issues, Stdin: errorReader{err: fmt.Errorf("stdin failed")}} @@ -154,38 +229,35 @@ func TestExplicitStdinReadErrorsAreWrapped(t *testing.T) { } func TestEmptyStdinRejectsCommentAndReply(t *testing.T) { - tests := []struct { - name string - args []string - check func(*recordingIssueService) bool - }{ - {name: "comment", args: []string{"CEN-123", "--body", "-"}, check: func(issues *recordingIssueService) bool { - return issues.replyBody == "" - }}, - {name: "reply", args: []string{"CEN-123", "comment-123", "--body", "-"}, check: func(issues *recordingIssueService) bool { - return issues.replyBody == "" - }}, - } + t.Run("comment rejects before making client requests", func(t *testing.T) { + transport := &recordingCommentTransport{} + client := linear.NewClient("test-token") + client.GetBase().SetHTTPClient(&http.Client{Transport: transport}) + deps := &Dependencies{Client: client, Stdin: strings.NewReader("")} + cmd := NewCmdWithDeps(deps, newIssuesCommentCmd) + cmd.SetArgs([]string{"CEN-123", "--body", "-"}) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - issues := &recordingIssueService{} - client := linear.NewClient("test-token") - deps := &Dependencies{Client: client, Issues: issues, Stdin: strings.NewReader("")} - factory := newIssuesReplyCmd - if tt.name == "comment" { - factory = newIssuesCommentCmd - } - cmd := NewCmdWithDeps(deps, factory) - cmd.SetArgs(tt.args) - if err := cmd.Execute(); err == nil { - t.Fatal("expected empty body error") - } - if !tt.check(issues) { - t.Fatal("empty body reached service") - } - }) - } + if err := cmd.Execute(); err == nil { + t.Fatal("expected empty body error") + } + if transport.requests != 0 { + t.Fatalf("empty comment made %d client requests, want zero", transport.requests) + } + }) + + t.Run("reply rejects before calling service", func(t *testing.T) { + issues := &recordingIssueService{} + deps := &Dependencies{Issues: issues, Stdin: strings.NewReader("")} + cmd := NewCmdWithDeps(deps, newIssuesReplyCmd) + cmd.SetArgs([]string{"CEN-123", "comment-123", "--body", "-"}) + + if err := cmd.Execute(); err == nil { + t.Fatal("expected empty body error") + } + if issues.replyBody != "" { + t.Fatal("empty reply reached service") + } + }) } type recordingIssueService struct { @@ -254,10 +326,22 @@ func (s *recordingProjectService) Update(_ string, input *service.UpdateProjectI } type recordingCommentTransport struct { - body string + body string + requests int + uploadURL string } func (t *recordingCommentTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.requests++ + if req.Method == http.MethodPut { + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("")), + Request: req, + }, nil + } + payload, err := io.ReadAll(req.Body) if err != nil { return nil, err @@ -271,7 +355,9 @@ func (t *recordingCommentTransport) RoundTrip(req *http.Request) (*http.Response } response := `{"data":{"issue":{"id":"issue-id","identifier":"CEN-123","title":"Test issue"}}}` - if strings.Contains(request.Query, "commentCreate") { + if strings.Contains(request.Query, "fileUpload") { + response = fmt.Sprintf(`{"data":{"fileUpload":{"success":true,"uploadFile":{"uploadUrl":%q,"assetUrl":"https://assets.test/attachment.txt","headers":[]}}}}`, t.uploadURL) + } else if strings.Contains(request.Query, "commentCreate") { t.body, _ = request.Variables["body"].(string) response = `{"data":{"commentCreate":{"success":true,"comment":{"id":"comment-id","body":"comment body","user":{"id":"user-id","name":"Test User","email":"test@example.com"},"issue":{"id":"issue-id","identifier":"CEN-123"}}}}}` } diff --git a/internal/cli/stdin_test.go b/internal/cli/stdin_test.go index ca4d3fe..9ead12b 100644 --- a/internal/cli/stdin_test.go +++ b/internal/cli/stdin_test.go @@ -39,6 +39,13 @@ func TestDescriptionFromFlagOrStdin(t *testing.T) { } } +func TestDescriptionFromFlagOrStdinRejectsNilReader(t *testing.T) { + _, err := getDescriptionFromFlagOrStdinWithReader("-", nil) + if err == nil || !strings.Contains(err.Error(), "stdin reader is not configured") { + t.Fatalf("got error %v, want deterministic nil-reader error", err) + } +} + func TestReadStdinFromReturnsReaderError(t *testing.T) { wantErr := errors.New("read failed") _, err := readStdinFrom(errorReader{err: wantErr}) From 3a82a282b093365f3991d3c60e6f9e726201bb95 Mon Sep 17 00:00:00 2001 From: Felix Lisczyk <5102728+FelixLisczyk@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:38:16 +0200 Subject: [PATCH 3/3] Removed plan.md --- plan.md | 145 -------------------------------------------------------- 1 file changed, 145 deletions(-) delete mode 100644 plan.md diff --git a/plan.md b/plan.md deleted file mode 100644 index 0f54d36..0000000 --- a/plan.md +++ /dev/null @@ -1,145 +0,0 @@ -# Implementation Plan - -## Task Overview -- **Source**: TL-561 -- **Title**: Make CLI stdin body handling explicit and regression-tested -- **Description**: `linear issues create/update`, project create/update, and issue comment/reply commands currently describe implicit piped stdin even though stdin is only read when the corresponding flag value is explicitly `-`. The implementation must make that contract unambiguous, preserve the historical non-blocking behavior, and add coverage proving body values reach the service/API inputs. - -## Requirements Analysis -- **Core Functionality**: - - Standardize the documented convention: `--description -` for issue/project descriptions and `--body -` for comments/replies. - - Keep stdin consumption opt-in only; do not restore automatic TTY/non-TTY detection. - - Preserve ordinary flag values and current `strings.TrimSpace` handling of explicit stdin. - - Preserve current empty-input behavior, including the existing update/clearing semantics, unless tests reveal only documentation or helper behavior needs clarification. - - Ensure explicit stdin content reaches issue create/update, project create/update, comment, and reply inputs. -- **Acceptance Criteria**: - - Every affected command's help text and examples explicitly show the `-` value and no longer imply that an unadorned pipe is consumed. - - README and embedded CLI skill documentation contain no stale implicit-pipe examples or wording. - - Tests cover the shared resolver, whitespace trimming, empty input, explicit propagation through all affected command surfaces, and the non-consuming implicit path. - - Existing ordinary `--description `/`--body `, attachments, and service-layer behavior remain intact. -- **Technical Constraints**: - - `readStdin` currently reads `os.Stdin` directly and trims leading/trailing whitespace. - - A prior fix (commit `2a47758`) intentionally removed automatic pipe detection because open stdin could make updates hang in CI, Claude Code, or scripts; this must not regress. - - Tests should not depend on mutating process-global stdin or on a live Linear API. - - No service or GraphQL schema change is required; the defect is at CLI input resolution and documentation. -- **Integration Points**: - - Shared helper in `internal/cli/helpers.go` is used by issue/project descriptions and issue comment/reply bodies. - - Cobra command closures in `internal/cli/issues.go` and `internal/cli/projects.go` assemble service inputs. - - `Dependencies`, `NewCmdWithDeps`, and the service interfaces provide test injection; the raw client path used by comments may require an HTTP test transport or a narrowly scoped test seam. - - README and `internal/skills/linear/SKILL.md` are user-facing documentation surfaces to audit and update. - -## Codebase Analysis -- **Existing Patterns**: - - `readStdin` uses a `bufio.Reader` over `os.Stdin` and returns `strings.TrimSpace(builder.String())` (`internal/cli/helpers.go:24-41`). - - `getDescriptionFromFlagOrStdin` reads only for the exact flag value `"-"` (`helpers.go:63-71`); all affected commands call it before building service inputs. - - Issue create resolves the body before attachment processing and passes it as `CreateIssueInput.Description` (`issues.go:375-394`); issue update does the same for `UpdateIssueInput.Description` (`issues.go:560-582`). - - Project create/update follow the same resolver pattern (`projects.go:194-205`, `projects.go:285-299`). Reply already uses the injectable `IssueServiceInterface`; comment currently resolves the issue and creates the comment through `deps.Client` (`issues.go:697-725`). - - Existing tests are table-driven Go tests, but `helpers_test.go` currently covers only date parsing. `NewCmdWithDeps` injects command dependencies, while service interfaces use hand-written test doubles in existing service tests. -- **Available Infrastructure**: - - `Dependencies` exposes issue/project/user services and the Linear client (`internal/cli/dependencies.go`). - - `pkg/linear/core` supports test HTTP clients/transports, and `pkg/linear/testutil` contains mock transport helpers for API-level assertions. - - `make test` is the repository-wide verification command. -- **Dependencies**: - - Likely changes are confined to CLI helpers, affected Cobra commands, CLI tests, and documentation. Service input types and GraphQL mutations should remain unchanged. -- **Architecture Notes**: - - The explicit `-` check is the safety boundary: the implementation should make this boundary clearer rather than infer intent from stdin state. - - Extract a pure `readStdinFrom(io.Reader)` helper and keep `readStdin()` as the production wrapper over `os.Stdin`; this gives tests isolated readers without a mutable package-global override. - - Use `NewCmdWithDeps` and complete test doubles for the issue/project service interfaces. Tests must pass explicit `--team` values so they do not depend on local config. For comment, preserve the existing output and raw-client path and use a test-local sequential recording `http.RoundTripper` that handles issue resolution and comment mutation responses while capturing the mutation variables; do not broaden public APIs. - -## Implementation Strategy -- **Problem Complexity**: Moderate, cross-cutting CLI contract/documentation bug with a small behavior surface and a significant regression-testing gap. -- **Core Problem**: The code already implements safe explicit stdin handling, but help and examples promise a different interface and lack tests proving the safe path is used consistently. -- **Approach**: Preserve the exact-`-` behavior and trimming policy, make the stdin reader testable, update all affected help/docs/error text to state the explicit convention, then add command-level propagation tests through dependency injection and a bounded non-consumption test. -- **Testing Approach**: Automated unit and CLI-level tests, followed by the full Go test suite. No manual GUI verification is relevant. -- **Phases**: - 1. **Stabilize and test the shared stdin contract** - - **Implementation**: - - Extract a pure `readStdinFrom(io.Reader)` helper and keep the production `readStdin()` wrapper sourced from `os.Stdin`. - - Keep `getDescriptionFromFlagOrStdin` exact-match behavior (`"-"` reads; all other values return unchanged) and retain `strings.TrimSpace`. - - Add helper tests for ordinary flag values, explicit non-empty stdin, leading/trailing whitespace and final newlines, empty stdin, reader errors, and a reader that records whether it was touched. - - **Verification**: - - Run the focused CLI helper tests. Confirm explicit `-` returns trimmed content, empty stdin remains empty, ordinary values bypass the reader, and no implicit path attempts a read. - 2. **Cover propagation through affected commands** - - **Implementation**: - - Add command-level tests using `NewCmdWithDeps`, complete issue/project service test doubles, and explicit `--team` values to capture `CreateIssueInput.Description`, `UpdateIssueInput.Description`, `CreateProjectInput.Description`, and `UpdateProjectInput.Description` for explicit stdin. - - Cover reply body propagation through the issue service fake. - - Cover comment body propagation without changing its current output path: use a test-local sequential recording `http.RoundTripper` that responds to the issue lookup and comment mutation and asserts the exact comment body variable. - - Add bounded non-blocking coverage that executes commands with ordinary flags and a reader that would fail/block if consumed; verify service calls still occur without reading unrelated stdin. The shared helper test is the primary proof of the no-read branch; command coverage should only be added where it exercises a distinct command path. - - Assert explicit stdin read errors are returned with the command's existing contextual error wrapping. - - Define empty-input compatibility precisely: the helper returns `""`; create inputs carry an empty description; update inputs retain the current nil `Description` behavior while `description == "-"` still passes the existing update gate; comment/reply commands reject empty bodies before lookup/mutation. Do not introduce description clearing. - - Assert ordinary literal flag values continue to pass through unchanged and that stdin-resolved text remains the body before attachment appending. - - **Verification**: - - Run all new CLI tests. Confirm each affected command forwards explicit stdin content and the implicit form does not consume stdin or silently claim to have accepted a body. - 3. **Correct user-facing help and documentation** - - **Implementation**: - - Update issue create/update, comment, and reply long descriptions, flag descriptions, examples, and required-body error messages in `internal/cli/issues.go` to say stdin is read only with `--description -` or `--body -`; specifically fix the unadorned `comment` and `reply` examples. - - Update project create/update examples and flag descriptions in `internal/cli/projects.go`, specifically replacing the unadorned project create/update pipe examples with explicit `--description -` examples. - - Audit `internal/cli/onboard.go` for its stale pipe example and correct it as well. - - Audit README piping and command examples for the same surfaces. Preserve examples that already use explicit `-` (including the current README issue/comment/reply forms), changing only stale or ambiguous wording and adding the whitespace policy where the stdin contract is documented. - - Audit `internal/skills/linear/SKILL.md` similarly; retain its already-correct explicit `-` examples unless the audit finds wording that still implies implicit reading. Search for command-shaped unadorned pipelines as well as `pipe to stdin` text before finishing. - - **Verification**: - - Run help/documentation-focused tests if added (including assertions against command help text), then inspect targeted grep results to ensure no affected command advertises implicit stdin. Run `make test` after documentation and command changes. - 4. **Final regression verification** - - **Implementation**: - - Review the diff for unchanged attachment handling, ordinary flag behavior, empty-input semantics, and the historical no-auto-detection constraint. - - Add or adjust comments only where they accurately describe explicit `-` behavior and trimming. - - **Verification**: - - Run `make test` from a clean working state. Confirm all tests pass and the final search/help checks show a single consistent stdin contract. - -**Phase Verification Approach**: Each phase ends with automated verification before the next begins. Tests are added after the corresponding implementation within each phase, using injected readers, service fakes, and deterministic HTTP transport rather than a live API. The final phase requires the complete repository test suite to pass. - -## Quality Assurance Plan -- **Testing Strategy**: - - Table-driven helper tests for the resolver and whitespace/empty-input policy. - - CLI execution tests for issue/project create/update and reply using `NewCmdWithDeps`. - - Deterministic client/transport coverage for comment request body propagation. - - Help-text assertions or focused string checks for explicit flag syntax, plus the full `make test` suite. -- **Edge Cases**: - - Literal body/description values other than `-` must not trigger stdin. - - Explicit stdin with leading/trailing spaces and final newline is trimmed exactly as today. - - Explicit stdin read errors are surfaced with the existing helper/command context. - - Empty stdin returns an empty string; create forwards empty description, update keeps `Description` nil while retaining the `"-"` update gate, and comment/reply fail before issue lookup or mutation. No new clearing behavior is introduced. - - Commands with unrelated open stdin must not block or consume it. - - Attachment appending must still work after body resolution, with the resolved body retained before the attachment markdown is added. - - Issue update's `description == "-"` must still count as an explicitly requested update even when the resulting body is empty, without inventing a new clearing behavior. -- **Regression Prevention**: - - Keep the exact-`-` condition in one shared helper. - - Test both the positive read path and the negative no-read path. - - Search all affected help/docs surfaces for stale implicit-pipe language. - - Preserve service interfaces and GraphQL inputs unless a narrowly scoped testability refactor is proven necessary. -- **Success Verification**: - - `make test` passes. - - Explicit stdin examples work consistently for all six command paths (issue create/update, project create/update, comment, reply). - - Ordinary values and non-body commands do not read stdin. - - Help, README, and embedded skill guidance all require the explicit `-` convention. - -## Development Environment -- **Setup Requirements**: Go toolchain and repository dependencies already defined by the project; no external service credentials or live Linear workspace are needed. -- **Debugging Strategy**: Capture service inputs and GraphQL request payloads in test doubles/transports; use focused test runs while iterating, then `make test`. -- **Iteration Approach**: Implement one phase at a time, run its focused tests, then proceed only after verification; finish with repository-wide tests and targeted documentation search. - -## Risk Assessment -- **Potential Issues**: - - Process-global stdin can make tests flaky or hang if the seam is incomplete. - - Comment command's direct client access may make body capture harder than the service-backed paths. - - Help wording can remain inconsistent in an overlooked README or embedded skill example. - - Changing empty stdin handling accidentally could alter description-clearing behavior. -- **Mitigation Strategies**: - - Use the pure reader helper rather than mutable package-global test state; keep command tests serial only where the existing command globals require it. - - Use a test-local sequential recording `http.RoundTripper` for the comment path so issue lookup and comment mutation receive distinct deterministic responses and the mutation body can be asserted; avoid requiring a live API or changing the production base URL. - - Complete every method on the issue/project service interfaces in test doubles, and pass explicit team flags to avoid machine-local configuration. - - Search all source/docs for `pipe`, `stdin`, `--description`, and `--body`, including unadorned command-shaped pipelines and onboarding examples, and add help assertions for affected commands. - - Treat whitespace, read errors, and empty-input behavior as explicit compatibility cases before editing command assembly. -- **Backup Approaches**: - - If the recording transport is too brittle, keep production comment code unchanged and isolate the GraphQL request sequence in a focused test helper that still asserts the mutation variables. - - If complete interface fakes become noisy, define test-only adapters that delegate unneeded methods to zero-value error implementations while keeping the captured methods explicit; do not weaken production interfaces or use a live API. - -## Files Likely to Change -- `internal/cli/helpers.go` - Add the internal stdin reader seam and clarify the explicit-`-` helper contract without changing production behavior. -- `internal/cli/helpers_test.go` - Add whitespace, empty-input, ordinary-value, explicit-read, and non-read helper tests. -- `internal/cli/issues.go` - Correct issue description/body help, examples, and errors; retain explicit stdin behavior. -- `internal/cli/projects.go` - Correct project description help and examples. -- `internal/cli/onboard.go` - Correct the stale onboarding pipeline example. -- `internal/cli/*_test.go` - Add command-level fakes/transport tests for issue, project, comment, and reply body propagation, read errors, and bounded non-consumption. -- `README.md` - Remove stale implicit-pipe examples and document explicit `--description -`/`--body -` usage and trimming policy. -- `internal/skills/linear/SKILL.md` - Audit and align embedded CLI stdin guidance.