diff --git a/README.md b/README.md index 9bf5dc0..684416a 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,12 @@ td init # Create your first issue td create "Add user auth" --type feature --priority P1 +# Agent-safe rich text input for markdown-heavy fields +td create "Document sync failure modes" \ + --description-file docs/issue-description.md \ + --acceptance-file docs/issue-acceptance.md +cat docs/issue-description.md | td update td-a1b2 --append --description-file - + # Start work td start ``` diff --git a/cmd/approve_test.go b/cmd/approve_test.go index b18bda4..2898cb7 100644 --- a/cmd/approve_test.go +++ b/cmd/approve_test.go @@ -1,8 +1,15 @@ package cmd import ( + "bytes" + "io" + "os" + "strings" "testing" + "github.com/marcus/td/internal/db" + "github.com/marcus/td/internal/models" + "github.com/marcus/td/internal/session" "github.com/spf13/cobra" ) @@ -104,3 +111,212 @@ func TestApprovalReasonWithNoteFlags(t *testing.T) { t.Fatalf("note vs comment: got %q, want %q", got, "n") } } + +func TestApproveNoArgsUsesSingleReviewableIssue(t *testing.T) { + saveAndRestoreGlobals(t) + t.Setenv("TD_SESSION_ID", "ses_cmd_test") + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + sess, err := session.GetOrCreate(database) + if err != nil { + t.Fatalf("GetOrCreate failed: %v", err) + } + + issue := &models.Issue{ + Title: "Single reviewable issue", + Status: models.StatusInReview, + Minor: true, + ImplementerSession: sess.ID, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + if err := database.UpdateIssue(issue); err != nil { + t.Fatalf("UpdateIssue failed: %v", err) + } + + var output bytes.Buffer + oldStdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + os.Stdout = w + + runErr := approveCmd.RunE(approveCmd, []string{}) + + w.Close() + os.Stdout = oldStdout + _, _ = io.Copy(&output, r) + + if runErr != nil { + t.Fatalf("approveCmd.RunE returned error: %v", runErr) + } + + got := output.String() + if !strings.Contains(got, "APPROVED "+issue.ID) { + t.Fatalf("expected approval output for %q, got %s", issue.ID, got) + } + + updated, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if updated.Status != models.StatusClosed { + t.Fatalf("status = %s, want %s", updated.Status, models.StatusClosed) + } + if updated.ReviewerSession != sess.ID { + t.Fatalf("reviewer session = %q, want %q", updated.ReviewerSession, sess.ID) + } +} + +func TestApproveClosedIssueIsIdempotent(t *testing.T) { + saveAndRestoreGlobals(t) + t.Setenv("TD_SESSION_ID", "ses_cmd_test") + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Already closed review target", + Status: models.StatusClosed, + ReviewerSession: "ses_original_reviewer", + ImplementerSession: "ses_impl", + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + if err := database.UpdateIssue(issue); err != nil { + t.Fatalf("UpdateIssue failed: %v", err) + } + + var output bytes.Buffer + oldStdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + os.Stdout = w + + runErr := approveCmd.RunE(approveCmd, []string{issue.ID}) + + _ = w.Close() + os.Stdout = oldStdout + _, _ = io.Copy(&output, r) + + if runErr != nil { + t.Fatalf("approveCmd.RunE returned error: %v", runErr) + } + + got := output.String() + if !strings.Contains(got, "already approved/closed") { + t.Fatalf("expected idempotent approval output, got %s", got) + } + + updated, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if updated.Status != models.StatusClosed { + t.Fatalf("status = %s, want %s", updated.Status, models.StatusClosed) + } + if updated.ReviewerSession != "ses_original_reviewer" { + t.Fatalf("reviewer session = %q, want %q", updated.ReviewerSession, "ses_original_reviewer") + } +} + +func TestApproveClosedIssueUsesLatestApprovalReasonContext(t *testing.T) { + saveAndRestoreGlobals(t) + t.Setenv("TD_SESSION_ID", "ses_cmd_test") + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Recently approved issue", + Status: models.StatusInReview, + ImplementerSession: "ses_impl", + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + if err := database.UpdateIssue(issue); err != nil { + t.Fatalf("UpdateIssue failed: %v", err) + } + if err := database.AddLog(&models.Log{ + IssueID: issue.ID, + SessionID: "ses_impl", + Message: "Submitted for review", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + if err := approveCmd.Flags().Set("reason", "ship it"); err != nil { + t.Fatalf("set reason: %v", err) + } + + var first bytes.Buffer + oldStdout := os.Stdout + r1, w1, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + os.Stdout = w1 + runErr := approveCmd.RunE(approveCmd, []string{issue.ID}) + _ = w1.Close() + os.Stdout = oldStdout + _, _ = io.Copy(&first, r1) + if runErr != nil { + t.Fatalf("first approveCmd.RunE returned error: %v", runErr) + } + + if err := approveCmd.Flags().Set("reason", ""); err != nil { + t.Fatalf("clear reason: %v", err) + } + + var second bytes.Buffer + r2, w2, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + os.Stdout = w2 + runErr = approveCmd.RunE(approveCmd, []string{issue.ID}) + _ = w2.Close() + os.Stdout = oldStdout + _, _ = io.Copy(&second, r2) + if runErr != nil { + t.Fatalf("second approveCmd.RunE returned error: %v", runErr) + } + + got := second.String() + if !strings.Contains(got, "Recent transition: approved") { + t.Fatalf("expected latest approval transition in %s", got) + } + if strings.Contains(got, "Recent transition: submitted for review") { + t.Fatalf("expected stale submitted-for-review context to be skipped in %s", got) + } +} diff --git a/cmd/context.go b/cmd/context.go index 383ea83..f693b5c 100644 --- a/cmd/context.go +++ b/cmd/context.go @@ -199,7 +199,7 @@ var usageCmd = &cobra.Command{ fmt.Println(" 3. `td handoff ` to capture state (REQUIRED)") fmt.Println(" Multi-issue: `td ws handoff`") fmt.Println(" 4. `td review ` to submit for review") - fmt.Println(" 5. Different session: `td approve ` to complete") + fmt.Println(" 5. Reviewer: `td approve ` to close in_review work, or `td reject ` to send it back to open") fmt.Println() fmt.Println(" Use `td ws` commands when implementing multiple related issues.") fmt.Println() @@ -209,6 +209,8 @@ var usageCmd = &cobra.Command{ fmt.Println(" td context Full context for resuming") fmt.Println(" td next Highest priority open issue") fmt.Println(" td critical-path What unblocks the most work") + fmt.Println(" td status --json Machine-readable session and review state") + fmt.Println(" td list --json Machine-readable issue listings for scripts") fmt.Println(" td reviewable Issues you can review") fmt.Println(" td approve/reject Complete review") fmt.Println() diff --git a/cmd/create.go b/cmd/create.go index 34ed613..09f9b00 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -20,6 +20,9 @@ var createCmd = &cobra.Command{ Aliases: []string{"add", "new"}, Short: "Create a new issue", Long: `Create a new issue with optional flags for type, priority, labels, and more.`, + Example: " td create \"Add user auth\" --type feature --priority P1\n" + + " td create \"Rich markdown issue\" --description-file description.md --acceptance-file acceptance.md\n" + + " cat acceptance.md | td create \"Import from stdin\" --acceptance-file -", GroupID: "core", RunE: func(cmd *cobra.Command, args []string) error { // Route "td new task Title" → td create --type task "Title" @@ -108,49 +111,57 @@ var createCmd = &cobra.Command{ } // Labels (support --labels, --label, --tags, --tag) - labelsStr, _ := cmd.Flags().GetString("labels") - if labelsStr == "" { - if s, _ := cmd.Flags().GetString("label"); s != "" { - labelsStr = s + // All label flags support repeated flags (-l a -l b) and comma-separated (-l "a,b") + labelsArr, _ := cmd.Flags().GetStringArray("labels") + if len(labelsArr) == 0 { + if arr, _ := cmd.Flags().GetStringArray("label"); len(arr) > 0 { + labelsArr = arr } } - if labelsStr == "" { - if s, _ := cmd.Flags().GetString("tags"); s != "" { - labelsStr = s + if len(labelsArr) == 0 { + if arr, _ := cmd.Flags().GetStringArray("tags"); len(arr) > 0 { + labelsArr = arr } } - if labelsStr == "" { - if s, _ := cmd.Flags().GetString("tag"); s != "" { - labelsStr = s + if len(labelsArr) == 0 { + if arr, _ := cmd.Flags().GetStringArray("tag"); len(arr) > 0 { + labelsArr = arr } } - if labelsStr != "" { - issue.Labels = strings.Split(labelsStr, ",") - for i := range issue.Labels { - issue.Labels[i] = strings.TrimSpace(issue.Labels[i]) - } + if len(labelsArr) > 0 { + issue.Labels = mergeMultiValueFlag(labelsArr) } - // Description (support --description, --desc, and --body) - issue.Description, _ = cmd.Flags().GetString("description") - if issue.Description == "" { - if desc, _ := cmd.Flags().GetString("desc"); desc != "" { - issue.Description = desc - } + stdinUsed := false + + description, descriptionProvided, nextStdinUsed, err := resolveRichTextField( + cmd, + []string{"description", "desc", "body", "notes"}, + "description-file", + stdinUsed, + ) + if err != nil { + output.Error("%v", err) + return err } - if issue.Description == "" { - if body, _ := cmd.Flags().GetString("body"); body != "" { - issue.Description = body - } + if descriptionProvided { + issue.Description = description } - if issue.Description == "" { - if notes, _ := cmd.Flags().GetString("notes"); notes != "" { - issue.Description = notes - } + stdinUsed = nextStdinUsed + + acceptance, acceptanceProvided, _, err := resolveRichTextField( + cmd, + []string{"acceptance"}, + "acceptance-file", + stdinUsed, + ) + if err != nil { + output.Error("%v", err) + return err + } + if acceptanceProvided { + issue.Acceptance = acceptance } - - // Acceptance - issue.Acceptance, _ = cmd.Flags().GetString("acceptance") // Parent (supports --parent and --epic) issue.ParentID, _ = cmd.Flags().GetString("parent") @@ -208,19 +219,17 @@ var createCmd = &cobra.Command{ output.Warning("failed to record session history: %v", err) } - // Handle dependencies - if dependsOn, _ := cmd.Flags().GetString("depends-on"); dependsOn != "" { - for _, dep := range strings.Split(dependsOn, ",") { - dep = strings.TrimSpace(dep) + // Handle dependencies (support repeated flags and comma-separated) + if dependsArr, _ := cmd.Flags().GetStringArray("depends-on"); len(dependsArr) > 0 { + for _, dep := range mergeMultiValueFlag(dependsArr) { if err := database.AddDependencyLogged(issue.ID, dep, "depends_on", sess.ID); err != nil { output.Warning("failed to add dependency %s: %v", dep, err) } } } - if blocks, _ := cmd.Flags().GetString("blocks"); blocks != "" { - for _, blocked := range strings.Split(blocks, ",") { - blocked = strings.TrimSpace(blocked) + if blocksArr, _ := cmd.Flags().GetStringArray("blocks"); len(blocksArr) > 0 { + for _, blocked := range mergeMultiValueFlag(blocksArr) { if err := database.AddDependencyLogged(blocked, issue.ID, "depends_on", sess.ID); err != nil { output.Warning("failed to add blocks %s: %v", blocked, err) } @@ -239,19 +248,21 @@ func init() { createCmd.Flags().StringP("type", "t", "", "Issue type (bug, feature, task, epic, chore)") createCmd.Flags().StringP("priority", "p", "", "Priority (P0, P1, P2, P3, P4)") createCmd.Flags().Int("points", 0, "Story points (Fibonacci: 1,2,3,5,8,13,21)") - createCmd.Flags().StringP("labels", "l", "", "Comma-separated labels") - createCmd.Flags().String("label", "", "Alias for --labels (single or comma-separated)") - createCmd.Flags().String("tags", "", "Alias for --labels") - createCmd.Flags().String("tag", "", "Alias for --labels") + createCmd.Flags().StringArrayP("labels", "l", nil, "Labels (repeatable, comma-separated)") + createCmd.Flags().StringArray("label", nil, "Alias for --labels") + createCmd.Flags().StringArray("tags", nil, "Alias for --labels") + createCmd.Flags().StringArray("tag", nil, "Alias for --labels") createCmd.Flags().StringP("description", "d", "", "Description text") createCmd.Flags().String("desc", "", "Alias for --description") createCmd.Flags().String("body", "", "Alias for --description") createCmd.Flags().String("notes", "", "Alias for --description") + createCmd.Flags().String("description-file", "", "Read description from file or - for stdin (preserves formatting)") createCmd.Flags().String("acceptance", "", "Acceptance criteria") + createCmd.Flags().String("acceptance-file", "", "Read acceptance criteria from file or - for stdin (preserves formatting)") createCmd.Flags().String("parent", "", "Parent issue ID") createCmd.Flags().String("epic", "", "Parent issue ID (alias for --parent)") - createCmd.Flags().String("depends-on", "", "Issues this depends on") - createCmd.Flags().String("blocks", "", "Issues this blocks") + createCmd.Flags().StringArray("depends-on", nil, "Issues this depends on (repeatable, comma-separated)") + createCmd.Flags().StringArray("blocks", nil, "Issues this blocks (repeatable, comma-separated)") createCmd.Flags().Bool("minor", false, "Mark as minor task (allows self-review)") createCmd.Flags().String("defer", "", "Defer until date (e.g., +7d, monday, 2026-03-01)") createCmd.Flags().String("due", "", "Due date (e.g., friday, +2w, 2026-03-15)") @@ -277,6 +288,24 @@ func parseTypeFromTitle(title string) (models.Type, string) { return "", title } +// mergeMultiValueFlag takes a string array from a repeated flag, splits each +// element on commas, trims whitespace, and deduplicates. This allows both +// `-l "a,b"` and `-l a -l b` (and mixed: `-l "a,b" -l c`) to work. +func mergeMultiValueFlag(values []string) []string { + var result []string + seen := make(map[string]bool) + for _, v := range values { + for _, part := range strings.Split(v, ",") { + part = strings.TrimSpace(part) + if part != "" && !seen[part] { + seen[part] = true + result = append(result, part) + } + } + } + return result +} + // validateTitle checks that the title is descriptive enough func validateTitle(title string, minLength, maxLength int) error { // Generic titles that should be rejected (case-insensitive) diff --git a/cmd/create_test.go b/cmd/create_test.go index eb5fea7..029f0c0 100644 --- a/cmd/create_test.go +++ b/cmd/create_test.go @@ -2,6 +2,8 @@ package cmd import ( "fmt" + "os" + "path/filepath" "strings" "testing" @@ -419,33 +421,30 @@ func TestCreateTagFlagParsing(t *testing.T) { t.Error("Expected --tags flag to be defined") } - // Test that --tag flag can be set + // Test that --tag flag can be set (StringArray accepts individual values via Set) if err := createCmd.Flags().Set("tag", "test,data"); err != nil { t.Errorf("Failed to set --tag flag: %v", err) } - tagValue, err := createCmd.Flags().GetString("tag") + tagValue, err := createCmd.Flags().GetStringArray("tag") if err != nil { t.Errorf("Failed to get --tag flag value: %v", err) } - if tagValue != "test,data" { - t.Errorf("Expected tag value 'test,data', got %s", tagValue) + if len(tagValue) != 1 || tagValue[0] != "test,data" { + t.Errorf("Expected tag value ['test,data'], got %v", tagValue) } - // Reset flags - createCmd.Flags().Set("tag", "") - // Test that --tags flag can be set if err := createCmd.Flags().Set("tags", "backend,api"); err != nil { t.Errorf("Failed to set --tags flag: %v", err) } - tagsValue, err := createCmd.Flags().GetString("tags") + tagsValue, err := createCmd.Flags().GetStringArray("tags") if err != nil { t.Errorf("Failed to get --tags flag value: %v", err) } - if tagsValue != "backend,api" { - t.Errorf("Expected tags value 'backend,api', got %s", tagsValue) + if len(tagsValue) != 1 || tagsValue[0] != "backend,api" { + t.Errorf("Expected tags value ['backend,api'], got %v", tagsValue) } } @@ -850,13 +849,13 @@ func TestValidateTitleMinLength(t *testing.T) { maxLen int wantError bool }{ - {"Short", 15, 200, true}, // 5 chars < 15 - {"This is fine!", 15, 200, true}, // 13 chars < 15 - {"This is long enough to pass", 15, 200, false}, // 27 chars >= 15 - {"Exactly fifteen!", 15, 200, false}, // 16 chars >= 15 - {"Fix the login bug", 15, 200, false}, // 17 chars >= 15 - {"A", 1, 200, false}, // Custom min=1 - {"Unicode: 日本語テスト長さ確認", 15, 200, false}, // Unicode rune count (19 runes >= 15) + {"Short", 15, 200, true}, // 5 chars < 15 + {"This is fine!", 15, 200, true}, // 13 chars < 15 + {"This is long enough to pass", 15, 200, false}, // 27 chars >= 15 + {"Exactly fifteen!", 15, 200, false}, // 16 chars >= 15 + {"Fix the login bug", 15, 200, false}, // 17 chars >= 15 + {"A", 1, 200, false}, // Custom min=1 + {"Unicode: 日本語テスト長さ確認", 15, 200, false}, // Unicode rune count (19 runes >= 15) } for _, tt := range tests { @@ -879,11 +878,11 @@ func TestValidateTitleMaxLength(t *testing.T) { wantError bool }{ {"This is a normal length title that should pass easily", 15, 200, false}, - {strings.Repeat("a", 200), 15, 200, false}, // Exactly 200 chars - {strings.Repeat("a", 201), 15, 200, true}, // 201 chars > 200 - {strings.Repeat("a", 300), 15, 200, true}, // Way too long - {strings.Repeat("a", 50), 15, 50, false}, // Custom max - {strings.Repeat("a", 51), 15, 50, true}, // Custom max exceeded + {strings.Repeat("a", 200), 15, 200, false}, // Exactly 200 chars + {strings.Repeat("a", 201), 15, 200, true}, // 201 chars > 200 + {strings.Repeat("a", 300), 15, 200, true}, // Way too long + {strings.Repeat("a", 50), 15, 50, false}, // Custom max + {strings.Repeat("a", 51), 15, 50, true}, // Custom max exceeded } for _, tt := range tests { @@ -961,3 +960,98 @@ func TestConfigGetTitleLengthLimitsDefaults(t *testing.T) { t.Errorf("Expected default max %d, got %d", config.DefaultTitleMaxLength, max) } } + +func TestCreateRichTextFromFileAndStdin(t *testing.T) { + saveAndRestoreGlobals(t) + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + saveAndRestoreCommandFlags(t, createCmd, "description", "desc", "body", "notes", "description-file", "acceptance", "acceptance-file") + + description := "Intro\n\n```go\nfmt.Println(\"hello\")\n```\n indented\n" + descriptionPath := filepath.Join(dir, "description.md") + if err := os.WriteFile(descriptionPath, []byte(description), 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + + acceptance := "- keep blank lines\n\n- preserve `code`\n" + replaceStdinWithFile(t, acceptance) + + if err := createCmd.Flags().Set("description-file", descriptionPath); err != nil { + t.Fatalf("Set description-file failed: %v", err) + } + if err := createCmd.Flags().Set("acceptance-file", "-"); err != nil { + t.Fatalf("Set acceptance-file failed: %v", err) + } + + if err := createCmd.RunE(createCmd, []string{"Rich text import task"}); err != nil { + t.Fatalf("createCmd.RunE failed: %v", err) + } + + issues, err := database.ListIssues(db.ListIssuesOptions{}) + if err != nil { + t.Fatalf("ListIssues failed: %v", err) + } + if len(issues) != 1 { + t.Fatalf("expected 1 issue, got %d", len(issues)) + } + + if issues[0].Description != description { + t.Fatalf("description mismatch\nwant: %q\ngot: %q", description, issues[0].Description) + } + if issues[0].Acceptance != acceptance { + t.Fatalf("acceptance mismatch\nwant: %q\ngot: %q", acceptance, issues[0].Acceptance) + } +} + +func TestCreateRichTextConflictErrors(t *testing.T) { + saveAndRestoreGlobals(t) + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + saveAndRestoreCommandFlags(t, createCmd, "description", "desc", "body", "notes", "description-file") + + descriptionPath := filepath.Join(dir, "description.md") + if err := os.WriteFile(descriptionPath, []byte("file body"), 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + + if err := createCmd.Flags().Set("description", "inline body"); err != nil { + t.Fatalf("Set description failed: %v", err) + } + if err := createCmd.Flags().Set("description-file", descriptionPath); err != nil { + t.Fatalf("Set description-file failed: %v", err) + } + + err = createCmd.RunE(createCmd, []string{"Conflicting rich text task"}) + if err == nil { + t.Fatal("expected createCmd.RunE to fail") + } + if !strings.Contains(err.Error(), "--description-file") || !strings.Contains(err.Error(), "--description") { + t.Fatalf("expected actionable conflict error, got %v", err) + } + + issues, err := database.ListIssues(db.ListIssuesOptions{}) + if err != nil { + t.Fatalf("ListIssues failed: %v", err) + } + if len(issues) != 0 { + t.Fatalf("expected 0 issues after conflict, got %d", len(issues)) + } +} diff --git a/cmd/epic.go b/cmd/epic.go index 51a2b3c..29dfe4f 100644 --- a/cmd/epic.go +++ b/cmd/epic.go @@ -103,11 +103,11 @@ func init() { epicCreateCmd.Flags().String("title", "", "Issue title (max 200 characters)") epicCreateCmd.Flags().StringP("priority", "p", "", "Priority (P0, P1, P2, P3, P4)") epicCreateCmd.Flags().StringP("description", "d", "", "Description text") - epicCreateCmd.Flags().String("labels", "", "Comma-separated labels") + epicCreateCmd.Flags().StringArrayP("labels", "l", nil, "Labels (repeatable, comma-separated)") epicCreateCmd.Flags().String("parent", "", "Parent issue ID") epicCreateCmd.Flags().String("epic", "", "Parent issue ID (alias for --parent)") - epicCreateCmd.Flags().String("depends-on", "", "Issues this depends on") - epicCreateCmd.Flags().String("blocks", "", "Issues this blocks") + epicCreateCmd.Flags().StringArray("depends-on", nil, "Issues this depends on (repeatable, comma-separated)") + epicCreateCmd.Flags().StringArray("blocks", nil, "Issues this blocks (repeatable, comma-separated)") // Hidden type flag - set programmatically to "epic" epicCreateCmd.Flags().StringP("type", "t", "", "") epicCreateCmd.Flags().MarkHidden("type") diff --git a/cmd/list.go b/cmd/list.go index d5005e9..390de6f 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -179,7 +179,7 @@ var listCmd = &cobra.Command{ // Parent filter if parentID, _ := cmd.Flags().GetString("parent"); parentID != "" { - resolvedParentID, err := resolveListIssueFilterID(baseDir, parentID, "parent") + resolvedParentID, err := resolveListIssueFilterID(database, baseDir, parentID, "parent") if err != nil { output.Error("%v", err) return err @@ -189,7 +189,7 @@ var listCmd = &cobra.Command{ // Epic filter if epicID, _ := cmd.Flags().GetString("epic"); epicID != "" { - resolvedEpicID, err := resolveListIssueFilterID(baseDir, epicID, "epic") + resolvedEpicID, err := resolveListIssueFilterID(database, baseDir, epicID, "epic") if err != nil { output.Error("%v", err) return err @@ -595,7 +595,7 @@ func parseDateFilter(s string) (after, before time.Time) { return time.Time{}, time.Time{} } -func resolveListIssueFilterID(baseDir, rawID, flagName string) (string, error) { +func resolveListIssueFilterID(database *db.DB, baseDir, rawID, flagName string) (string, error) { trimmedID := strings.TrimSpace(rawID) if trimmedID == "" { return "", nil @@ -607,7 +607,23 @@ func resolveListIssueFilterID(baseDir, rawID, flagName string) (string, error) { return "", fmt.Errorf("resolve --%s .: %w", flagName, err) } if strings.TrimSpace(focusedID) == "" { - return "", fmt.Errorf("--%s . requires a focused issue", flagName) + var resolveErr error + if resolvedID, err := resolveListIssueFilterFromSession(database); err == nil && resolvedID != "" { + return resolvedID, nil + } else if err != nil { + resolveErr = err + } + + if resolvedID, err := resolveListIssueFilterFromWorkSession(database, baseDir); err == nil && resolvedID != "" { + return resolvedID, nil + } else if err != nil { + resolveErr = err + } + + if resolveErr != nil { + return "", fmt.Errorf("--%s . requires a focused issue, recent session activity, or an active work session: %w", flagName, resolveErr) + } + return "", fmt.Errorf("--%s . requires a focused issue, recent session activity, or an active work session", flagName) } return db.NormalizeIssueID(focusedID), nil } @@ -615,6 +631,102 @@ func resolveListIssueFilterID(baseDir, rawID, flagName string) (string, error) { return db.NormalizeIssueID(trimmedID), nil } +func resolveListIssueFilterFromSession(database *db.DB) (string, error) { + sess, err := session.GetOrCreate(database) + if err != nil { + return "", err + } + + issues, err := database.ListIssues(db.ListIssuesOptions{ + // Review-phase work still needs to resolve the epic root when the + // current session has already moved the issue into review. + Status: []models.Status{models.StatusInProgress, models.StatusInReview}, + Implementer: sess.ID, + }) + if err != nil { + return "", err + } + if resolved, err := resolveCommonIssueRootAncestorID(database, issueIDsFromIssues(issues)); err != nil || resolved != "" { + return resolved, err + } + + // Fall back to the session's logged issue history so the dot path keeps + // working after review transitions have cleared the focused work item. + sessionLogIDs, err := database.GetIssueSessionLog(sess.ID) + if err != nil { + return "", err + } + + return resolveCommonIssueRootAncestorID(database, sessionLogIDs) +} + +func resolveListIssueFilterFromWorkSession(database *db.DB, baseDir string) (string, error) { + wsID, err := config.GetActiveWorkSession(baseDir) + if err != nil || strings.TrimSpace(wsID) == "" { + return "", err + } + + issueIDs, err := database.GetWorkSessionIssues(wsID) + if err != nil { + return "", err + } + + return resolveCommonIssueRootAncestorID(database, issueIDs) +} + +func issueIDsFromIssues(issues []models.Issue) []string { + ids := make([]string, 0, len(issues)) + for _, issue := range issues { + ids = append(ids, issue.ID) + } + return ids +} + +func resolveCommonIssueRootAncestorID(database *db.DB, issueIDs []string) (string, error) { + resolved := "" + for _, issueID := range issueIDs { + rootID, err := resolveIssueRootAncestorID(database, issueID) + if err != nil { + return "", err + } + if rootID == "" { + continue + } + if resolved == "" { + resolved = rootID + continue + } + if resolved != rootID { + return "", fmt.Errorf("multiple issue roots found") + } + } + return resolved, nil +} + +func resolveIssueRootAncestorID(database *db.DB, issueID string) (string, error) { + currentID := db.NormalizeIssueID(strings.TrimSpace(issueID)) + if currentID == "" { + return "", nil + } + + seen := map[string]bool{} + for { + if seen[currentID] { + return "", fmt.Errorf("cycle detected while resolving issue root for %s", currentID) + } + seen[currentID] = true + + issue, err := database.GetIssue(currentID) + if err != nil { + return "", err + } + if strings.TrimSpace(issue.ParentID) == "" { + return issue.ID, nil + } + currentID = issue.ParentID + } +} + func init() { rootCmd.AddCommand(listCmd) rootCmd.AddCommand(reviewableCmd) diff --git a/cmd/list_test.go b/cmd/list_test.go index 50042a8..fdbb2dd 100644 --- a/cmd/list_test.go +++ b/cmd/list_test.go @@ -8,6 +8,7 @@ import ( "github.com/marcus/td/internal/config" "github.com/marcus/td/internal/db" "github.com/marcus/td/internal/models" + "github.com/marcus/td/internal/session" ) func TestParseDateFilter(t *testing.T) { @@ -517,6 +518,12 @@ func TestStatusAllVariations(t *testing.T) { func TestResolveListIssueFilterID(t *testing.T) { dir := t.TempDir() + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + if err := config.SetFocus(dir, "td-focused"); err != nil { t.Fatalf("SetFocus failed: %v", err) } @@ -549,7 +556,7 @@ func TestResolveListIssueFilterID(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got, err := resolveListIssueFilterID(dir, tc.rawID, tc.flagName) + got, err := resolveListIssueFilterID(database, dir, tc.rawID, tc.flagName) if err != nil { t.Fatalf("resolveListIssueFilterID returned error: %v", err) } @@ -562,12 +569,216 @@ func TestResolveListIssueFilterID(t *testing.T) { func TestResolveListIssueFilterIDDotRequiresFocus(t *testing.T) { dir := t.TempDir() + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() - _, err := resolveListIssueFilterID(dir, ".", "epic") + _, err = resolveListIssueFilterID(database, dir, ".", "epic") if err == nil { - t.Fatal("expected error when resolving --epic . without focus") + t.Fatal("expected error when resolving --epic . without session context") } - if !strings.Contains(err.Error(), "--epic . requires a focused issue") { + if !strings.Contains(err.Error(), "--epic . requires a focused issue, recent session activity, or an active work session") { t.Fatalf("unexpected error: %v", err) } } + +func TestResolveListIssueFilterIDDotUsesSessionLogHistory(t *testing.T) { + dir := t.TempDir() + t.Setenv("TD_SESSION_ID", "ses_cmd_test") + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + sess, err := session.GetOrCreate(database) + if err != nil { + t.Fatalf("GetOrCreate failed: %v", err) + } + + epic := &models.Issue{ + Title: "Session log epic", + Type: models.TypeEpic, + Status: models.StatusOpen, + } + if err := database.CreateIssue(epic); err != nil { + t.Fatalf("CreateIssue epic failed: %v", err) + } + + child := &models.Issue{ + Title: "Session log child", + Status: models.StatusClosed, + ParentID: epic.ID, + } + if err := database.CreateIssue(child); err != nil { + t.Fatalf("CreateIssue child failed: %v", err) + } + + if err := database.AddLog(&models.Log{ + IssueID: child.ID, + SessionID: sess.ID, + Message: "Approved after review", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + got, err := resolveListIssueFilterID(database, dir, ".", "epic") + if err != nil { + t.Fatalf("resolveListIssueFilterID returned error: %v", err) + } + if got != epic.ID { + t.Fatalf("resolveListIssueFilterID returned %q, want epic root %q", got, epic.ID) + } +} + +func TestResolveListIssueFilterIDDotUsesCurrentSessionEpic(t *testing.T) { + dir := t.TempDir() + t.Setenv("TD_SESSION_ID", "ses_cmd_test") + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + sess, err := session.GetOrCreate(database) + if err != nil { + t.Fatalf("GetOrCreate failed: %v", err) + } + + epic := &models.Issue{ + Title: "Session epic", + Type: models.TypeEpic, + Status: models.StatusInProgress, + ImplementerSession: sess.ID, + } + if err := database.CreateIssue(epic); err != nil { + t.Fatalf("CreateIssue epic failed: %v", err) + } + if err := database.UpdateIssue(epic); err != nil { + t.Fatalf("UpdateIssue epic failed: %v", err) + } + + child := &models.Issue{ + Title: "Session child", + ParentID: epic.ID, + Status: models.StatusInProgress, + ImplementerSession: sess.ID, + } + if err := database.CreateIssue(child); err != nil { + t.Fatalf("CreateIssue child failed: %v", err) + } + if err := database.UpdateIssue(child); err != nil { + t.Fatalf("UpdateIssue child failed: %v", err) + } + + got, err := resolveListIssueFilterID(database, dir, ".", "epic") + if err != nil { + t.Fatalf("resolveListIssueFilterID returned error: %v", err) + } + if got != epic.ID { + t.Fatalf("resolveListIssueFilterID returned %q, want epic root %q", got, epic.ID) + } +} + +func TestResolveListIssueFilterIDDotUsesCurrentSessionReviewPhaseEpic(t *testing.T) { + dir := t.TempDir() + t.Setenv("TD_SESSION_ID", "ses_cmd_test") + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + sess, err := session.GetOrCreate(database) + if err != nil { + t.Fatalf("GetOrCreate failed: %v", err) + } + + epic := &models.Issue{ + Title: "Review phase epic", + Type: models.TypeEpic, + Status: models.StatusInProgress, + ImplementerSession: sess.ID, + } + if err := database.CreateIssue(epic); err != nil { + t.Fatalf("CreateIssue epic failed: %v", err) + } + if err := database.UpdateIssue(epic); err != nil { + t.Fatalf("UpdateIssue epic failed: %v", err) + } + + reviewTask := &models.Issue{ + Title: "Review phase child", + ParentID: epic.ID, + Status: models.StatusInReview, + ImplementerSession: sess.ID, + } + if err := database.CreateIssue(reviewTask); err != nil { + t.Fatalf("CreateIssue review task failed: %v", err) + } + if err := database.UpdateIssue(reviewTask); err != nil { + t.Fatalf("UpdateIssue review task failed: %v", err) + } + + got, err := resolveListIssueFilterID(database, dir, ".", "epic") + if err != nil { + t.Fatalf("resolveListIssueFilterID returned error: %v", err) + } + if got != epic.ID { + t.Fatalf("resolveListIssueFilterID returned %q, want epic root %q", got, epic.ID) + } +} + +func TestResolveListIssueFilterIDDotUsesActiveWorkSessionEpic(t *testing.T) { + dir := t.TempDir() + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + sess, err := session.GetOrCreate(database) + if err != nil { + t.Fatalf("GetOrCreate failed: %v", err) + } + + ws := &models.WorkSession{Name: "dispatch-loop", SessionID: sess.ID} + if err := database.CreateWorkSession(ws); err != nil { + t.Fatalf("CreateWorkSession failed: %v", err) + } + if err := config.SetActiveWorkSession(dir, ws.ID); err != nil { + t.Fatalf("SetActiveWorkSession failed: %v", err) + } + + epic := &models.Issue{ + Title: "Work session epic", + Type: models.TypeEpic, + Status: models.StatusOpen, + } + if err := database.CreateIssue(epic); err != nil { + t.Fatalf("CreateIssue epic failed: %v", err) + } + + child := &models.Issue{ + Title: "Work session child", + ParentID: epic.ID, + Status: models.StatusInProgress, + } + if err := database.CreateIssue(child); err != nil { + t.Fatalf("CreateIssue child failed: %v", err) + } + if err := database.TagIssueToWorkSession(ws.ID, child.ID, sess.ID); err != nil { + t.Fatalf("TagIssueToWorkSession failed: %v", err) + } + + got, err := resolveListIssueFilterID(database, dir, ".", "epic") + if err != nil { + t.Fatalf("resolveListIssueFilterID returned error: %v", err) + } + if got != epic.ID { + t.Fatalf("resolveListIssueFilterID returned %q, want epic root %q", got, epic.ID) + } +} diff --git a/cmd/multivalue_flags_test.go b/cmd/multivalue_flags_test.go new file mode 100644 index 0000000..9b17f7c --- /dev/null +++ b/cmd/multivalue_flags_test.go @@ -0,0 +1,247 @@ +package cmd + +import ( + "reflect" + "testing" +) + +// TestMergeMultiValueFlag tests the mergeMultiValueFlag helper function +func TestMergeMultiValueFlag(t *testing.T) { + tests := []struct { + name string + input []string + expect []string + }{ + { + name: "single comma-separated value", + input: []string{"dispatch,agent:claude"}, + expect: []string{"dispatch", "agent:claude"}, + }, + { + name: "repeated flags", + input: []string{"dispatch", "agent:claude"}, + expect: []string{"dispatch", "agent:claude"}, + }, + { + name: "mixed repeated and comma-separated", + input: []string{"dispatch,agent:claude", "backend"}, + expect: []string{"dispatch", "agent:claude", "backend"}, + }, + { + name: "deduplication", + input: []string{"dispatch", "dispatch"}, + expect: []string{"dispatch"}, + }, + { + name: "deduplication across comma and repeated", + input: []string{"a,b", "b,c"}, + expect: []string{"a", "b", "c"}, + }, + { + name: "whitespace trimming", + input: []string{" dispatch , agent:claude "}, + expect: []string{"dispatch", "agent:claude"}, + }, + { + name: "empty strings filtered", + input: []string{"", "dispatch", ""}, + expect: []string{"dispatch"}, + }, + { + name: "all empty returns nil", + input: []string{"", ""}, + expect: nil, + }, + { + name: "nil input returns nil", + input: nil, + expect: nil, + }, + { + name: "single value no comma", + input: []string{"dispatch"}, + expect: []string{"dispatch"}, + }, + { + name: "trailing comma", + input: []string{"dispatch,"}, + expect: []string{"dispatch"}, + }, + { + name: "leading comma", + input: []string{",dispatch"}, + expect: []string{"dispatch"}, + }, + { + name: "three repeated flags", + input: []string{"a", "b", "c"}, + expect: []string{"a", "b", "c"}, + }, + { + name: "issue IDs for depends-on", + input: []string{"td-abc123", "td-def456"}, + expect: []string{"td-abc123", "td-def456"}, + }, + { + name: "comma-separated issue IDs", + input: []string{"td-abc123,td-def456"}, + expect: []string{"td-abc123", "td-def456"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := mergeMultiValueFlag(tt.input) + if !reflect.DeepEqual(got, tt.expect) { + t.Errorf("mergeMultiValueFlag(%v) = %v, want %v", tt.input, got, tt.expect) + } + }) + } +} + +// TestCreateLabelsRepeatedFlag tests that create command labels flag accepts StringArray +func TestCreateLabelsRepeatedFlag(t *testing.T) { + flag := createCmd.Flags().Lookup("labels") + if flag == nil { + t.Fatal("Expected --labels flag to be defined") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --labels to be stringArray, got %s", flag.Value.Type()) + } + if flag.Shorthand != "l" { + t.Errorf("Expected --labels shorthand to be 'l', got %q", flag.Shorthand) + } +} + +// TestCreateDependsOnRepeatedFlag tests that create command depends-on flag accepts StringArray +func TestCreateDependsOnRepeatedFlag(t *testing.T) { + flag := createCmd.Flags().Lookup("depends-on") + if flag == nil { + t.Fatal("Expected --depends-on flag to be defined") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --depends-on to be stringArray, got %s", flag.Value.Type()) + } +} + +// TestCreateBlocksRepeatedFlag tests that create command blocks flag accepts StringArray +func TestCreateBlocksRepeatedFlag(t *testing.T) { + flag := createCmd.Flags().Lookup("blocks") + if flag == nil { + t.Fatal("Expected --blocks flag to be defined") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --blocks to be stringArray, got %s", flag.Value.Type()) + } +} + +// TestUpdateLabelsRepeatedFlag tests that update command labels flag accepts StringArray +func TestUpdateLabelsRepeatedFlag(t *testing.T) { + flag := updateCmd.Flags().Lookup("labels") + if flag == nil { + t.Fatal("Expected --labels flag to be defined on update command") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --labels to be stringArray, got %s", flag.Value.Type()) + } + if flag.Shorthand != "l" { + t.Errorf("Expected --labels shorthand to be 'l', got %q", flag.Shorthand) + } +} + +// TestUpdateDependsOnRepeatedFlag tests that update command depends-on flag accepts StringArray +func TestUpdateDependsOnRepeatedFlag(t *testing.T) { + flag := updateCmd.Flags().Lookup("depends-on") + if flag == nil { + t.Fatal("Expected --depends-on flag to be defined on update command") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --depends-on to be stringArray, got %s", flag.Value.Type()) + } +} + +// TestUpdateBlocksRepeatedFlag tests that update command blocks flag accepts StringArray +func TestUpdateBlocksRepeatedFlag(t *testing.T) { + flag := updateCmd.Flags().Lookup("blocks") + if flag == nil { + t.Fatal("Expected --blocks flag to be defined on update command") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --blocks to be stringArray, got %s", flag.Value.Type()) + } +} + +// TestTaskCreateLabelsRepeatedFlag tests that task create command labels flag accepts StringArray +func TestTaskCreateLabelsRepeatedFlag(t *testing.T) { + flag := taskCreateCmd.Flags().Lookup("labels") + if flag == nil { + t.Fatal("Expected --labels flag to be defined on task create command") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --labels to be stringArray, got %s", flag.Value.Type()) + } + if flag.Shorthand != "l" { + t.Errorf("Expected --labels shorthand to be 'l', got %q", flag.Shorthand) + } +} + +// TestEpicCreateLabelsRepeatedFlag tests that epic create command labels flag accepts StringArray +func TestEpicCreateLabelsRepeatedFlag(t *testing.T) { + flag := epicCreateCmd.Flags().Lookup("labels") + if flag == nil { + t.Fatal("Expected --labels flag to be defined on epic create command") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --labels to be stringArray, got %s", flag.Value.Type()) + } + if flag.Shorthand != "l" { + t.Errorf("Expected --labels shorthand to be 'l', got %q", flag.Shorthand) + } +} + +// TestLabelAliasesAreStringArray tests that label alias flags are also StringArray +func TestLabelAliasesAreStringArray(t *testing.T) { + aliases := []string{"label", "tags", "tag"} + for _, alias := range aliases { + flag := createCmd.Flags().Lookup(alias) + if flag == nil { + t.Errorf("Expected --%s flag to be defined", alias) + continue + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected --%s to be stringArray, got %s", alias, flag.Value.Type()) + } + } +} + +// TestListLabelsUnchanged tests that list command labels flag is still StringArrayP (was already correct) +func TestListLabelsUnchanged(t *testing.T) { + flag := listCmd.Flags().Lookup("labels") + if flag == nil { + t.Fatal("Expected --labels flag to be defined on list command") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected list --labels to be stringArray, got %s", flag.Value.Type()) + } +} + +// TestSearchLabelsUnchanged tests that search command labels flag is still StringArrayP (was already correct) +func TestSearchLabelsUnchanged(t *testing.T) { + flag := searchCmd.Flags().Lookup("labels") + if flag == nil { + t.Fatal("Expected --labels flag to be defined on search command") + } + if flag.Value.Type() != "stringArray" { + t.Errorf("Expected search --labels to be stringArray, got %s", flag.Value.Type()) + } +} + +// TestMergeMultiValueFlagPreservesOrder tests that order is preserved (first seen wins) +func TestMergeMultiValueFlagPreservesOrder(t *testing.T) { + input := []string{"c", "a", "b"} + got := mergeMultiValueFlag(input) + expect := []string{"c", "a", "b"} + if !reflect.DeepEqual(got, expect) { + t.Errorf("Expected order preserved: got %v, want %v", got, expect) + } +} diff --git a/cmd/reject_test.go b/cmd/reject_test.go new file mode 100644 index 0000000..4c7479e --- /dev/null +++ b/cmd/reject_test.go @@ -0,0 +1,96 @@ +package cmd + +import ( + "bytes" + "io" + "os" + "strings" + "testing" + + "github.com/marcus/td/internal/db" + "github.com/marcus/td/internal/models" + "github.com/marcus/td/internal/session" +) + +func runRejectCommand(t *testing.T, dir string, args ...string) string { + t.Helper() + + saveAndRestoreGlobals(t) + t.Setenv("TD_SESSION_ID", "ses_reject_cmd") + + baseDir := dir + baseDirOverride = &baseDir + + _ = rejectCmd.Flags().Set("json", "false") + _ = rejectCmd.Flags().Set("reason", "") + _ = rejectCmd.Flags().Set("comment", "") + _ = rejectCmd.Flags().Set("message", "") + _ = rejectCmd.Flags().Set("note", "") + _ = rejectCmd.Flags().Set("notes", "") + + var output bytes.Buffer + oldStdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + os.Stdout = w + + runErr := rejectCmd.RunE(rejectCmd, args) + + _ = w.Close() + os.Stdout = oldStdout + _, _ = io.Copy(&output, r) + + if runErr != nil { + t.Fatalf("rejectCmd.RunE returned error: %v", runErr) + } + + return output.String() +} + +func TestRejectOpenIssueIsIdempotent(t *testing.T) { + dir := t.TempDir() + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Already reopened review target", + Status: models.StatusOpen, + ImplementerSession: "ses_impl", + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + if err := database.UpdateIssue(issue); err != nil { + t.Fatalf("UpdateIssue failed: %v", err) + } + + sess, err := session.GetOrCreate(database) + if err != nil { + t.Fatalf("GetOrCreate failed: %v", err) + } + if sess.ID == "" { + t.Fatal("expected active session") + } + + output := runRejectCommand(t, dir, issue.ID) + if !strings.Contains(output, "already reopened") { + t.Fatalf("expected idempotent reject output, got %s", output) + } + + updated, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if updated.Status != models.StatusOpen { + t.Fatalf("status = %s, want %s", updated.Status, models.StatusOpen) + } + if updated.ImplementerSession != "ses_impl" { + t.Fatalf("implementer session = %q, want %q", updated.ImplementerSession, "ses_impl") + } +} diff --git a/cmd/review.go b/cmd/review.go index baf3ef7..89b7bb4 100644 --- a/cmd/review.go +++ b/cmd/review.go @@ -1,7 +1,9 @@ package cmd import ( + "errors" "fmt" + "strings" "time" "github.com/marcus/td/internal/config" @@ -27,10 +29,25 @@ type SubmitReviewResult struct { Message string } +const autoReviewHandoffMessage = "Auto-generated for review submission" + +func newAutoReviewHandoff(issueID, sessionID string) *models.Handoff { + return &models.Handoff{ + IssueID: issueID, + SessionID: sessionID, + Done: []string{autoReviewHandoffMessage}, + Remaining: []string{}, + Decisions: []string{}, + Uncertain: []string{}, + } +} + // submitIssueForReview submits a single issue for review with proper validation, // logging, and undo support. This is the shared logic for both reviewCmd and // ws handoff --review. func submitIssueForReview(database *db.DB, issue *models.Issue, sess *session.Session, baseDir string, logMsg string) SubmitReviewResult { + fromStatus := issue.Status + // Validate transition with state machine sm := workflow.DefaultMachine() ctx := &workflow.TransitionContext{ @@ -42,6 +59,12 @@ func submitIssueForReview(database *db.DB, issue *models.Issue, sess *session.Se } _, err := sm.Validate(ctx) if err != nil { + if issue.Status == models.StatusInReview { + return SubmitReviewResult{ + Success: false, + Message: fmt.Sprintf("cannot review %s: already in review", issue.ID) + "\n" + reviewFollowupGuidance(issue), + } + } return SubmitReviewResult{ Success: false, Message: fmt.Sprintf("cannot review %s: %v", issue.ID, err), @@ -60,10 +83,10 @@ func submitIssueForReview(database *db.DB, issue *models.Issue, sess *session.Se issue.ImplementerSession = sess.ID } - if err := database.UpdateIssueLogged(issue, sess.ID, models.ActionReview); err != nil { + if err := database.UpdateIssueLoggedIfStatus(issue, fromStatus, sess.ID, models.ActionReview); err != nil { return SubmitReviewResult{ Success: false, - Message: fmt.Sprintf("failed to update %s: %v", issue.ID, err), + Message: describeStaleTransitionUpdate(database, "review", issue.ID, err, reviewFollowupGuidance), } } @@ -86,6 +109,79 @@ func submitIssueForReview(database *db.DB, issue *models.Issue, sess *session.Se return SubmitReviewResult{Success: true} } +func shouldWarnAboutAutoHandoff(database *db.DB, issueID, sessionID string) bool { + logs, err := database.GetLogs(issueID, 10) + if err != nil { + return true + } + + const substantiveContextChars = 16 + totalChars := 0 + for _, log := range logs { + if log.SessionID != sessionID { + continue + } + + if !isSubstantiveReviewContextLog(log) { + continue + } + + totalChars += len([]rune(strings.TrimSpace(log.Message))) + if log.Type != models.LogTypeProgress { + return false + } + if totalChars >= substantiveContextChars { + return false + } + } + + return true +} + +func isSubstantiveReviewContextLog(log models.Log) bool { + message := strings.TrimSpace(log.Message) + if message == "" { + return false + } + + switch log.Type { + case models.LogTypeDecision, models.LogTypeHypothesis, models.LogTypeTried, models.LogTypeResult, models.LogTypeBlocker, models.LogTypeSecurity: + return true + case models.LogTypeProgress, models.LogTypeOrchestration: + return !isRoutineWorkflowLogMessage(message) + default: + return true + } +} + +func isRoutineWorkflowLogMessage(message string) bool { + normalized := strings.ToLower(strings.TrimSpace(message)) + if normalized == "" { + return true + } + + switch normalized { + case "started work", "submitted for review", "approved", "rejected", "closed": + return true + } + + prefixes := []string{ + "submitted for review via ", + "approved: ", + "rejected: ", + "closed: ", + "cascaded review from ", + "auto-unblocked (", + } + for _, prefix := range prefixes { + if strings.HasPrefix(normalized, prefix) { + return true + } + } + + return false +} + var reviewCmd = &cobra.Command{ Use: "review [issue-id...]", Aliases: []string{"submit", "finish"}, @@ -143,14 +239,7 @@ Supports bulk operations: handoff, err := database.GetLatestHandoff(issueID) if err != nil || handoff == nil { // Auto-create minimal handoff - autoHandoff := &models.Handoff{ - IssueID: issueID, - SessionID: sess.ID, - Done: []string{"Auto-generated for review submission"}, - Remaining: []string{}, - Decisions: []string{}, - Uncertain: []string{}, - } + autoHandoff := newAutoReviewHandoff(issueID, sess.ID) if err := database.AddHandoff(autoHandoff); err != nil { if jsonOutput { output.JSONError(output.ErrCodeDatabaseError, fmt.Sprintf("failed to create handoff: %v", err)) @@ -160,7 +249,9 @@ Supports bulk operations: skipped++ continue } - output.Warning("auto-created minimal handoff for %s - consider using 'td handoff' for better documentation", issueID) + if shouldWarnAboutAutoHandoff(database, issueID, sess.ID) { + output.Warning("auto-created minimal handoff for %s - consider using 'td handoff' for better documentation", issueID) + } } // Handle --minor flag @@ -241,6 +332,156 @@ func approvalReason(cmd *cobra.Command) string { return "" } +func describeStaleTransitionUpdate(database *db.DB, action, issueID string, err error, guidance func(*models.Issue) string) string { + var staleErr *db.StaleIssueStatusError + if !errors.As(err, &staleErr) { + return fmt.Sprintf("failed to update %s: %v", issueID, err) + } + + current := &models.Issue{ID: issueID, Status: staleErr.Actual} + if database != nil { + if refreshed, refreshErr := database.GetIssue(issueID); refreshErr == nil { + current = refreshed + } + } + + lines := []string{ + fmt.Sprintf("cannot %s %s: status changed from %s to %s in another session", action, issueID, staleErr.Expected, current.Status), + fmt.Sprintf(" Current status: %s", current.Status), + } + if recent := recentWorkflowTransitionContext(database, issueID); recent != "" { + lines = append(lines, recent) + } + lines = append(lines, guidance(current)) + return strings.Join(lines, "\n") +} + +func recentWorkflowTransitionContext(database *db.DB, issueID string) string { + if database == nil { + return "" + } + + logs, err := database.GetLogs(issueID, 5) + if err != nil { + return "" + } + + for i := len(logs) - 1; i >= 0; i-- { + log := logs[i] + if summary, ok := summarizeWorkflowTransition(log.Message); ok { + return fmt.Sprintf(" Recent transition: %s by %s at %s", summary, log.SessionID, log.Timestamp.Format("2006-01-02 15:04")) + } + } + + return "" +} + +func summarizeWorkflowTransition(message string) (string, bool) { + normalized := strings.ToLower(strings.TrimSpace(message)) + switch { + case normalized == "approved" || strings.HasPrefix(normalized, "approved:") || strings.Contains(normalized, "] approved "): + return "approved", true + case normalized == "rejected" || strings.HasPrefix(normalized, "rejected:"): + return "rejected", true + case normalized == "reopened" || strings.HasPrefix(normalized, "reopened:"): + return "reopened", true + case normalized == "closed" || strings.HasPrefix(normalized, "closed:") || strings.Contains(normalized, "] closed "): + return "closed", true + case normalized == "submitted for review" || strings.HasPrefix(normalized, "submitted for review:") || strings.HasPrefix(normalized, "cascaded review from "): + return "submitted for review", true + default: + return "", false + } +} + +func describeReviewerNoop(database *db.DB, action string, issue *models.Issue) string { + if issue == nil { + return "" + } + + header := "" + switch action { + case "approve": + header = fmt.Sprintf("already approved/closed %s", issue.ID) + case "reject": + header = fmt.Sprintf("already reopened %s", issue.ID) + default: + header = fmt.Sprintf("already handled %s", issue.ID) + } + + lines := []string{ + header, + fmt.Sprintf(" Current status: %s", issue.Status), + } + if recent := recentWorkflowTransitionContext(database, issue.ID); recent != "" { + lines = append(lines, recent) + } + if action == "approve" { + lines = append(lines, approveFollowupGuidance(issue)) + } else { + lines = append(lines, rejectFollowupGuidance(issue)) + } + return strings.Join(lines, "\n") +} + +func closeFollowupGuidance(issue *models.Issue) string { + if issue == nil { + return " Submit for review: td review " + } + switch issue.Status { + case models.StatusInReview: + return fmt.Sprintf(" Already in review: td approve %s", issue.ID) + case models.StatusClosed: + return fmt.Sprintf(" Already closed: td show %s", issue.ID) + } + return fmt.Sprintf(" Submit for review: td review %s", issue.ID) +} + +// reviewFollowupGuidance returns the next workflow command after a failed +// review submission attempt. +func reviewFollowupGuidance(issue *models.Issue) string { + if issue == nil { + return " Submit for review: td review " + } + switch issue.Status { + case models.StatusInReview: + return fmt.Sprintf(" Already in review: td approve %s", issue.ID) + case models.StatusClosed: + return fmt.Sprintf(" Already closed: td show %s", issue.ID) + } + return fmt.Sprintf(" Submit for review: td review %s", issue.ID) +} + +// approveFollowupGuidance returns the next workflow command after a failed +// approval attempt. +func approveFollowupGuidance(issue *models.Issue) string { + if issue == nil { + return " Submit for review first: td review " + } + switch issue.Status { + case models.StatusInReview: + return fmt.Sprintf(" Approve it: td approve %s", issue.ID) + case models.StatusClosed: + return fmt.Sprintf(" Already approved/closed: td show %s", issue.ID) + } + return fmt.Sprintf(" Submit for review first: td review %s", issue.ID) +} + +func rejectFollowupGuidance(issue *models.Issue) string { + if issue == nil { + return " Already reopened: td show " + } + switch issue.Status { + case models.StatusOpen: + return fmt.Sprintf(" Already reopened: td show %s", issue.ID) + case models.StatusInReview: + return fmt.Sprintf(" Reject it: td reject %s", issue.ID) + case models.StatusClosed: + return fmt.Sprintf(" Already closed: td show %s", issue.ID) + } + return fmt.Sprintf(" Submit for review first: td review %s", issue.ID) +} + var approveCmd = &cobra.Command{ Use: "approve [issue-id...]", Short: "Approve and close one or more issues", @@ -285,6 +526,28 @@ Supports bulk operations: } } else { issueIDs = args + if len(issueIDs) == 0 { + issues, err := database.ListIssues(reviewableByOptions(baseDir, sess.ID)) + if err != nil { + output.Error("failed to list reviewable issues: %v", err) + return err + } + switch len(issues) { + case 0: + output.Error("no issues to approve. Provide issue IDs or use --all") + return fmt.Errorf("no issues specified") + case 1: + issueIDs = []string{issues[0].ID} + default: + output.Error("no issue ID specified. Multiple issues awaiting your review:") + for _, issue := range issues { + fmt.Printf(" %s: %s\n", issue.ID, issue.Title) + } + fmt.Printf("\nUsage: td approve \n") + fmt.Printf("Or use: td approve --all\n") + return fmt.Errorf("issue ID required") + } + } } if len(issueIDs) == 0 { @@ -308,12 +571,30 @@ Supports bulk operations: // Validate transition with state machine sm := workflow.DefaultMachine() + if issue.Status == models.StatusClosed { + message := describeReviewerNoop(database, "approve", issue) + if jsonOutput { + if err := output.JSON(map[string]interface{}{ + "id": issueID, + "status": string(issue.Status), + "action": "already approved/closed", + "message": message, + }); err != nil { + output.JSONError(output.ErrCodeDatabaseError, err.Error()) + } + } else if !all { + output.Warning("%s", message) + } + skipped++ + continue + } if !sm.IsValidTransition(issue.Status, models.StatusClosed) { if !all { if jsonOutput { output.JSONError(output.ErrCodeDatabaseError, fmt.Sprintf("cannot approve %s: invalid transition from %s", issueID, issue.Status)) } else { output.Warning("cannot approve %s: invalid transition from %s", issueID, issue.Status) + fmt.Println(approveFollowupGuidance(issue)) } } skipped++ @@ -372,8 +653,8 @@ Supports bulk operations: now := time.Now() issue.ClosedAt = &now - if err := database.UpdateIssueLogged(issue, sess.ID, models.ActionApprove); err != nil { - output.Warning("failed to update %s: %v", issueID, err) + if err := database.UpdateIssueLoggedIfStatus(issue, models.StatusInReview, sess.ID, models.ActionApprove); err != nil { + output.Warning("%s", describeStaleTransitionUpdate(database, "approve", issueID, err, approveFollowupGuidance)) skipped++ continue } @@ -387,7 +668,7 @@ Supports bulk operations: logMsg := "Approved" logType := models.LogTypeProgress if reason != "" { - logMsg = reason + logMsg = "Approved: " + reason } if eligibility.CreatorException { agentInfo := sess.AgentType @@ -496,6 +777,23 @@ Supports bulk operations: } // Reject is only valid from in_review (matches HTTP API behavior) + if issue.Status == models.StatusOpen { + message := describeReviewerNoop(database, "reject", issue) + if jsonOutput { + if err := output.JSON(map[string]interface{}{ + "id": issueID, + "status": string(issue.Status), + "action": "already reopened", + "message": message, + }); err != nil { + output.JSONError(output.ErrCodeDatabaseError, err.Error()) + } + } else { + output.Warning("%s", message) + } + skipped++ + continue + } if issue.Status != models.StatusInReview { if jsonOutput { output.JSONError(output.ErrCodeDatabaseError, fmt.Sprintf("cannot reject %s: must be in_review (currently %s)", issueID, issue.Status)) @@ -510,11 +808,11 @@ Supports bulk operations: issue.Status = models.StatusOpen issue.ImplementerSession = "" - if err := database.UpdateIssueLogged(issue, sess.ID, models.ActionReject); err != nil { + if err := database.UpdateIssueLoggedIfStatus(issue, models.StatusInReview, sess.ID, models.ActionReject); err != nil { if jsonOutput { output.JSONError(output.ErrCodeDatabaseError, err.Error()) } else { - output.Warning("failed to update %s: %v", issueID, err) + output.Warning("%s", describeStaleTransitionUpdate(database, "reject", issueID, err, rejectFollowupGuidance)) } skipped++ continue @@ -647,7 +945,7 @@ Examples: if !eligibility.Allowed { if selfCloseException == "" { output.Error("%s", eligibility.RejectionMessage) - output.Error(" Submit for review: td review %s", issueID) + output.Error("%s", closeFollowupGuidance(issue)) skipped++ continue } @@ -656,12 +954,13 @@ Examples: } // Update issue (atomic update + action log) + fromStatus := issue.Status issue.Status = models.StatusClosed now := time.Now() issue.ClosedAt = &now - if err := database.UpdateIssueLogged(issue, sess.ID, models.ActionClose); err != nil { - output.Warning("failed to update %s: %v", issueID, err) + if err := database.UpdateIssueLoggedIfStatus(issue, fromStatus, sess.ID, models.ActionClose); err != nil { + output.Warning("%s", describeStaleTransitionUpdate(database, "close", issueID, err, closeFollowupGuidance)) skipped++ continue } diff --git a/cmd/review_policy_test.go b/cmd/review_policy_test.go index 73df899..c2ea4a7 100644 --- a/cmd/review_policy_test.go +++ b/cmd/review_policy_test.go @@ -1,9 +1,12 @@ package cmd import ( + "strings" "testing" + "time" "github.com/marcus/td/internal/config" + "github.com/marcus/td/internal/db" "github.com/marcus/td/internal/features" "github.com/marcus/td/internal/models" ) @@ -252,3 +255,303 @@ func TestEvaluateCloseEligibility(t *testing.T) { }) } } + +func TestCloseFollowupGuidance(t *testing.T) { + tests := []struct { + name string + issue *models.Issue + want string + }{ + { + name: "open issue points to review", + issue: &models.Issue{ID: "td-open", Status: models.StatusOpen}, + want: " Submit for review: td review td-open", + }, + { + name: "in progress issue points to review", + issue: &models.Issue{ID: "td-progress", Status: models.StatusInProgress}, + want: " Submit for review: td review td-progress", + }, + { + name: "in review issue points to approve", + issue: &models.Issue{ID: "td-review", Status: models.StatusInReview}, + want: " Already in review: td approve td-review", + }, + { + name: "closed issue points to show", + issue: &models.Issue{ID: "td-closed", Status: models.StatusClosed}, + want: " Already closed: td show td-closed", + }, + { + name: "nil issue falls back to review wording", + issue: nil, + want: " Submit for review: td review ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := closeFollowupGuidance(tt.issue) + if got != tt.want { + t.Fatalf("closeFollowupGuidance() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestReviewFollowupGuidance(t *testing.T) { + tests := []struct { + name string + issue *models.Issue + want string + }{ + { + name: "open issue points to review", + issue: &models.Issue{ID: "td-open", Status: models.StatusOpen}, + want: " Submit for review: td review td-open", + }, + { + name: "in progress issue points to review", + issue: &models.Issue{ID: "td-progress", Status: models.StatusInProgress}, + want: " Submit for review: td review td-progress", + }, + { + name: "in review issue points to approve", + issue: &models.Issue{ID: "td-review", Status: models.StatusInReview}, + want: " Already in review: td approve td-review", + }, + { + name: "closed issue points to show", + issue: &models.Issue{ID: "td-closed", Status: models.StatusClosed}, + want: " Already closed: td show td-closed", + }, + { + name: "nil issue falls back to review wording", + issue: nil, + want: " Submit for review: td review ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := reviewFollowupGuidance(tt.issue) + if got != tt.want { + t.Fatalf("reviewFollowupGuidance() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestApproveFollowupGuidance(t *testing.T) { + tests := []struct { + name string + issue *models.Issue + want string + }{ + { + name: "in review issue points to approve", + issue: &models.Issue{ID: "td-review", Status: models.StatusInReview}, + want: " Approve it: td approve td-review", + }, + { + name: "open issue points to review first", + issue: &models.Issue{ID: "td-open", Status: models.StatusOpen}, + want: " Submit for review first: td review td-open", + }, + { + name: "closed issue points to show", + issue: &models.Issue{ID: "td-closed", Status: models.StatusClosed}, + want: " Already approved/closed: td show td-closed", + }, + { + name: "nil issue falls back to review wording", + issue: nil, + want: " Submit for review first: td review ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := approveFollowupGuidance(tt.issue) + if got != tt.want { + t.Fatalf("approveFollowupGuidance() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestRejectFollowupGuidance(t *testing.T) { + tests := []struct { + name string + issue *models.Issue + want string + }{ + { + name: "open issue points to show as reopened", + issue: &models.Issue{ID: "td-open", Status: models.StatusOpen}, + want: " Already reopened: td show td-open", + }, + { + name: "in review issue points to reject", + issue: &models.Issue{ID: "td-review", Status: models.StatusInReview}, + want: " Reject it: td reject td-review", + }, + { + name: "closed issue points to show", + issue: &models.Issue{ID: "td-closed", Status: models.StatusClosed}, + want: " Already closed: td show td-closed", + }, + { + name: "nil issue falls back to reopened wording", + issue: nil, + want: " Already reopened: td show ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := rejectFollowupGuidance(tt.issue) + if got != tt.want { + t.Fatalf("rejectFollowupGuidance() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestDescribeStaleTransitionUpdate(t *testing.T) { + dir := t.TempDir() + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Closed elsewhere", + Status: models.StatusClosed, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + msg := describeStaleTransitionUpdate( + database, + "approve", + issue.ID, + &db.StaleIssueStatusError{ + IssueID: issue.ID, + Expected: models.StatusInReview, + Actual: models.StatusClosed, + }, + approveFollowupGuidance, + ) + t.Log(msg) + + want := "cannot approve " + issue.ID + ": status changed from in_review to closed in another session\n Current status: closed\n Already approved/closed: td show " + issue.ID + if msg != want { + t.Fatalf("describeStaleTransitionUpdate() = %q, want %q", msg, want) + } +} + +func TestDescribeStaleTransitionUpdateIncludesRecentContext(t *testing.T) { + dir := t.TempDir() + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Reopened elsewhere", + Status: models.StatusOpen, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + if err := database.AddLog(&models.Log{ + IssueID: issue.ID, + SessionID: "ses_reviewer", + Message: "Reopened", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + msg := describeStaleTransitionUpdate( + database, + "reject", + issue.ID, + &db.StaleIssueStatusError{ + IssueID: issue.ID, + Expected: models.StatusInReview, + Actual: models.StatusOpen, + }, + rejectFollowupGuidance, + ) + t.Log(msg) + + if !strings.Contains(msg, "Current status: open") { + t.Fatalf("expected current status context in %q", msg) + } + if !strings.Contains(msg, "Recent transition: reopened by ses_reviewer") { + t.Fatalf("expected recent transition context in %q", msg) + } + if !strings.Contains(msg, "Already reopened: td show "+issue.ID) { + t.Fatalf("expected reopened guidance in %q", msg) + } +} + +func TestDescribeStaleTransitionUpdatePrefersNewestWorkflowContext(t *testing.T) { + dir := t.TempDir() + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Newest transition wins", + Status: models.StatusOpen, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + if err := database.AddLog(&models.Log{ + IssueID: issue.ID, + SessionID: "ses_impl", + Message: "Submitted for review", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog submitted failed: %v", err) + } + time.Sleep(10 * time.Millisecond) + if err := database.AddLog(&models.Log{ + IssueID: issue.ID, + SessionID: "ses_reviewer", + Message: "Rejected", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog rejected failed: %v", err) + } + + msg := describeStaleTransitionUpdate( + database, + "reject", + issue.ID, + &db.StaleIssueStatusError{ + IssueID: issue.ID, + Expected: models.StatusInReview, + Actual: models.StatusOpen, + }, + rejectFollowupGuidance, + ) + + if !strings.Contains(msg, "Recent transition: rejected by ses_reviewer") { + t.Fatalf("expected newest transition context in %q", msg) + } + if strings.Contains(msg, "Recent transition: submitted for review by ses_impl") { + t.Fatalf("expected stale transition context to be ignored in %q", msg) + } +} diff --git a/cmd/review_test.go b/cmd/review_test.go index 732847e..2b86198 100644 --- a/cmd/review_test.go +++ b/cmd/review_test.go @@ -1,11 +1,16 @@ package cmd import ( + "bytes" + "io" + "os" + "strings" "testing" "github.com/marcus/td/internal/config" "github.com/marcus/td/internal/db" "github.com/marcus/td/internal/models" + "github.com/marcus/td/internal/session" ) func TestClearFocusIfNeeded(t *testing.T) { @@ -86,6 +91,55 @@ func TestClearFocusIfNeededNoFocus(t *testing.T) { } } +func runReviewCommand(t *testing.T, dir string, args ...string) string { + t.Helper() + + saveAndRestoreGlobals(t) + t.Setenv("TD_SESSION_ID", "ses_review_cmd") + + baseDir := dir + baseDirOverride = &baseDir + + _ = reviewCmd.Flags().Set("json", "false") + _ = reviewCmd.Flags().Set("minor", "false") + _ = reviewCmd.Flags().Set("reason", "") + _ = reviewCmd.Flags().Set("message", "") + _ = reviewCmd.Flags().Set("comment", "") + _ = reviewCmd.Flags().Set("note", "") + _ = reviewCmd.Flags().Set("notes", "") + + var output bytes.Buffer + oldStdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + os.Stdout = w + + runErr := reviewCmd.RunE(reviewCmd, args) + + _ = w.Close() + os.Stdout = oldStdout + _, _ = io.Copy(&output, r) + + if runErr != nil { + t.Fatalf("reviewCmd.RunE returned error: %v", runErr) + } + + return output.String() +} + +func reviewCommandSessionID(t *testing.T, database *db.DB) string { + t.Helper() + + t.Setenv("TD_SESSION_ID", "ses_review_cmd") + sess, err := session.GetOrCreate(database) + if err != nil { + t.Fatalf("GetOrCreate failed: %v", err) + } + return sess.ID +} + func TestReviewRequiresHandoff(t *testing.T) { dir := t.TempDir() @@ -117,6 +171,53 @@ func TestReviewRequiresHandoff(t *testing.T) { // This test verifies the handoff check logic by checking database state } +func TestSubmitIssueForReviewDetectsStaleTransition(t *testing.T) { + dir := t.TempDir() + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Concurrent stale review issue", + Status: models.StatusOpen, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + staleIssue, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue stale snapshot failed: %v", err) + } + + current, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue current failed: %v", err) + } + current.Status = models.StatusClosed + if err := database.UpdateIssue(current); err != nil { + t.Fatalf("UpdateIssue failed: %v", err) + } + + sessionID := reviewCommandSessionID(t, database) + sess := &session.Session{ID: sessionID} + + result := submitIssueForReview(database, staleIssue, sess, dir, "") + if result.Success { + t.Fatal("expected stale transition to fail") + } + + want := "cannot review " + issue.ID + ": status changed from open to closed in another session\n Current status: closed\n Already closed: td show " + issue.ID + if result.Message != want { + t.Fatalf("message = %q, want %q", result.Message, want) + } + + t.Log(result.Message) +} + func TestApproveRequiresDifferentSession(t *testing.T) { dir := t.TempDir() @@ -652,7 +753,7 @@ func TestCascadeUpToReviewAllChildrenReview(t *testing.T) { database.UpdateIssue(child2) // Cascade up should now update epic - cascaded, _ := database.CascadeUpParentStatus( child2.ID, models.StatusInReview, sessionID) + cascaded, _ := database.CascadeUpParentStatus(child2.ID, models.StatusInReview, sessionID) if cascaded != 1 { t.Errorf("Expected 1 cascaded, got %d", cascaded) @@ -704,7 +805,7 @@ func TestCascadeUpToClosedAllChildrenClosed(t *testing.T) { sessionID := "ses_test" // All children closed, cascade up should update epic - cascaded, _ := database.CascadeUpParentStatus( child2.ID, models.StatusClosed, sessionID) + cascaded, _ := database.CascadeUpParentStatus(child2.ID, models.StatusClosed, sessionID) if cascaded != 1 { t.Errorf("Expected 1 cascaded, got %d", cascaded) @@ -761,7 +862,7 @@ func TestCascadeUpRecursive(t *testing.T) { // Child is only child of parent, parent is only child of grandparent // Cascade up from child should update both parent and grandparent - cascaded, _ := database.CascadeUpParentStatus( child.ID, models.StatusInReview, sessionID) + cascaded, _ := database.CascadeUpParentStatus(child.ID, models.StatusInReview, sessionID) if cascaded != 2 { t.Errorf("Expected 2 cascaded (parent + grandparent), got %d", cascaded) @@ -809,7 +910,7 @@ func TestCascadeUpNoActionNonEpicParent(t *testing.T) { sessionID := "ses_test" // Should NOT cascade up to non-epic parent - cascaded, _ := database.CascadeUpParentStatus( child.ID, models.StatusInReview, sessionID) + cascaded, _ := database.CascadeUpParentStatus(child.ID, models.StatusInReview, sessionID) if cascaded != 0 { t.Errorf("Expected 0 cascaded (parent not epic), got %d", cascaded) @@ -861,7 +962,7 @@ func TestCascadeUpNoActionNotAllChildrenReady(t *testing.T) { sessionID := "ses_test" // Should NOT cascade up because child2 is still open - cascaded, _ := database.CascadeUpParentStatus( child1.ID, models.StatusInReview, sessionID) + cascaded, _ := database.CascadeUpParentStatus(child1.ID, models.StatusInReview, sessionID) if cascaded != 0 { t.Errorf("Expected 0 cascaded (not all children ready), got %d", cascaded) @@ -913,7 +1014,7 @@ func TestCascadeUpReviewAllowsClosedSiblings(t *testing.T) { sessionID := "ses_test" // For in_review target, closed siblings should count as "ready" - cascaded, _ := database.CascadeUpParentStatus( child1.ID, models.StatusInReview, sessionID) + cascaded, _ := database.CascadeUpParentStatus(child1.ID, models.StatusInReview, sessionID) if cascaded != 1 { t.Errorf("Expected 1 cascaded, got %d", cascaded) @@ -1181,7 +1282,7 @@ func TestCascadeUpNoActionNoParent(t *testing.T) { sessionID := "ses_test" // Should return 0 since no parent - cascaded, _ := database.CascadeUpParentStatus( task.ID, models.StatusInReview, sessionID) + cascaded, _ := database.CascadeUpParentStatus(task.ID, models.StatusInReview, sessionID) if cascaded != 0 { t.Errorf("Expected 0 cascaded (no parent), got %d", cascaded) @@ -1262,6 +1363,168 @@ func TestReviewAutoCreatesHandoffWhenMissing(t *testing.T) { } } +func TestReviewWarnsWhenAutoCreatingHandoffWithoutContext(t *testing.T) { + dir := t.TempDir() + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Needs review handoff", + Status: models.StatusInProgress, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + output := runReviewCommand(t, dir, issue.ID) + + if !strings.Contains(output, "Warning: auto-created minimal handoff for "+issue.ID) { + t.Fatalf("expected auto-handoff warning, got %q", output) + } + if !strings.Contains(output, "REVIEW REQUESTED "+issue.ID) { + t.Fatalf("expected review output for %q, got %q", issue.ID, output) + } + + handoff, err := database.GetLatestHandoff(issue.ID) + if err != nil { + t.Fatalf("GetLatestHandoff failed: %v", err) + } + if handoff == nil { + t.Fatal("expected auto-created handoff") + } +} + +func TestReviewWarnsWhenOnlyRoutineWorkflowLogsExist(t *testing.T) { + dir := t.TempDir() + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Routine logs only", + Status: models.StatusInProgress, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + sessionID := reviewCommandSessionID(t, database) + if err := database.AddLog(&models.Log{ + IssueID: issue.ID, + SessionID: sessionID, + Message: "Started work", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + output := runReviewCommand(t, dir, issue.ID) + + if !strings.Contains(output, "Warning: auto-created minimal handoff for "+issue.ID) { + t.Fatalf("expected warning to remain for routine logs, got %q", output) + } +} + +func TestReviewWarnsWhenSubstantiveLogsBelongToDifferentSession(t *testing.T) { + dir := t.TempDir() + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Different session context", + Status: models.StatusInProgress, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + if err := database.AddLog(&models.Log{ + IssueID: issue.ID, + SessionID: "ses_other_context", + Message: "Implemented retry handling and added regression coverage", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + output := runReviewCommand(t, dir, issue.ID) + + if !strings.Contains(output, "Warning: auto-created minimal handoff for "+issue.ID) { + t.Fatalf("expected warning when substantive logs are from another session, got %q", output) + } + if !strings.Contains(output, "REVIEW REQUESTED "+issue.ID) { + t.Fatalf("expected review output for %q, got %q", issue.ID, output) + } +} + +func TestReviewSuppressesAutoHandoffWarningWhenWorkSessionContextExists(t *testing.T) { + dir := t.TempDir() + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Work session context exists", + Status: models.StatusInProgress, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + sessionID := reviewCommandSessionID(t, database) + ws := &models.WorkSession{ + ID: "ws-review-proof", + Name: "Review proof session", + SessionID: sessionID, + } + if err := database.CreateWorkSession(ws); err != nil { + t.Fatalf("CreateWorkSession failed: %v", err) + } + if err := database.TagIssueToWorkSession(ws.ID, issue.ID, "review-proof"); err != nil { + t.Fatalf("TagIssueToWorkSession failed: %v", err) + } + if err := database.AddLog(&models.Log{ + IssueID: "", + SessionID: sessionID, + WorkSessionID: ws.ID, + Message: "Validated review flow and captured remaining edge cases", + Type: models.LogTypeProgress, + }); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + output := runReviewCommand(t, dir, issue.ID) + + if strings.Contains(output, "Warning: auto-created minimal handoff for "+issue.ID) { + t.Fatalf("expected warning suppression when substantive work-session context exists, got %q", output) + } + if !strings.Contains(output, "REVIEW REQUESTED "+issue.ID) { + t.Fatalf("expected review output for %q, got %q", issue.ID, output) + } + + handoff, err := database.GetLatestHandoff(issue.ID) + if err != nil { + t.Fatalf("GetLatestHandoff failed: %v", err) + } + if handoff == nil { + t.Fatal("expected auto-created handoff") + } +} + func TestReviewPreservesExistingHandoff(t *testing.T) { dir := t.TempDir() diff --git a/cmd/rich_text_input.go b/cmd/rich_text_input.go new file mode 100644 index 0000000..e9f10b3 --- /dev/null +++ b/cmd/rich_text_input.go @@ -0,0 +1,76 @@ +package cmd + +import ( + "errors" + "fmt" + "os" + "strings" + + "github.com/marcus/td/internal/input" + "github.com/spf13/cobra" +) + +func resolveRichTextField(cmd *cobra.Command, inlineFlags []string, fileFlag string, stdinUsed bool) (string, bool, bool, error) { + inlineProvided := false + inlineValue := "" + for _, name := range inlineFlags { + if cmd.Flags().Changed(name) { + inlineProvided = true + } + value, err := cmd.Flags().GetString(name) + if err != nil { + return "", false, stdinUsed, err + } + if inlineValue == "" && value != "" { + inlineValue = value + } + } + + if !cmd.Flags().Changed(fileFlag) { + if inlineValue == "" { + return "", false, stdinUsed, nil + } + return inlineValue, true, stdinUsed, nil + } + + if inlineProvided { + return "", false, stdinUsed, fmt.Errorf( + "cannot use %s with %s; choose one source", + formatLongFlag(fileFlag), + formatLongFlags(inlineFlags), + ) + } + + source, err := cmd.Flags().GetString(fileFlag) + if err != nil { + return "", false, stdinUsed, err + } + if source == "" { + return "", false, stdinUsed, fmt.Errorf("%s requires a path or - for stdin", formatLongFlag(fileFlag)) + } + + value, stdinUsed, err := input.ReadText(source, os.Stdin, stdinUsed) + if err != nil { + if errors.Is(err, input.ErrStdinAlreadyUsed) { + return "", false, stdinUsed, fmt.Errorf("%s cannot read from stdin more than once in a single command", formatLongFlag(fileFlag)) + } + if source == "-" { + return "", false, stdinUsed, fmt.Errorf("failed to read %s from stdin: %w", formatLongFlag(fileFlag), err) + } + return "", false, stdinUsed, fmt.Errorf("failed to read %s from %q: %w", formatLongFlag(fileFlag), source, err) + } + + return value, true, stdinUsed, nil +} + +func formatLongFlag(name string) string { + return "--" + name +} + +func formatLongFlags(names []string) string { + formatted := make([]string, 0, len(names)) + for _, name := range names { + formatted = append(formatted, formatLongFlag(name)) + } + return strings.Join(formatted, ", ") +} diff --git a/cmd/rich_text_test.go b/cmd/rich_text_test.go new file mode 100644 index 0000000..cd6a364 --- /dev/null +++ b/cmd/rich_text_test.go @@ -0,0 +1,67 @@ +package cmd + +import ( + "os" + "testing" + + "github.com/spf13/cobra" +) + +type commandFlagSnapshot struct { + value string + changed bool +} + +func saveAndRestoreCommandFlags(t *testing.T, cmd *cobra.Command, names ...string) { + t.Helper() + + flags := cmd.Flags() + snapshots := make(map[string]commandFlagSnapshot, len(names)) + for _, name := range names { + flag := flags.Lookup(name) + if flag == nil { + t.Fatalf("flag %q not found", name) + } + snapshots[name] = commandFlagSnapshot{ + value: flag.Value.String(), + changed: flag.Changed, + } + if err := flag.Value.Set(flag.DefValue); err != nil { + t.Fatalf("reset flag %q: %v", name, err) + } + flag.Changed = false + } + + t.Cleanup(func() { + for _, name := range names { + flag := flags.Lookup(name) + snapshot := snapshots[name] + if err := flag.Value.Set(snapshot.value); err != nil { + t.Fatalf("restore flag %q: %v", name, err) + } + flag.Changed = snapshot.changed + } + }) +} + +func replaceStdinWithFile(t *testing.T, content string) { + t.Helper() + + file, err := os.CreateTemp(t.TempDir(), "stdin-*") + if err != nil { + t.Fatalf("CreateTemp failed: %v", err) + } + if _, err := file.WriteString(content); err != nil { + t.Fatalf("WriteString failed: %v", err) + } + if _, err := file.Seek(0, 0); err != nil { + t.Fatalf("Seek failed: %v", err) + } + + oldStdin := os.Stdin + os.Stdin = file + t.Cleanup(func() { + os.Stdin = oldStdin + file.Close() + }) +} diff --git a/cmd/show.go b/cmd/show.go index 0516276..35e2ba8 100644 --- a/cmd/show.go +++ b/cmd/show.go @@ -55,11 +55,26 @@ Examples: fmt.Printf("\nUsage: td show \n") return fmt.Errorf("issue ID required") } else { - output.Error("no issue ID specified and no issues in progress") - fmt.Printf("\nUsage: td show \n") - fmt.Printf("Try: td list # see all issues\n") - fmt.Printf(" td next # see highest priority open issue\n") - return fmt.Errorf("issue ID required") + inReview, _ := database.ListIssues(db.ListIssuesOptions{ + Status: []models.Status{models.StatusInReview}, + Limit: 5, + }) + if len(inReview) == 1 { + args = []string{inReview[0].ID} + } else if len(inReview) > 1 { + output.Error("no issue ID specified. Multiple issues in review:") + for _, issue := range inReview { + fmt.Printf(" %s: %s\n", issue.ID, issue.Title) + } + fmt.Printf("\nUsage: td show \n") + return fmt.Errorf("issue ID required") + } else { + output.Error("no issue ID specified and no issues in progress") + fmt.Printf("\nUsage: td show \n") + fmt.Printf("Try: td list # see all issues\n") + fmt.Printf(" td next # see highest priority open issue\n") + return fmt.Errorf("issue ID required") + } } } } diff --git a/cmd/show_test.go b/cmd/show_test.go index b09b8a3..20ac04a 100644 --- a/cmd/show_test.go +++ b/cmd/show_test.go @@ -1,7 +1,14 @@ package cmd import ( + "bytes" + "io" + "os" + "strings" "testing" + + "github.com/marcus/td/internal/db" + "github.com/marcus/td/internal/models" ) // TestShowFormatFlagParsing tests that --format flag is defined and works @@ -85,3 +92,51 @@ func TestShowRenderMarkdownFlagExists(t *testing.T) { t.Error("Expected -m shorthand to be defined for --render-markdown") } } + +func TestShowNoArgsUsesSingleInReviewIssue(t *testing.T) { + saveAndRestoreGlobals(t) + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Reviewable single issue", + Status: models.StatusInReview, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + var output bytes.Buffer + oldStdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + os.Stdout = w + + runErr := showCmd.RunE(showCmd, []string{}) + + w.Close() + os.Stdout = oldStdout + _, _ = io.Copy(&output, r) + + if runErr != nil { + t.Fatalf("showCmd.RunE returned error: %v", runErr) + } + + got := output.String() + if !strings.Contains(got, issue.ID) { + t.Fatalf("expected output to contain issue ID %q, got %s", issue.ID, got) + } + if !strings.Contains(got, issue.Title) { + t.Fatalf("expected output to contain issue title %q, got %s", issue.Title, got) + } +} diff --git a/cmd/task.go b/cmd/task.go index 5cc3b75..0b658c7 100644 --- a/cmd/task.go +++ b/cmd/task.go @@ -103,11 +103,11 @@ func init() { taskCreateCmd.Flags().String("title", "", "Issue title (max 200 characters)") taskCreateCmd.Flags().StringP("priority", "p", "", "Priority (P0, P1, P2, P3, P4)") taskCreateCmd.Flags().StringP("description", "d", "", "Description text") - taskCreateCmd.Flags().String("labels", "", "Comma-separated labels") + taskCreateCmd.Flags().StringArrayP("labels", "l", nil, "Labels (repeatable, comma-separated)") taskCreateCmd.Flags().String("parent", "", "Parent issue ID") taskCreateCmd.Flags().String("epic", "", "Parent issue ID (alias for --parent)") - taskCreateCmd.Flags().String("depends-on", "", "Issues this depends on") - taskCreateCmd.Flags().String("blocks", "", "Issues this blocks") + taskCreateCmd.Flags().StringArray("depends-on", nil, "Issues this depends on (repeatable, comma-separated)") + taskCreateCmd.Flags().StringArray("blocks", nil, "Issues this blocks (repeatable, comma-separated)") taskCreateCmd.Flags().Bool("minor", false, "Mark as minor task (allows self-review)") // Hidden type flag - set programmatically to "task" taskCreateCmd.Flags().StringP("type", "t", "", "") diff --git a/cmd/update.go b/cmd/update.go index ec843b7..c6f7b8f 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -2,7 +2,6 @@ package cmd import ( "fmt" - "strings" "github.com/marcus/td/internal/dateparse" "github.com/marcus/td/internal/db" @@ -17,6 +16,9 @@ var updateCmd = &cobra.Command{ Use: "update [issue-id...]", Aliases: []string{"edit"}, Short: "Update one or more fields on existing issues", + Example: " td update td-a1b2 --priority P1 --description \"Short inline note\"\n" + + " td update td-a1b2 --description-file description.md\n" + + " cat acceptance.md | td update td-a1b2 --append --acceptance-file -", GroupID: "core", Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -50,24 +52,38 @@ var updateCmd = &cobra.Command{ } appendMode, _ := cmd.Flags().GetBool("append") + stdinUsed := false - // Check description and its aliases (--desc, --body, -d) - desc, _ := cmd.Flags().GetString("description") - if desc == "" { - desc, _ = cmd.Flags().GetString("desc") - } - if desc == "" { - desc, _ = cmd.Flags().GetString("body") + description, descriptionProvided, nextStdinUsed, err := resolveRichTextField( + cmd, + []string{"description", "desc", "body"}, + "description-file", + stdinUsed, + ) + if err != nil { + output.Error("%v", err) + return err } - if desc != "" { + if descriptionProvided { if appendMode && issue.Description != "" { - issue.Description = issue.Description + "\n\n" + desc + issue.Description = issue.Description + "\n\n" + description } else { - issue.Description = desc + issue.Description = description } } + stdinUsed = nextStdinUsed - if acceptance, _ := cmd.Flags().GetString("acceptance"); acceptance != "" { + acceptance, acceptanceProvided, _, err := resolveRichTextField( + cmd, + []string{"acceptance"}, + "acceptance-file", + stdinUsed, + ) + if err != nil { + output.Error("%v", err) + return err + } + if acceptanceProvided { if appendMode && issue.Acceptance != "" { issue.Acceptance = issue.Acceptance + "\n\n" + acceptance } else { @@ -99,14 +115,13 @@ var updateCmd = &cobra.Command{ issue.Points = pts } - if labels, _ := cmd.Flags().GetString("labels"); cmd.Flags().Changed("labels") { - if labels == "" { + if cmd.Flags().Changed("labels") { + labelsArr, _ := cmd.Flags().GetStringArray("labels") + merged := mergeMultiValueFlag(labelsArr) + if len(merged) == 0 { issue.Labels = nil } else { - issue.Labels = strings.Split(labels, ",") - for i := range issue.Labels { - issue.Labels[i] = strings.TrimSpace(issue.Labels[i]) - } + issue.Labels = merged } } @@ -183,34 +198,26 @@ var updateCmd = &cobra.Command{ } } - // Update dependencies - if dependsOn, _ := cmd.Flags().GetString("depends-on"); cmd.Flags().Changed("depends-on") { - // Clear existing and set new + // Update dependencies (support repeated flags and comma-separated) + if cmd.Flags().Changed("depends-on") { existingDeps, _ := database.GetDependencies(issueID) for _, dep := range existingDeps { database.RemoveDependencyLogged(issueID, dep, sess.ID) } - // Add new deps - if dependsOn != "" { - for _, dep := range strings.Split(dependsOn, ",") { - dep = strings.TrimSpace(dep) - database.AddDependencyLogged(issueID, dep, "depends_on", sess.ID) - } + dependsArr, _ := cmd.Flags().GetStringArray("depends-on") + for _, dep := range mergeMultiValueFlag(dependsArr) { + database.AddDependencyLogged(issueID, dep, "depends_on", sess.ID) } } - if blocks, _ := cmd.Flags().GetString("blocks"); cmd.Flags().Changed("blocks") { - // For blocks, we need to find issues that depend on this one and update them + if cmd.Flags().Changed("blocks") { blocked, _ := database.GetBlockedBy(issueID) for _, b := range blocked { database.RemoveDependencyLogged(b, issueID, sess.ID) } - // Add new blocks - if blocks != "" { - for _, b := range strings.Split(blocks, ",") { - b = strings.TrimSpace(b) - database.AddDependencyLogged(b, issueID, "depends_on", sess.ID) - } + blocksArr, _ := cmd.Flags().GetStringArray("blocks") + for _, b := range mergeMultiValueFlag(blocksArr) { + database.AddDependencyLogged(b, issueID, "depends_on", sess.ID) } } @@ -249,15 +256,17 @@ func init() { updateCmd.Flags().StringP("description", "d", "", "New description") updateCmd.Flags().String("desc", "", "Alias for --description") updateCmd.Flags().String("body", "", "Alias for --description") + updateCmd.Flags().String("description-file", "", "Read description from file or - for stdin (preserves formatting)") updateCmd.Flags().String("acceptance", "", "New acceptance criteria") + updateCmd.Flags().String("acceptance-file", "", "Read acceptance criteria from file or - for stdin (preserves formatting)") updateCmd.Flags().String("type", "", "New type") updateCmd.Flags().String("priority", "", "New priority") updateCmd.Flags().Int("points", 0, "New story points") - updateCmd.Flags().String("labels", "", "Replace labels") + updateCmd.Flags().StringArrayP("labels", "l", nil, "Replace labels (repeatable, comma-separated)") updateCmd.Flags().String("sprint", "", "New sprint name (empty string to clear)") updateCmd.Flags().String("parent", "", "New parent issue ID") - updateCmd.Flags().String("depends-on", "", "Replace dependencies") - updateCmd.Flags().String("blocks", "", "Replace blocked issues") + updateCmd.Flags().StringArray("depends-on", nil, "Replace dependencies (repeatable, comma-separated)") + updateCmd.Flags().StringArray("blocks", nil, "Replace blocked issues (repeatable, comma-separated)") updateCmd.Flags().Bool("append", false, "Append to text fields instead of replacing") updateCmd.Flags().String("status", "", "New status (open, in_progress, in_review, blocked, closed)") updateCmd.Flags().StringP("comment", "m", "", "Add a comment to the updated issue(s)") diff --git a/cmd/update_test.go b/cmd/update_test.go index a2644a7..c185fbe 100644 --- a/cmd/update_test.go +++ b/cmd/update_test.go @@ -1,6 +1,9 @@ package cmd import ( + "os" + "path/filepath" + "strings" "testing" "github.com/marcus/td/internal/db" @@ -561,3 +564,115 @@ func TestUpdateCmdHasStatusFlag(t *testing.T) { // Reset updateCmd.Flags().Set("status", "") } + +func TestUpdateRichTextAppendFromFileAndStdin(t *testing.T) { + saveAndRestoreGlobals(t) + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Rich text update target", + Description: "Existing description", + Acceptance: "Existing acceptance", + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + saveAndRestoreCommandFlags(t, updateCmd, "description", "desc", "body", "description-file", "acceptance", "acceptance-file", "append") + + description := "## Imported\n\n```sh\necho hello\n```\n" + descriptionPath := filepath.Join(dir, "description.md") + if err := os.WriteFile(descriptionPath, []byte(description), 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + + acceptance := "- [ ] keep spacing\n\n- [ ] preserve quotes \"ok\"\n" + replaceStdinWithFile(t, acceptance) + + if err := updateCmd.Flags().Set("description-file", descriptionPath); err != nil { + t.Fatalf("Set description-file failed: %v", err) + } + if err := updateCmd.Flags().Set("acceptance-file", "-"); err != nil { + t.Fatalf("Set acceptance-file failed: %v", err) + } + if err := updateCmd.Flags().Set("append", "true"); err != nil { + t.Fatalf("Set append failed: %v", err) + } + + if err := updateCmd.RunE(updateCmd, []string{issue.ID}); err != nil { + t.Fatalf("updateCmd.RunE failed: %v", err) + } + + updated, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + + wantDescription := "Existing description\n\n" + description + if updated.Description != wantDescription { + t.Fatalf("description mismatch\nwant: %q\ngot: %q", wantDescription, updated.Description) + } + + wantAcceptance := "Existing acceptance\n\n" + acceptance + if updated.Acceptance != wantAcceptance { + t.Fatalf("acceptance mismatch\nwant: %q\ngot: %q", wantAcceptance, updated.Acceptance) + } +} + +func TestUpdateRichTextConflictErrors(t *testing.T) { + saveAndRestoreGlobals(t) + + dir := t.TempDir() + baseDir := dir + baseDirOverride = &baseDir + + database, err := db.Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{Title: "Conflict target", Description: "old"} + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + saveAndRestoreCommandFlags(t, updateCmd, "description", "desc", "body", "description-file") + + descriptionPath := filepath.Join(dir, "description.md") + if err := os.WriteFile(descriptionPath, []byte("from file"), 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + + if err := updateCmd.Flags().Set("description", "inline"); err != nil { + t.Fatalf("Set description failed: %v", err) + } + if err := updateCmd.Flags().Set("description-file", descriptionPath); err != nil { + t.Fatalf("Set description-file failed: %v", err) + } + + err = updateCmd.RunE(updateCmd, []string{issue.ID}) + if err == nil { + t.Fatal("expected updateCmd.RunE to fail") + } + if !strings.Contains(err.Error(), "--description-file") || !strings.Contains(err.Error(), "--description") { + t.Fatalf("expected actionable conflict error, got %v", err) + } + + updated, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if updated.Description != "old" { + t.Fatalf("expected description to remain unchanged, got %q", updated.Description) + } +} diff --git a/internal/db/issues_logged.go b/internal/db/issues_logged.go index e7705e7..cc10c4c 100644 --- a/internal/db/issues_logged.go +++ b/internal/db/issues_logged.go @@ -10,6 +10,18 @@ import ( "github.com/marcus/td/internal/models" ) +// StaleIssueStatusError indicates the issue status changed after the caller +// loaded the issue but before the logged transition was persisted. +type StaleIssueStatusError struct { + IssueID string + Expected models.Status + Actual models.Status +} + +func (e *StaleIssueStatusError) Error() string { + return fmt.Sprintf("issue %s status changed from %s to %s", e.IssueID, e.Expected, e.Actual) +} + // marshalIssue returns a JSON representation of an issue for action_log storage. func marshalIssue(issue *models.Issue) string { data, _ := json.Marshal(issue) @@ -151,6 +163,10 @@ func (db *DB) updateIssueAndLog(issue *models.Issue, sessionID string, actionTyp if err != nil { return err } + return db.updateIssueAndLogFromPrevious(issue, prev, sessionID, actionType) +} + +func (db *DB) updateIssueAndLogFromPrevious(issue, prev *models.Issue, sessionID string, actionType models.ActionType) error { previousData := marshalIssue(prev) // Apply update @@ -166,7 +182,7 @@ func (db *DB) updateIssueAndLog(issue *models.Issue, sessionID string, actionTyp dueDate = sql.NullString{String: *issue.DueDate, Valid: true} } - _, err = db.conn.Exec(` + _, err := db.conn.Exec(` UPDATE issues SET title = ?, description = ?, status = ?, type = ?, priority = ?, points = ?, labels = ?, parent_id = ?, acceptance = ?, sprint = ?, implementer_session = ?, reviewer_session = ?, updated_at = ?, @@ -223,6 +239,25 @@ func (db *DB) UpdateIssueLogged(issue *models.Issue, sessionID string, actionTyp }) } +// UpdateIssueLoggedIfStatus updates an issue and logs the action atomically, +// but only if the current persisted status still matches expectedStatus. +func (db *DB) UpdateIssueLoggedIfStatus(issue *models.Issue, expectedStatus models.Status, sessionID string, actionType models.ActionType) error { + return db.withWriteLock(func() error { + prev, err := db.scanIssueRow(issue.ID) + if err != nil { + return err + } + if prev.Status != expectedStatus { + return &StaleIssueStatusError{ + IssueID: issue.ID, + Expected: expectedStatus, + Actual: prev.Status, + } + } + return db.updateIssueAndLogFromPrevious(issue, prev, sessionID, actionType) + }) +} + // DeleteIssueLogged soft-deletes an issue and logs the action atomically within a single withWriteLock call. func (db *DB) DeleteIssueLogged(issueID, sessionID string) error { return db.withWriteLock(func() error { diff --git a/internal/db/issues_logged_test.go b/internal/db/issues_logged_test.go index 9850d96..cacf8af 100644 --- a/internal/db/issues_logged_test.go +++ b/internal/db/issues_logged_test.go @@ -2,6 +2,7 @@ package db import ( "encoding/json" + "errors" "testing" "github.com/marcus/td/internal/models" @@ -152,6 +153,73 @@ func TestUpdateIssueLogged(t *testing.T) { } } +func TestUpdateIssueLoggedIfStatusDetectsStaleTransition(t *testing.T) { + dir := t.TempDir() + database, err := Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer database.Close() + + issue := &models.Issue{ + Title: "Concurrent review target", + Status: models.StatusOpen, + Type: models.TypeTask, + Priority: models.PriorityP2, + } + if err := database.CreateIssue(issue); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + staleCopy, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + + current, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue current failed: %v", err) + } + current.Status = models.StatusInProgress + if err := database.UpdateIssue(current); err != nil { + t.Fatalf("UpdateIssue failed: %v", err) + } + + staleCopy.Status = models.StatusInReview + err = database.UpdateIssueLoggedIfStatus(staleCopy, models.StatusOpen, "sess-reviewer", models.ActionReview) + if err == nil { + t.Fatal("expected stale status error") + } + + var staleErr *StaleIssueStatusError + if !errors.As(err, &staleErr) { + t.Fatalf("expected StaleIssueStatusError, got %T", err) + } + if staleErr.Expected != models.StatusOpen { + t.Fatalf("expected stale error expected=%s, got %s", models.StatusOpen, staleErr.Expected) + } + if staleErr.Actual != models.StatusInProgress { + t.Fatalf("expected stale error actual=%s, got %s", models.StatusInProgress, staleErr.Actual) + } + t.Log(staleErr.Error()) + + got, err := database.GetIssue(issue.ID) + if err != nil { + t.Fatalf("GetIssue after stale update failed: %v", err) + } + if got.Status != models.StatusInProgress { + t.Fatalf("status = %s, want %s", got.Status, models.StatusInProgress) + } + + var actions int + if err := database.conn.QueryRow(`SELECT COUNT(*) FROM action_log WHERE entity_id = ?`, issue.ID).Scan(&actions); err != nil { + t.Fatalf("count action_log failed: %v", err) + } + if actions != 0 { + t.Fatalf("expected no action log entry for stale transition, got %d", actions) + } +} + func TestDeleteIssueLogged(t *testing.T) { dir := t.TempDir() database, err := Initialize(dir) diff --git a/internal/input/input.go b/internal/input/input.go index 9c6eb53..83070b9 100644 --- a/internal/input/input.go +++ b/internal/input/input.go @@ -1,9 +1,10 @@ -// Package input provides helpers for reading flag values from stdin and files -// (@file syntax). +// Package input provides helpers for reading flag values from stdin and files. package input import ( "bufio" + "errors" + "fmt" "io" "os" "strings" @@ -11,6 +12,9 @@ import ( "github.com/marcus/td/internal/output" ) +// ErrStdinAlreadyUsed is returned when multiple flags try to consume stdin. +var ErrStdinAlreadyUsed = errors.New("stdin already used") + // ExpandFlagValues expands flag values that use - (stdin) or @file syntax. // Returns the expanded values and whether stdin was consumed. func ExpandFlagValues(values []string, stdinUsed bool) ([]string, bool) { @@ -53,3 +57,28 @@ func ReadLinesFromReader(r io.Reader) []string { } return lines } + +// ReadText preserves the full contents of a file or stdin as a single block. +// The special source "-" reads from stdin exactly once. +func ReadText(source string, stdin io.Reader, stdinUsed bool) (string, bool, error) { + if source == "" { + return "", stdinUsed, fmt.Errorf("input source is required") + } + + if source == "-" { + if stdinUsed { + return "", stdinUsed, ErrStdinAlreadyUsed + } + data, err := io.ReadAll(stdin) + if err != nil { + return "", true, fmt.Errorf("read stdin: %w", err) + } + return string(data), true, nil + } + + data, err := os.ReadFile(source) + if err != nil { + return "", stdinUsed, fmt.Errorf("read %s: %w", source, err) + } + return string(data), stdinUsed, nil +} diff --git a/internal/input/input_test.go b/internal/input/input_test.go index 59af0de..f90b79c 100644 --- a/internal/input/input_test.go +++ b/internal/input/input_test.go @@ -1,6 +1,7 @@ package input import ( + "errors" "os" "path/filepath" "strings" @@ -264,3 +265,47 @@ func TestExpandFlagValuesNilSlice(t *testing.T) { t.Errorf("Expected nil or empty result, got %v", result) } } + +func TestReadTextFromFilePreservesExactContent(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "body.md") + want := "Intro\n\n```go\nfmt.Println(\"hi\")\n```\n indented\n" + if err := os.WriteFile(filePath, []byte(want), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + got, stdinUsed, err := ReadText(filePath, strings.NewReader("unused"), false) + if err != nil { + t.Fatalf("ReadText failed: %v", err) + } + if stdinUsed { + t.Fatal("stdinUsed should be false for file input") + } + if got != want { + t.Fatalf("content mismatch\nwant: %q\ngot: %q", want, got) + } +} + +func TestReadTextFromStdinPreservesExactContent(t *testing.T) { + want := "# Title\n\n- item\n\n> quote\n" + got, stdinUsed, err := ReadText("-", strings.NewReader(want), false) + if err != nil { + t.Fatalf("ReadText failed: %v", err) + } + if !stdinUsed { + t.Fatal("stdinUsed should be true for stdin input") + } + if got != want { + t.Fatalf("content mismatch\nwant: %q\ngot: %q", want, got) + } +} + +func TestReadTextRejectsSecondStdinRead(t *testing.T) { + _, stdinUsed, err := ReadText("-", strings.NewReader("ignored"), true) + if !stdinUsed { + t.Fatal("stdinUsed should remain true") + } + if !errors.Is(err, ErrStdinAlreadyUsed) { + t.Fatalf("expected ErrStdinAlreadyUsed, got %v", err) + } +} diff --git a/proof/td-2b41b2/PROOF.md b/proof/td-2b41b2/PROOF.md new file mode 100644 index 0000000..f6e6108 --- /dev/null +++ b/proof/td-2b41b2/PROOF.md @@ -0,0 +1,13 @@ +# td-2b41b2 Proof + +Verified with captured CLI output in this run: + +- `init.txt`, `create-epic.txt`, `create-task.txt` +- `ws-start.txt`, `ws-tag.txt`, `list-epic-dot.txt` +- `review.txt`, `show-no-args.txt`, `approve-no-args.txt`, `show-approved.txt` + +Notes: + +- The repository does not include a browser screenshot harness for this CLI-only task, so the proof artifact here is command output instead of screenshots. +- `list-epic-dot.txt` shows `td list --epic .` resolving to the active epic context. +- `show-no-args.txt` and `approve-no-args.txt` demonstrate the no-args review-flow behavior. diff --git a/proof/td-2b41b2/approve-no-args.txt b/proof/td-2b41b2/approve-no-args.txt new file mode 100644 index 0000000..1a03eb0 --- /dev/null +++ b/proof/td-2b41b2/approve-no-args.txt @@ -0,0 +1 @@ +APPROVED td-a0da0b (reviewer: ses_db4242) diff --git a/proof/td-2b41b2/create-epic.txt b/proof/td-2b41b2/create-epic.txt new file mode 100644 index 0000000..815e0f4 --- /dev/null +++ b/proof/td-2b41b2/create-epic.txt @@ -0,0 +1 @@ +CREATED td-33e909 diff --git a/proof/td-2b41b2/create-task.txt b/proof/td-2b41b2/create-task.txt new file mode 100644 index 0000000..26740db --- /dev/null +++ b/proof/td-2b41b2/create-task.txt @@ -0,0 +1 @@ +CREATED td-7f3c03 diff --git a/proof/td-2b41b2/init.txt b/proof/td-2b41b2/init.txt new file mode 100644 index 0000000..ee8a8f3 --- /dev/null +++ b/proof/td-2b41b2/init.txt @@ -0,0 +1,14 @@ +INITIALIZED /private/tmp/td-proof-YA1mwd/.todos +Session: ses_fd015e + +Tip: Add this to your CLAUDE.md, AGENTS.md, or similar agent file: + +## MANDATORY: Use td for Task Management + +Run td usage --new-session at conversation start (or after /clear). This tells you what to work on next. + +Sessions are automatic (based on terminal/agent context). Optional: +- td session "name" to label the current session +- td session --new to force a new session in the same context + +Use td usage -q after first read. diff --git a/proof/td-2b41b2/list-epic-dot.txt b/proof/td-2b41b2/list-epic-dot.txt new file mode 100644 index 0000000..e909653 --- /dev/null +++ b/proof/td-2b41b2/list-epic-dot.txt @@ -0,0 +1 @@ +td-7f3c03 [P2] Dispatch loop proof task task [in_progress] diff --git a/proof/td-2b41b2/review.txt b/proof/td-2b41b2/review.txt new file mode 100644 index 0000000..b3e3c11 --- /dev/null +++ b/proof/td-2b41b2/review.txt @@ -0,0 +1,3 @@ +Warning: auto-created minimal handoff for td-7f3c03 - consider using 'td handoff' for better documentation +REVIEW REQUESTED td-7f3c03 (session: ses_fd015e) + ↑ Parent td-33e909 auto-cascaded to in_review diff --git a/proof/td-2b41b2/show-approved.txt b/proof/td-2b41b2/show-approved.txt new file mode 100644 index 0000000..adc3949 --- /dev/null +++ b/proof/td-2b41b2/show-approved.txt @@ -0,0 +1,15 @@ +td-a0da0b: Standalone review proof task +Status: [closed] +Type: task | Priority: P2 + +CURRENT HANDOFF (ses_bcb8db, just now): + Done: + - Auto-generated for review submission + +SESSION LOG: + [18:23] Submitted for review + [18:23] Approved + +SESSIONS INVOLVED: + ses_bcb8db (implementer) + ses_db4242 (reviewer) diff --git a/proof/td-2b41b2/show-no-args.txt b/proof/td-2b41b2/show-no-args.txt new file mode 100644 index 0000000..06ac3c7 --- /dev/null +++ b/proof/td-2b41b2/show-no-args.txt @@ -0,0 +1,15 @@ +td-a0da0b: Standalone review proof task +Status: [in_review] +Type: task | Priority: P2 + +CURRENT HANDOFF (ses_bcb8db, just now): + Done: + - Auto-generated for review submission + +SESSION LOG: + [18:23] Submitted for review + +AWAITING REVIEW - requires different session to approve/reject + +SESSIONS INVOLVED: + ses_bcb8db (implementer) diff --git a/proof/td-2b41b2/ws-start.txt b/proof/td-2b41b2/ws-start.txt new file mode 100644 index 0000000..2f599ec --- /dev/null +++ b/proof/td-2b41b2/ws-start.txt @@ -0,0 +1,5 @@ +WORK SESSION STARTED: ws-f87b +Name: proof-loop + +Tag issues with: td ws tag +Log progress with: td ws log "message" diff --git a/proof/td-2b41b2/ws-tag.txt b/proof/td-2b41b2/ws-tag.txt new file mode 100644 index 0000000..53adfd2 --- /dev/null +++ b/proof/td-2b41b2/ws-tag.txt @@ -0,0 +1,2 @@ +TAGGED td-7f3c03 → ws-f87b +STARTED td-7f3c03 (session: ses_fd015e) diff --git a/proof/td-377aae/PROOF.md b/proof/td-377aae/PROOF.md new file mode 100644 index 0000000..c965e6f --- /dev/null +++ b/proof/td-377aae/PROOF.md @@ -0,0 +1,10 @@ +# td-377aae Proof + +Artifacts in this directory are generated by `run-proof.sh`. + +What they cover: + +- A normal `td review` + `td approve` flow still works end to end. +- The new DB transition guard rejects stale status writes when another session has already moved the issue. +- The review command family now explains stale concurrent transitions with the current status and a concrete next command. +- Browser screenshots are rendered from `proof.html` after the artifacts are generated. diff --git a/proof/td-377aae/approve-happy.txt b/proof/td-377aae/approve-happy.txt new file mode 100644 index 0000000..3ed32f3 --- /dev/null +++ b/proof/td-377aae/approve-happy.txt @@ -0,0 +1 @@ +APPROVED td-c5932a (reviewer: ses_543a53) diff --git a/proof/td-377aae/cmd-stale-test.txt b/proof/td-377aae/cmd-stale-test.txt new file mode 100644 index 0000000..9b6afd4 --- /dev/null +++ b/proof/td-377aae/cmd-stale-test.txt @@ -0,0 +1,6 @@ +=== RUN TestDescribeStaleTransitionUpdate + review_policy_test.go:408: cannot approve td-764f76: status changed from in_review to closed in another session + Already closed: td show td-764f76 +--- PASS: TestDescribeStaleTransitionUpdate (0.04s) +PASS +ok github.com/marcus/td/cmd (cached) diff --git a/proof/td-377aae/create-happy.txt b/proof/td-377aae/create-happy.txt new file mode 100644 index 0000000..c064bd4 --- /dev/null +++ b/proof/td-377aae/create-happy.txt @@ -0,0 +1 @@ +CREATED td-c5932a diff --git a/proof/td-377aae/db-stale-test.txt b/proof/td-377aae/db-stale-test.txt new file mode 100644 index 0000000..9d1223f --- /dev/null +++ b/proof/td-377aae/db-stale-test.txt @@ -0,0 +1,5 @@ +=== RUN TestUpdateIssueLoggedIfStatusDetectsStaleTransition + issues_logged_test.go:204: issue td-4367a5 status changed from open to in_progress +--- PASS: TestUpdateIssueLoggedIfStatusDetectsStaleTransition (0.06s) +PASS +ok github.com/marcus/td/internal/db (cached) diff --git a/proof/td-377aae/guidance-tests.txt b/proof/td-377aae/guidance-tests.txt new file mode 100644 index 0000000..ac2cdc2 --- /dev/null +++ b/proof/td-377aae/guidance-tests.txt @@ -0,0 +1,36 @@ +=== RUN TestCloseFollowupGuidance +=== RUN TestCloseFollowupGuidance/open_issue_points_to_review +=== RUN TestCloseFollowupGuidance/in_progress_issue_points_to_review +=== RUN TestCloseFollowupGuidance/in_review_issue_points_to_approve +=== RUN TestCloseFollowupGuidance/closed_issue_points_to_show +=== RUN TestCloseFollowupGuidance/nil_issue_falls_back_to_review_wording +--- PASS: TestCloseFollowupGuidance (0.00s) + --- PASS: TestCloseFollowupGuidance/open_issue_points_to_review (0.00s) + --- PASS: TestCloseFollowupGuidance/in_progress_issue_points_to_review (0.00s) + --- PASS: TestCloseFollowupGuidance/in_review_issue_points_to_approve (0.00s) + --- PASS: TestCloseFollowupGuidance/closed_issue_points_to_show (0.00s) + --- PASS: TestCloseFollowupGuidance/nil_issue_falls_back_to_review_wording (0.00s) +=== RUN TestReviewFollowupGuidance +=== RUN TestReviewFollowupGuidance/open_issue_points_to_review +=== RUN TestReviewFollowupGuidance/in_progress_issue_points_to_review +=== RUN TestReviewFollowupGuidance/in_review_issue_points_to_approve +=== RUN TestReviewFollowupGuidance/closed_issue_points_to_show +=== RUN TestReviewFollowupGuidance/nil_issue_falls_back_to_review_wording +--- PASS: TestReviewFollowupGuidance (0.00s) + --- PASS: TestReviewFollowupGuidance/open_issue_points_to_review (0.00s) + --- PASS: TestReviewFollowupGuidance/in_progress_issue_points_to_review (0.00s) + --- PASS: TestReviewFollowupGuidance/in_review_issue_points_to_approve (0.00s) + --- PASS: TestReviewFollowupGuidance/closed_issue_points_to_show (0.00s) + --- PASS: TestReviewFollowupGuidance/nil_issue_falls_back_to_review_wording (0.00s) +=== RUN TestApproveFollowupGuidance +=== RUN TestApproveFollowupGuidance/in_review_issue_points_to_approve +=== RUN TestApproveFollowupGuidance/open_issue_points_to_review_first +=== RUN TestApproveFollowupGuidance/closed_issue_points_to_show +=== RUN TestApproveFollowupGuidance/nil_issue_falls_back_to_review_wording +--- PASS: TestApproveFollowupGuidance (0.00s) + --- PASS: TestApproveFollowupGuidance/in_review_issue_points_to_approve (0.00s) + --- PASS: TestApproveFollowupGuidance/open_issue_points_to_review_first (0.00s) + --- PASS: TestApproveFollowupGuidance/closed_issue_points_to_show (0.00s) + --- PASS: TestApproveFollowupGuidance/nil_issue_falls_back_to_review_wording (0.00s) +PASS +ok github.com/marcus/td/cmd (cached) diff --git a/proof/td-377aae/handoff-happy.txt b/proof/td-377aae/handoff-happy.txt new file mode 100644 index 0000000..09f822e --- /dev/null +++ b/proof/td-377aae/handoff-happy.txt @@ -0,0 +1,4 @@ +HANDOFF RECORDED td-c5932a +Git: 53f499a (dispatch/td-377aae-0004) +0 commits since start + +Next: `td review td-c5932a` to submit for review diff --git a/proof/td-377aae/init.txt b/proof/td-377aae/init.txt new file mode 100644 index 0000000..555e0fe --- /dev/null +++ b/proof/td-377aae/init.txt @@ -0,0 +1,15 @@ +INITIALIZED /var/folders/9z/_hxsyhcx59d_cbrbhxfk9j000000gn/T/td-377aae-proof.2udVZo/.todos +Added .todos/ to .gitignore +Session: ses_3bce40 + +Tip: Add this to your CLAUDE.md, AGENTS.md, or similar agent file: + +## MANDATORY: Use td for Task Management + +Run td usage --new-session at conversation start (or after /clear). This tells you what to work on next. + +Sessions are automatic (based on terminal/agent context). Optional: +- td session "name" to label the current session +- td session --new to force a new session in the same context + +Use td usage -q after first read. diff --git a/proof/td-377aae/proof-full.png b/proof/td-377aae/proof-full.png new file mode 100644 index 0000000..48dd2f5 Binary files /dev/null and b/proof/td-377aae/proof-full.png differ diff --git a/proof/td-377aae/proof-normal.png b/proof/td-377aae/proof-normal.png new file mode 100644 index 0000000..1549a42 Binary files /dev/null and b/proof/td-377aae/proof-normal.png differ diff --git a/proof/td-377aae/proof-stale.png b/proof/td-377aae/proof-stale.png new file mode 100644 index 0000000..733e4f3 Binary files /dev/null and b/proof/td-377aae/proof-stale.png differ diff --git a/proof/td-377aae/proof.html b/proof/td-377aae/proof.html new file mode 100644 index 0000000..fe152e9 --- /dev/null +++ b/proof/td-377aae/proof.html @@ -0,0 +1,208 @@ + + + + +td-377aae proof + + + +

td-377aae proof

+

This proof combines two things: the happy-path CLI flow still works end to end for td-c5932a, and the new stale-transition protection is exercised by deterministic tests that capture the exact messages users now see when a concurrent status change makes their local transition stale.

+ +
+
Happy path: create → start → handoff → review → approve
+
DB guard: refuses stale status write and preserves the newer state
+
CLI wording: explains the concurrent status change and the next command
+
+ +
+
+
Happy Path
+

Review command still works normally

+ 
REVIEW REQUESTED td-c5932a (session: ses_3bce40)
+
+
+ +
+
Happy Path
+

Approve still closes the reviewed issue

+ 
APPROVED td-c5932a (reviewer: ses_543a53)
+
+
+
+ +
+
+
Stale Guard
+

DB rejects a stale concurrent transition

+ 
=== RUN   TestUpdateIssueLoggedIfStatusDetectsStaleTransition
+    issues_logged_test.go:204: issue td-4367a5 status changed from open to in_progress
+--- PASS: TestUpdateIssueLoggedIfStatusDetectsStaleTransition (0.06s)
+PASS
+ok  	github.com/marcus/td/internal/db	(cached)
+
+
+ +
+
User Message
+

CLI guidance names the new status and next step

+ 
=== RUN   TestDescribeStaleTransitionUpdate
+    review_policy_test.go:408: cannot approve td-764f76: status changed from in_review to closed in another session
+          Already closed: td show td-764f76
+--- PASS: TestDescribeStaleTransitionUpdate (0.04s)
+PASS
+ok  	github.com/marcus/td/cmd	(cached)
+
+
+
+ +
+
+
Follow-up Guidance
+

Closed-state guidance stays explicit

+ 
=== RUN   TestCloseFollowupGuidance
+=== RUN   TestCloseFollowupGuidance/open_issue_points_to_review
+=== RUN   TestCloseFollowupGuidance/in_progress_issue_points_to_review
+=== RUN   TestCloseFollowupGuidance/in_review_issue_points_to_approve
+=== RUN   TestCloseFollowupGuidance/closed_issue_points_to_show
+=== RUN   TestCloseFollowupGuidance/nil_issue_falls_back_to_review_wording
+--- PASS: TestCloseFollowupGuidance (0.00s)
+    --- PASS: TestCloseFollowupGuidance/open_issue_points_to_review (0.00s)
+    --- PASS: TestCloseFollowupGuidance/in_progress_issue_points_to_review (0.00s)
+    --- PASS: TestCloseFollowupGuidance/in_review_issue_points_to_approve (0.00s)
+    --- PASS: TestCloseFollowupGuidance/closed_issue_points_to_show (0.00s)
+    --- PASS: TestCloseFollowupGuidance/nil_issue_falls_back_to_review_wording (0.00s)
+=== RUN   TestReviewFollowupGuidance
+=== RUN   TestReviewFollowupGuidance/open_issue_points_to_review
+=== RUN   TestReviewFollowupGuidance/in_progress_issue_points_to_review
+=== RUN   TestReviewFollowupGuidance/in_review_issue_points_to_approve
+=== RUN   TestReviewFollowupGuidance/closed_issue_points_to_show
+=== RUN   TestReviewFollowupGuidance/nil_issue_falls_back_to_review_wording
+--- PASS: TestReviewFollowupGuidance (0.00s)
+    --- PASS: TestReviewFollowupGuidance/open_issue_points_to_review (0.00s)
+    --- PASS: TestReviewFollowupGuidance/in_progress_issue_points_to_review (0.00s)
+    --- PASS: TestReviewFollowupGuidance/in_review_issue_points_to_approve (0.00s)
+    --- PASS: TestReviewFollowupGuidance/closed_issue_points_to_show (0.00s)
+    --- PASS: TestReviewFollowupGuidance/nil_issue_falls_back_to_review_wording (0.00s)
+=== RUN   TestApproveFollowupGuidance
+=== RUN   TestApproveFollowupGuidance/in_review_issue_points_to_approve
+=== RUN   TestApproveFollowupGuidance/open_issue_points_to_review_first
+=== RUN   TestApproveFollowupGuidance/closed_issue_points_to_show
+=== RUN   TestApproveFollowupGuidance/nil_issue_falls_back_to_review_wording
+--- PASS: TestApproveFollowupGuidance (0.00s)
+    --- PASS: TestApproveFollowupGuidance/in_review_issue_points_to_approve (0.00s)
+    --- PASS: TestApproveFollowupGuidance/open_issue_points_to_review_first (0.00s)
+    --- PASS: TestApproveFollowupGuidance/closed_issue_points_to_show (0.00s)
+    --- PASS: TestApproveFollowupGuidance/nil_issue_falls_back_to_review_wording (0.00s)
+PASS
+ok  	github.com/marcus/td/cmd	(cached)
+
+
+ +
+
Final State
+

Happy-path issue ends closed with history intact

+ 
td-c5932a: Proof stale transition workflow
+Status: [closed]
+Type: task | Priority: P2
+
+CURRENT HANDOFF (ses_3bce40, just now):
+  Done:
+    - Captured reviewer handoff context
+
+SESSION LOG:
+  [18:02] Started work
+  [18:02] Submitted for review
+  [18:02] Approved
+
+GIT STATE:
+  Started: 53f499a (dispatch/td-377aae-0004) just now
+  Current: 53f499a (dispatch/td-377aae-0004) +0 commits
+
+SESSIONS INVOLVED:
+  ses_3bce40 (implementer)
+  ses_543a53 (reviewer)
+
+
+
+ + diff --git a/proof/td-377aae/review-happy.txt b/proof/td-377aae/review-happy.txt new file mode 100644 index 0000000..d7b23d7 --- /dev/null +++ b/proof/td-377aae/review-happy.txt @@ -0,0 +1 @@ +REVIEW REQUESTED td-c5932a (session: ses_3bce40) diff --git a/proof/td-377aae/run-proof.sh b/proof/td-377aae/run-proof.sh new file mode 100755 index 0000000..34c13b4 --- /dev/null +++ b/proof/td-377aae/run-proof.sh @@ -0,0 +1,212 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +OUT="$ROOT/proof/td-377aae" +BIN="$ROOT/td" +TMP="$(mktemp -d "${TMPDIR:-/tmp}/td-377aae-proof.XXXXXX")" + +cleanup() { + rm -rf "$TMP" +} +trap cleanup EXIT + +mkdir -p "$OUT" +rm -f "$OUT"/*.txt "$OUT"/*.png "$OUT"/proof.html + +capture() { + local name="$1" + shift + "$@" >"$OUT/$name" 2>&1 +} + +capture init.txt env TD_SESSION_ID=ses_impl "$BIN" -w "$TMP" init +capture create-happy.txt env TD_SESSION_ID=ses_impl "$BIN" -w "$TMP" create "Proof stale transition workflow" +ISSUE_ID="$(sed -n 's/^CREATED //p' "$OUT/create-happy.txt" | head -n1)" +capture start-happy.txt env TD_SESSION_ID=ses_impl "$BIN" -w "$TMP" start "$ISSUE_ID" +capture handoff-happy.txt env TD_SESSION_ID=ses_impl "$BIN" -w "$TMP" handoff "$ISSUE_ID" --done "Captured reviewer handoff context" +capture review-happy.txt env TD_SESSION_ID=ses_impl "$BIN" -w "$TMP" review "$ISSUE_ID" +capture approve-happy.txt env TD_SESSION_ID=ses_reviewer "$BIN" -w "$TMP" approve "$ISSUE_ID" +capture show-happy.txt env TD_SESSION_ID=ses_reviewer "$BIN" -w "$TMP" show "$ISSUE_ID" + +capture db-stale-test.txt go test ./internal/db -run TestUpdateIssueLoggedIfStatusDetectsStaleTransition -v +capture cmd-stale-test.txt go test ./cmd -run TestDescribeStaleTransitionUpdate -v +capture guidance-tests.txt go test ./cmd -run 'TestCloseFollowupGuidance|TestReviewFollowupGuidance|TestApproveFollowupGuidance' -v + +python3 - "$OUT" "$ISSUE_ID" <<'PY' +from html import escape +from pathlib import Path +import sys + +out = Path(sys.argv[1]) +issue_id = sys.argv[2] + +def read(name: str) -> str: + return (out / name).read_text() + +sections = { + "init": read("init.txt"), + "create": read("create-happy.txt"), + "start": read("start-happy.txt"), + "handoff": read("handoff-happy.txt"), + "review": read("review-happy.txt"), + "approve": read("approve-happy.txt"), + "show": read("show-happy.txt"), + "db_stale": read("db-stale-test.txt"), + "cmd_stale": read("cmd-stale-test.txt"), + "guidance": read("guidance-tests.txt"), +} + +html = f""" + + + +td-377aae proof + + + +

td-377aae proof

+

This proof combines two things: the happy-path CLI flow still works end to end for {escape(issue_id)}, and the new stale-transition protection is exercised by deterministic tests that capture the exact messages users now see when a concurrent status change makes their local transition stale.

+ +
+
Happy path: create → start → handoff → review → approve
+
DB guard: refuses stale status write and preserves the newer state
+
CLI wording: explains the concurrent status change and the next command
+
+ +
+
+
Happy Path
+

Review command still works normally

+ 
{escape(sections["review"])}
+
+ +
+
Happy Path
+

Approve still closes the reviewed issue

+ 
{escape(sections["approve"])}
+
+
+ +
+
+
Stale Guard
+

DB rejects a stale concurrent transition

+ 
{escape(sections["db_stale"])}
+
+ +
+
User Message
+

CLI guidance names the new status and next step

+ 
{escape(sections["cmd_stale"])}
+
+
+ +
+
+
Follow-up Guidance
+

Closed-state guidance stays explicit

+ 
{escape(sections["guidance"])}
+
+ +
+
Final State
+

Happy-path issue ends closed with history intact

+ 
{escape(sections["show"])}
+
+
+ + +""" + +(out / "proof.html").write_text(html) +PY + +if ! command -v playwright >/dev/null 2>&1; then + echo "playwright is required to capture proof screenshots" >&2 + exit 1 +fi + +PROOF_URL="file://$OUT/proof.html" +playwright screenshot --device "Desktop Chrome HiDPI" --wait-for-timeout 750 --full-page "$PROOF_URL" "$OUT/proof-full.png" >/dev/null +playwright screenshot --device "Desktop Chrome HiDPI" --wait-for-timeout 750 "$PROOF_URL" "$OUT/proof-normal.png" >/dev/null +playwright screenshot --device "Desktop Chrome HiDPI" --wait-for-timeout 750 "$PROOF_URL#stale" "$OUT/proof-stale.png" >/dev/null diff --git a/proof/td-377aae/show-happy.txt b/proof/td-377aae/show-happy.txt new file mode 100644 index 0000000..929d031 --- /dev/null +++ b/proof/td-377aae/show-happy.txt @@ -0,0 +1,20 @@ +td-c5932a: Proof stale transition workflow +Status: [closed] +Type: task | Priority: P2 + +CURRENT HANDOFF (ses_3bce40, just now): + Done: + - Captured reviewer handoff context + +SESSION LOG: + [18:02] Started work + [18:02] Submitted for review + [18:02] Approved + +GIT STATE: + Started: 53f499a (dispatch/td-377aae-0004) just now + Current: 53f499a (dispatch/td-377aae-0004) +0 commits + +SESSIONS INVOLVED: + ses_3bce40 (implementer) + ses_543a53 (reviewer) diff --git a/proof/td-377aae/start-happy.txt b/proof/td-377aae/start-happy.txt new file mode 100644 index 0000000..11244ab --- /dev/null +++ b/proof/td-377aae/start-happy.txt @@ -0,0 +1,3 @@ +STARTED td-c5932a (session: ses_3bce40) +Git: 53f499a (dispatch/td-377aae-0004) 5 modified, 1 untracked +Warning: Starting with uncommitted changes diff --git a/proof/td-90f95e/PROOF.md b/proof/td-90f95e/PROOF.md new file mode 100644 index 0000000..8094e6d --- /dev/null +++ b/proof/td-90f95e/PROOF.md @@ -0,0 +1,25 @@ +# td-90f95e Proof + +Verified with CLI output captured in this round: + +- `init.txt` +- `create-epic.txt` +- `create-review-task.txt` +- `create-open-task.txt` +- `start-review-task.txt` +- `review-task.txt` +- `list-epic-dot-in-review.txt` +- `list-epic-dot-open-in-progress.txt` +- `close-guidance.txt` +- `status-json.txt` + +What this proves: + +- `td list --epic . -s in_review` resolves the active epic context without focus. +- `td list --epic . -s open,in_progress` also resolves the active epic context without focus. +- `td close` on an already-in-review task now points to `td approve` instead of sending the user back to review. +- `td status --json` returns machine-readable state for scripted lookups. + +Notes: + +- This repository does not include a browser screenshot harness for the CLI task, so the proof artifact here is command output. diff --git a/proof/td-90f95e/close-guidance.txt b/proof/td-90f95e/close-guidance.txt new file mode 100644 index 0000000..3bc6a94 --- /dev/null +++ b/proof/td-90f95e/close-guidance.txt @@ -0,0 +1,2 @@ +ERROR: cannot close own implementation: td-fe5f4f +ERROR: Already in review: td approve td-fe5f4f diff --git a/proof/td-90f95e/create-epic.txt b/proof/td-90f95e/create-epic.txt new file mode 100644 index 0000000..c20a33b --- /dev/null +++ b/proof/td-90f95e/create-epic.txt @@ -0,0 +1 @@ +CREATED td-e16c23 diff --git a/proof/td-90f95e/create-open-task.txt b/proof/td-90f95e/create-open-task.txt new file mode 100644 index 0000000..281045e --- /dev/null +++ b/proof/td-90f95e/create-open-task.txt @@ -0,0 +1 @@ +CREATED td-f91d8a diff --git a/proof/td-90f95e/create-review-task.txt b/proof/td-90f95e/create-review-task.txt new file mode 100644 index 0000000..49ca27b --- /dev/null +++ b/proof/td-90f95e/create-review-task.txt @@ -0,0 +1 @@ +CREATED td-fe5f4f diff --git a/proof/td-90f95e/ids.txt b/proof/td-90f95e/ids.txt new file mode 100644 index 0000000..2ca38bc --- /dev/null +++ b/proof/td-90f95e/ids.txt @@ -0,0 +1,3 @@ +epic=td-e16c23 +review=td-fe5f4f +open=td-f91d8a diff --git a/proof/td-90f95e/init.txt b/proof/td-90f95e/init.txt new file mode 100644 index 0000000..6b80a79 --- /dev/null +++ b/proof/td-90f95e/init.txt @@ -0,0 +1,15 @@ +INITIALIZED /var/folders/9z/_hxsyhcx59d_cbrbhxfk9j000000gn/T/tmp.g7wGDmcKge/.todos +Added .todos/ to .gitignore +Session: ses_a9fc51 + +Tip: Add this to your CLAUDE.md, AGENTS.md, or similar agent file: + +## MANDATORY: Use td for Task Management + +Run td usage --new-session at conversation start (or after /clear). This tells you what to work on next. + +Sessions are automatic (based on terminal/agent context). Optional: +- td session "name" to label the current session +- td session --new to force a new session in the same context + +Use td usage -q after first read. diff --git a/proof/td-90f95e/list-epic-dot-in-review.txt b/proof/td-90f95e/list-epic-dot-in-review.txt new file mode 100644 index 0000000..2845ef6 --- /dev/null +++ b/proof/td-90f95e/list-epic-dot-in-review.txt @@ -0,0 +1 @@ +td-fe5f4f [P1] Review phase child task task [in_review] diff --git a/proof/td-90f95e/list-epic-dot-open-in-progress.txt b/proof/td-90f95e/list-epic-dot-open-in-progress.txt new file mode 100644 index 0000000..ee2b280 --- /dev/null +++ b/proof/td-90f95e/list-epic-dot-open-in-progress.txt @@ -0,0 +1 @@ +td-f91d8a [P1] Review phase open sibling task task [open] diff --git a/proof/td-90f95e/review-task.txt b/proof/td-90f95e/review-task.txt new file mode 100644 index 0000000..9fed1ff --- /dev/null +++ b/proof/td-90f95e/review-task.txt @@ -0,0 +1,2 @@ +Warning: auto-created minimal handoff for td-fe5f4f - consider using 'td handoff' for better documentation +REVIEW REQUESTED td-fe5f4f (session: ses_a9fc51) diff --git a/proof/td-90f95e/start-review-task.txt b/proof/td-90f95e/start-review-task.txt new file mode 100644 index 0000000..08011bb --- /dev/null +++ b/proof/td-90f95e/start-review-task.txt @@ -0,0 +1,3 @@ +STARTED td-fe5f4f (session: ses_a9fc51) +Git: e73c752 (main) 6 modified, 13 untracked +Warning: Starting with uncommitted changes diff --git a/proof/td-90f95e/status-json.txt b/proof/td-90f95e/status-json.txt new file mode 100644 index 0000000..38257c3 --- /dev/null +++ b/proof/td-90f95e/status-json.txt @@ -0,0 +1,62 @@ +{ + "blocked": [], + "in_review": { + "implemented_by_you": [ + { + "id": "td-fe5f4f", + "title": "Review phase child task", + "status": "in_review", + "type": "task", + "priority": "P1", + "points": 0, + "parent_id": "td-e16c23", + "implementer_session": "ses_a9fc51", + "creator_session": "ses_a9fc51", + "reviewer_session": "", + "created_at": "2026-03-27T18:31:04.633958-07:00", + "updated_at": "2026-03-27T18:31:05.106096-07:00", + "minor": false, + "created_branch": "main", + "defer_count": 0 + } + ], + "reviewable_by_you": [], + "total": 1 + }, + "ready_to_start": [ + { + "id": "td-e16c23", + "title": "Review phase epic root", + "status": "open", + "type": "epic", + "priority": "P1", + "points": 0, + "implementer_session": "", + "creator_session": "ses_a9fc51", + "reviewer_session": "", + "created_at": "2026-03-27T18:31:04.468854-07:00", + "updated_at": "2026-03-27T18:31:04.468854-07:00", + "minor": false, + "created_branch": "main", + "defer_count": 0 + }, + { + "id": "td-f91d8a", + "title": "Review phase open sibling task", + "status": "open", + "type": "task", + "priority": "P1", + "points": 0, + "parent_id": "td-e16c23", + "implementer_session": "", + "creator_session": "ses_a9fc51", + "reviewer_session": "", + "created_at": "2026-03-27T18:31:04.794918-07:00", + "updated_at": "2026-03-27T18:31:04.794918-07:00", + "minor": false, + "created_branch": "main", + "defer_count": 0 + } + ], + "session": "ses_a9fc51" +} diff --git a/proof/td-db1f2a/PROOF.md b/proof/td-db1f2a/PROOF.md new file mode 100644 index 0000000..de642da --- /dev/null +++ b/proof/td-db1f2a/PROOF.md @@ -0,0 +1,25 @@ +# td-db1f2a Proof + +Artifacts captured from a fresh temp project: + +- `init.txt` +- `create-warning.txt` +- `start-warning.txt` +- `review-warning.txt` +- `show-warning.txt` +- `create-suppressed.txt` +- `start-suppressed.txt` +- `log-suppressed.txt` +- `review-suppressed.txt` +- `show-suppressed.txt` +- `proof.html` +- `proof-full.png` +- `proof-warning.png` +- `proof-suppressed.png` + +What this proves: + +- `td review` still auto-creates a minimal handoff when no handoff exists. +- The warning is retained when the issue only has routine workflow context. +- The warning is suppressed when substantive session context already exists. +- Both paths still leave the issue in `in_review` with the auto-generated handoff visible in `td show`. diff --git a/proof/td-db1f2a/create-suppressed.txt b/proof/td-db1f2a/create-suppressed.txt new file mode 100644 index 0000000..3e0b380 --- /dev/null +++ b/proof/td-db1f2a/create-suppressed.txt @@ -0,0 +1 @@ +CREATED td-a95255 diff --git a/proof/td-db1f2a/create-warning.txt b/proof/td-db1f2a/create-warning.txt new file mode 100644 index 0000000..15aebd5 --- /dev/null +++ b/proof/td-db1f2a/create-warning.txt @@ -0,0 +1 @@ +CREATED td-3bdf46 diff --git a/proof/td-db1f2a/init.txt b/proof/td-db1f2a/init.txt new file mode 100644 index 0000000..c069278 --- /dev/null +++ b/proof/td-db1f2a/init.txt @@ -0,0 +1,14 @@ +INITIALIZED /tmp/td-db1f2a-proof.kmsmIp/.todos +Session: ses_b6551d + +Tip: Add this to your CLAUDE.md, AGENTS.md, or similar agent file: + +## MANDATORY: Use td for Task Management + +Run td usage --new-session at conversation start (or after /clear). This tells you what to work on next. + +Sessions are automatic (based on terminal/agent context). Optional: +- td session "name" to label the current session +- td session --new to force a new session in the same context + +Use td usage -q after first read. diff --git a/proof/td-db1f2a/log-suppressed.txt b/proof/td-db1f2a/log-suppressed.txt new file mode 100644 index 0000000..d3cc045 --- /dev/null +++ b/proof/td-db1f2a/log-suppressed.txt @@ -0,0 +1 @@ +LOGGED td-a95255 diff --git a/proof/td-db1f2a/project-dir.txt b/proof/td-db1f2a/project-dir.txt new file mode 100644 index 0000000..b1ca263 --- /dev/null +++ b/proof/td-db1f2a/project-dir.txt @@ -0,0 +1 @@ +/tmp/td-db1f2a-proof.kmsmIp diff --git a/proof/td-db1f2a/proof-full.png b/proof/td-db1f2a/proof-full.png new file mode 100644 index 0000000..74447af Binary files /dev/null and b/proof/td-db1f2a/proof-full.png differ diff --git a/proof/td-db1f2a/proof-suppressed.png b/proof/td-db1f2a/proof-suppressed.png new file mode 100644 index 0000000..af8ac62 Binary files /dev/null and b/proof/td-db1f2a/proof-suppressed.png differ diff --git a/proof/td-db1f2a/proof-warning.png b/proof/td-db1f2a/proof-warning.png new file mode 100644 index 0000000..af8ac62 Binary files /dev/null and b/proof/td-db1f2a/proof-warning.png differ diff --git a/proof/td-db1f2a/proof.html b/proof/td-db1f2a/proof.html new file mode 100644 index 0000000..7dec653 --- /dev/null +++ b/proof/td-db1f2a/proof.html @@ -0,0 +1,134 @@ + + + + +td-db1f2a proof + + + +

td-db1f2a proof

+

Fresh CLI proof project showing both paths after the `td review` change: the minimal auto-handoff is still created when missing, but the warning only appears when existing context is not substantive.

+ +
+
Setup
+

Fresh proof project

+ 
INITIALIZED /tmp/td-db1f2a-proof.kmsmIp/.todos
+Session: ses_b6551d
+
+ +
+
+
Warning Retained
+

Routine context only

+

This issue only had the routine Started work log before review, so the warning still fires.

+ 
Warning: auto-created minimal handoff for td-3bdf46 - consider using 'td handoff' for better documentation
+REVIEW REQUESTED td-3bdf46 (session: ses_b6551d)
+
+ +
+
Warning Suppressed
+

Substantive session context already exists

+

This issue had a meaningful progress log before review, so review stays quiet while still creating the handoff.

+ 
REVIEW REQUESTED td-a95255 (session: ses_b6551d)
+
+
+ +
+
+
Auto-Handoff Proof
+

Warning case still gets a minimal handoff

+ 
td-3bdf46: Warning case review task
+Status: [in_review]
+Type: task | Priority: P2
+
+CURRENT HANDOFF (ses_b6551d, just now):
+  Done:
+    - Auto-generated for review submission
+
+SESSION LOG:
+  [17:34] Started work
+  [17:34] Submitted for review
+
+ +
+
Auto-Handoff Proof
+

Suppressed-warning case still gets a minimal handoff

+ 
td-a95255: Suppressed warning review task
+Status: [in_review]
+Type: task | Priority: P2
+
+CURRENT HANDOFF (ses_b6551d, just now):
+  Done:
+    - Auto-generated for review submission
+
+SESSION LOG:
+  [17:34] Started work
+  [17:34] Validated review context and captured remaining edge cases for the reviewer.
+  [17:34] Submitted for review
+
+
+ + diff --git a/proof/td-db1f2a/review-suppressed.txt b/proof/td-db1f2a/review-suppressed.txt new file mode 100644 index 0000000..597884b --- /dev/null +++ b/proof/td-db1f2a/review-suppressed.txt @@ -0,0 +1 @@ +REVIEW REQUESTED td-a95255 (session: ses_b6551d) diff --git a/proof/td-db1f2a/review-warning.txt b/proof/td-db1f2a/review-warning.txt new file mode 100644 index 0000000..7150bc1 --- /dev/null +++ b/proof/td-db1f2a/review-warning.txt @@ -0,0 +1,2 @@ +Warning: auto-created minimal handoff for td-3bdf46 - consider using 'td handoff' for better documentation +REVIEW REQUESTED td-3bdf46 (session: ses_b6551d) diff --git a/proof/td-db1f2a/show-suppressed.txt b/proof/td-db1f2a/show-suppressed.txt new file mode 100644 index 0000000..eec5e89 --- /dev/null +++ b/proof/td-db1f2a/show-suppressed.txt @@ -0,0 +1,17 @@ +td-a95255: Suppressed warning review task +Status: [in_review] +Type: task | Priority: P2 + +CURRENT HANDOFF (ses_b6551d, just now): + Done: + - Auto-generated for review submission + +SESSION LOG: + [17:34] Started work + [17:34] Validated review context and captured remaining edge cases for the reviewer. + [17:34] Submitted for review + +AWAITING REVIEW - requires different session to approve/reject + +SESSIONS INVOLVED: + ses_b6551d (implementer) diff --git a/proof/td-db1f2a/show-warning.txt b/proof/td-db1f2a/show-warning.txt new file mode 100644 index 0000000..da8150d --- /dev/null +++ b/proof/td-db1f2a/show-warning.txt @@ -0,0 +1,16 @@ +td-3bdf46: Warning case review task +Status: [in_review] +Type: task | Priority: P2 + +CURRENT HANDOFF (ses_b6551d, just now): + Done: + - Auto-generated for review submission + +SESSION LOG: + [17:34] Started work + [17:34] Submitted for review + +AWAITING REVIEW - requires different session to approve/reject + +SESSIONS INVOLVED: + ses_b6551d (implementer) diff --git a/proof/td-db1f2a/start-suppressed.txt b/proof/td-db1f2a/start-suppressed.txt new file mode 100644 index 0000000..c5036f3 --- /dev/null +++ b/proof/td-db1f2a/start-suppressed.txt @@ -0,0 +1 @@ +STARTED td-a95255 (session: ses_b6551d) diff --git a/proof/td-db1f2a/start-warning.txt b/proof/td-db1f2a/start-warning.txt new file mode 100644 index 0000000..636c8df --- /dev/null +++ b/proof/td-db1f2a/start-warning.txt @@ -0,0 +1 @@ +STARTED td-3bdf46 (session: ses_b6551d) diff --git a/proof/td-db1f2a/suppressed-id.txt b/proof/td-db1f2a/suppressed-id.txt new file mode 100644 index 0000000..d387826 --- /dev/null +++ b/proof/td-db1f2a/suppressed-id.txt @@ -0,0 +1 @@ +td-a95255 diff --git a/proof/td-db1f2a/warn-id.txt b/proof/td-db1f2a/warn-id.txt new file mode 100644 index 0000000..7dcdbf9 --- /dev/null +++ b/proof/td-db1f2a/warn-id.txt @@ -0,0 +1 @@ +td-3bdf46 diff --git a/scripts/loop-prompt.md b/scripts/loop-prompt.md index b49a127..7f84db2 100644 --- a/scripts/loop-prompt.md +++ b/scripts/loop-prompt.md @@ -146,6 +146,14 @@ td start - Run relevant test suites - For TUI changes: describe what you verified manually +Batch review loops: + +- `EPIC_IDS=.` means "use the active epic context" when a focused issue is not set. +- `td list --epic . -s in_review` and `td list --epic . -s open,in_progress` should work in that batch mode. +- If `EPIC_IDS=.` cannot resolve cleanly, check `td status --json` and `td list --json` first; if there is still no active context, fall back to explicit epic IDs instead of guessing. +- For scripted state lookups, prefer `td status --json` and `td list --json`; do not scrape the human-readable dashboard output. +- Use `td review ` for `open`/`in_progress` work, and `td approve ` once a task is already `in_review`. + ### Step 4: Commit and close ```bash diff --git a/td-task-management/SKILL.md b/td-task-management/SKILL.md index d4275ff..707d019 100644 --- a/td-task-management/SKILL.md +++ b/td-task-management/SKILL.md @@ -149,6 +149,8 @@ td context td-a1b2 # Refresh context when blocker resolves ### Creating/Managing Issues - `td create "title" --type feature --priority P1` - Create +- `td create "title" --description-file body.md --acceptance-file acceptance.md` - Agent-safe rich text +- `cat body.md | td update --append --description-file -` - Append rich text from stdin - `td list` - List all - `td list --status in_progress` - Filter by status - `td block ` - Mark as blocked diff --git a/td-task-management/references/quick_reference.md b/td-task-management/references/quick_reference.md index 56a0d0a..6ec294a 100644 --- a/td-task-management/references/quick_reference.md +++ b/td-task-management/references/quick_reference.md @@ -28,6 +28,8 @@ ### Issue Management - `td create "title" --type feature --priority P1` - Create issue +- `td create "title" --description-file body.md --acceptance-file acceptance.md` - Create with rich markdown safely +- `cat body.md | td update --append --description-file -` - Append rich markdown from stdin - `td list` - List all issues - `td list --status in_progress` - Filter by status - `td show ` - View issue details diff --git a/website/docs/command-reference.md b/website/docs/command-reference.md index 5c7608b..28e9da1 100644 --- a/website/docs/command-reference.md +++ b/website/docs/command-reference.md @@ -10,10 +10,10 @@ Complete reference for all `td` commands. | Command | Description | |---------|-------------| -| `td create "title" [flags]` | Create issue. Flags: `--type`, `--priority`, `--description`, `--parent`, `--epic`, `--minor` | +| `td create "title" [flags]` | Create issue. Flags: `--type`, `--priority`, `--description`, `--description-file`, `--acceptance`, `--acceptance-file`, `--parent`, `--epic`, `--minor` | | `td list [flags]` | List issues. Flags: `--status`, `--type`, `--priority`, `--epic` | | `td show ` | Display full issue details | -| `td update [flags]` | Update fields. Flags: `--title`, `--type`, `--priority`, `--description`, `--labels` | +| `td update [flags]` | Update fields. Flags: `--title`, `--type`, `--priority`, `--description`, `--description-file`, `--acceptance`, `--acceptance-file`, `--labels` | | `td delete ` | Soft-delete issue | | `td restore ` | Restore soft-deleted issue | @@ -50,6 +50,18 @@ The `--defer` and `--due` flags are also available on `td create` and `td update **List filters:** `--all` (include deferred), `--deferred`, `--surfacing`, `--overdue`, `--due-soon` +## Agent-Safe Rich Text Input + +Use `--description-file` and `--acceptance-file` for markdown-heavy fields so shells do not mangle code fences, quotes, or blank lines. Pass `-` to read the full field from stdin. + +```bash +td create "Document sync failure modes" \ + --description-file docs/issue-description.md \ + --acceptance-file docs/issue-acceptance.md + +cat docs/acceptance.md | td update td-a1b2 --append --acceptance-file - +``` + ## Query & Search | Command | Description | diff --git a/website/docs/core-workflow.md b/website/docs/core-workflow.md index edc793b..0f6ef98 100644 --- a/website/docs/core-workflow.md +++ b/website/docs/core-workflow.md @@ -14,12 +14,19 @@ Create issues with a title, type, and optional priority: td create "Add user authentication" --type feature --priority P1 td create "Login button misaligned" --type bug td create "Refactor auth module" --type task --priority P2 +td create "Capture sync edge cases" --description-file docs/issue-description.md ``` **Types:** `bug`, `feature`, `task`, `epic`, `chore` **Priorities:** `P0` (critical) through `P4` (lowest). Defaults to P2 if omitted. +For markdown-heavy descriptions or acceptance criteria, prefer `--description-file` and `--acceptance-file`. They preserve blank lines, indentation, and fenced code blocks exactly, and `-` reads the whole field from stdin: + +```bash +cat docs/acceptance.md | td update td-a1b2 --append --acceptance-file - +``` + ## Starting Work Pick up an issue to work on: