Fix transaction.go AddDependency to match dependencies.go (Decision 004)

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>
This commit is contained in:
Steve Yegge
2025-12-18 11:14:06 -08:00
parent ee3d51fc21
commit 4981e84e86

View File

@@ -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 { func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependency, actor string) error {
// Validate dependency type // Validate dependency type
if !dep.Type.IsValid() { 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 // Validate that both issues exist
@@ -637,9 +637,11 @@ func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependen
dep.CreatedBy = actor dep.CreatedBy = actor
} }
// Cycle detection // Cycle detection - skip for relates-to (inherently bidirectional)
var cycleExists bool // See dependencies.go for full rationale on cycle prevention
err = t.conn.QueryRowContext(ctx, ` if dep.Type != types.DepRelatesTo {
var cycleExists bool
err = t.conn.QueryRowContext(ctx, `
WITH RECURSIVE paths AS ( WITH RECURSIVE paths AS (
SELECT SELECT
issue_id, issue_id,
@@ -664,20 +666,21 @@ func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependen
) )
`, dep.DependsOnID, dep.IssueID).Scan(&cycleExists) `, dep.DependsOnID, dep.IssueID).Scan(&cycleExists)
if err != nil { if err != nil {
return fmt.Errorf("failed to check for cycles: %w", err) 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 { // Insert dependency (including metadata and thread_id for edge consolidation - Decision 004)
return fmt.Errorf("cannot add dependency: would create a cycle (%s → %s → ... → %s)",
dep.IssueID, dep.DependsOnID, dep.IssueID)
}
// Insert dependency
_, err = t.conn.ExecContext(ctx, ` _, err = t.conn.ExecContext(ctx, `
INSERT INTO dependencies (issue_id, depends_on_id, type, created_at, created_by) INSERT INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id)
VALUES (?, ?, ?, ?, ?) VALUES (?, ?, ?, ?, ?, ?, ?)
`, dep.IssueID, dep.DependsOnID, dep.Type, dep.CreatedAt, dep.CreatedBy) `, dep.IssueID, dep.DependsOnID, dep.Type, dep.CreatedAt, dep.CreatedBy, dep.Metadata, dep.ThreadID)
if err != nil { if err != nil {
return fmt.Errorf("failed to add dependency: %w", err) return fmt.Errorf("failed to add dependency: %w", err)
} }