From 4981e84e860b8068be1753456547dd062b01dca2 Mon Sep 17 00:00:00 2001
From: Steve Yegge <steve.yegge@gmail.com>
Date: Thu, 18 Dec 2025 11:14:06 -0800
Subject: [PATCH] Fix transaction.go AddDependency to match dependencies.go
 (Decision 004)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Code review found three issues where transaction.go diverged from the
main dependencies.go implementation:

1. Add relates-to exemption from cycle detection - bidirectional
   relationships are valid and should not trigger cycle errors

2. Add metadata and thread_id fields to INSERT - required for
   replies-to threading to work in transaction context

3. Update error message to match dependencies.go wording

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 internal/storage/sqlite/transaction.go | 33 ++++++++++++++------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/internal/storage/sqlite/transaction.go b/internal/storage/sqlite/transaction.go
index 883c77d9..abfa933d 100644
--- a/internal/storage/sqlite/transaction.go
+++ b/internal/storage/sqlite/transaction.go
@@ -597,7 +597,7 @@ func (t *sqliteTxStorage) DeleteIssue(ctx context.Context, id string) error {
 func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependency, actor string) error {
 	// Validate dependency type
 	if !dep.Type.IsValid() {
-		return fmt.Errorf("invalid dependency type: %s (must be blocks, related, parent-child, or discovered-from)", dep.Type)
+		return fmt.Errorf("invalid dependency type: %q (must be non-empty string, max 50 chars)", dep.Type)
 	}
 
 	// Validate that both issues exist
@@ -637,9 +637,11 @@ func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependen
 		dep.CreatedBy = actor
 	}
 
-	// Cycle detection
-	var cycleExists bool
-	err = t.conn.QueryRowContext(ctx, `
+	// Cycle detection - skip for relates-to (inherently bidirectional)
+	// See dependencies.go for full rationale on cycle prevention
+	if dep.Type != types.DepRelatesTo {
+		var cycleExists bool
+		err = t.conn.QueryRowContext(ctx, `
 		WITH RECURSIVE paths AS (
 			SELECT
 				issue_id,
@@ -664,20 +666,21 @@ func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependen
 		)
 	`, dep.DependsOnID, dep.IssueID).Scan(&cycleExists)
 
-	if err != nil {
-		return fmt.Errorf("failed to check for cycles: %w", err)
+		if err != nil {
+			return fmt.Errorf("failed to check for cycles: %w", err)
+		}
+
+		if cycleExists {
+			return fmt.Errorf("cannot add dependency: would create a cycle (%s → %s → ... → %s)",
+				dep.IssueID, dep.DependsOnID, dep.IssueID)
+		}
 	}
 
-	if cycleExists {
-		return fmt.Errorf("cannot add dependency: would create a cycle (%s → %s → ... → %s)",
-			dep.IssueID, dep.DependsOnID, dep.IssueID)
-	}
-
-	// Insert dependency
+	// Insert dependency (including metadata and thread_id for edge consolidation - Decision 004)
 	_, err = t.conn.ExecContext(ctx, `
-		INSERT INTO dependencies (issue_id, depends_on_id, type, created_at, created_by)
-		VALUES (?, ?, ?, ?, ?)
-	`, dep.IssueID, dep.DependsOnID, dep.Type, dep.CreatedAt, dep.CreatedBy)
+		INSERT INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id)
+		VALUES (?, ?, ?, ?, ?, ?, ?)
+	`, dep.IssueID, dep.DependsOnID, dep.Type, dep.CreatedAt, dep.CreatedBy, dep.Metadata, dep.ThreadID)
 	if err != nil {
 		return fmt.Errorf("failed to add dependency: %w", err)
 	}