From abcadffe5d6fffc9eae2a6f65e62c162523b6bca Mon Sep 17 00:00:00 2001 From: spacebag <10285712+spacebag@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:59:26 -0700 Subject: [PATCH] Normalize issue ids in relation getters GetIssue normalizes a bare id, but six relation getters did not, so callers that pass the raw argument resolve the issue and then silently get nothing back for its relations. The visible case is `td show`. cmd/show.go:113 keeps the raw arg in issueID. GetIssue on line 138 succeeds because it normalizes its own local copy, leaving issueID bare. Lines 144-158 pass that to GetLogs, GetLatestHandoff, GetLinkedFiles, GetDependencies, GetBlockedBy, and GetStartSnapshot, each of which queries `abc123` against rows keyed `td-abc123`. Every one returns empty, and every call discards its error, so nothing surfaces. Result: `td show abc123` and `td show td-abc123` return the same issue, but the bare form omits CURRENT HANDOFF, SESSION LOG, GIT STATE, and the BLOCKED BY / BLOCKS edges. On a real issue that was 24 lines against 53. The output ends cleanly, so it reads as complete, and a blocked issue appears unblocked. Fixed in the DB layer rather than at the callsite, matching how AddComment, AddHandoff, and about forty other call sites already normalize. A callsite fix would leave the next caller of these getters exposed to the same bug. #199 named GetLatestHandoff as part of the same defect. Its write paths now normalize, so `td comment abc123` works, but the read path it described did not. Test fails without the change, with bare returning 0 where prefixed returns 1. make fmt clean, make test green. --- internal/db/activity.go | 3 + internal/db/bare_id_relations_test.go | 105 ++++++++++++++++++++++++++ internal/db/issue_relations.go | 3 + 3 files changed, 111 insertions(+) create mode 100644 internal/db/bare_id_relations_test.go diff --git a/internal/db/activity.go b/internal/db/activity.go index 78b06427..6b8e3707 100644 --- a/internal/db/activity.go +++ b/internal/db/activity.go @@ -54,6 +54,7 @@ func (db *DB) AddLog(log *models.Log) error { // GetLogs retrieves logs for an issue, including work session logs func (db *DB) GetLogs(issueID string, limit int) ([]models.Log, error) { + issueID = NormalizeIssueID(issueID) // Get logs that are either: // 1. Directly assigned to this issue (issue_id = ?) // 2. Work session logs (issue_id = '') from sessions where this issue is tagged @@ -267,6 +268,7 @@ func (db *DB) AddHandoff(handoff *models.Handoff) error { // GetLatestHandoff retrieves the latest handoff for an issue func (db *DB) GetLatestHandoff(issueID string) (*models.Handoff, error) { + issueID = NormalizeIssueID(issueID) var handoff models.Handoff var doneJSON, remainingJSON, decisionsJSON, uncertainJSON string @@ -784,6 +786,7 @@ func (db *DB) AddGitSnapshot(snapshot *models.GitSnapshot) error { // GetStartSnapshot returns the start snapshot for an issue func (db *DB) GetStartSnapshot(issueID string) (*models.GitSnapshot, error) { + issueID = NormalizeIssueID(issueID) var snapshot models.GitSnapshot err := db.conn.QueryRow(` diff --git a/internal/db/bare_id_relations_test.go b/internal/db/bare_id_relations_test.go new file mode 100644 index 00000000..a96f1c70 --- /dev/null +++ b/internal/db/bare_id_relations_test.go @@ -0,0 +1,105 @@ +package db + +import ( + "testing" + + "github.com/marcus/td/internal/models" +) + +// Relation getters must accept a bare id the same way GetIssue does. Before +// they normalized, `td show abc123` resolved the issue but returned no +// dependencies, logs, handoff, or git snapshot, because those queries ran +// against the raw `abc123` while the rows are keyed `td-abc123`. Each caller +// discards the error, so the omission was silent. See marcus/td#199 for the +// same defect on the write path. +func TestRelationGettersAcceptBareIDs(t *testing.T) { + dir := t.TempDir() + database, err := Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer func() { _ = database.Close() }() + + blocker := &models.Issue{Title: "Blocker", Type: models.TypeTask, Priority: models.PriorityP2} + if err := database.CreateIssue(blocker); err != nil { + t.Fatalf("CreateIssue(blocker) failed: %v", err) + } + dependent := &models.Issue{Title: "Dependent", Type: models.TypeTask, Priority: models.PriorityP2} + if err := database.CreateIssue(dependent); err != nil { + t.Fatalf("CreateIssue(dependent) failed: %v", err) + } + + if err := database.AddDependency(dependent.ID, blocker.ID, "depends_on"); err != nil { + t.Fatalf("AddDependency failed: %v", err) + } + if err := database.AddLog(&models.Log{IssueID: dependent.ID, Message: "entry"}); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + bareDependent := stripPrefix(t, dependent.ID) + bareBlocker := stripPrefix(t, blocker.ID) + + t.Run("GetDependencies", func(t *testing.T) { + prefixed, err := database.GetDependencies(dependent.ID) + if err != nil { + t.Fatalf("GetDependencies(prefixed) failed: %v", err) + } + if len(prefixed) != 1 { + t.Fatalf("GetDependencies(prefixed) returned %d deps, want 1", len(prefixed)) + } + bare, err := database.GetDependencies(bareDependent) + if err != nil { + t.Fatalf("GetDependencies(bare) failed: %v", err) + } + if len(bare) != len(prefixed) { + t.Errorf("bare id returned %d deps, prefixed returned %d; the two must agree", + len(bare), len(prefixed)) + } + }) + + t.Run("GetBlockedBy", func(t *testing.T) { + prefixed, err := database.GetBlockedBy(blocker.ID) + if err != nil { + t.Fatalf("GetBlockedBy(prefixed) failed: %v", err) + } + if len(prefixed) != 1 { + t.Fatalf("GetBlockedBy(prefixed) returned %d, want 1", len(prefixed)) + } + bare, err := database.GetBlockedBy(bareBlocker) + if err != nil { + t.Fatalf("GetBlockedBy(bare) failed: %v", err) + } + if len(bare) != len(prefixed) { + t.Errorf("bare id returned %d, prefixed returned %d; the two must agree", + len(bare), len(prefixed)) + } + }) + + t.Run("GetLogs", func(t *testing.T) { + prefixed, err := database.GetLogs(dependent.ID, 0) + if err != nil { + t.Fatalf("GetLogs(prefixed) failed: %v", err) + } + if len(prefixed) == 0 { + t.Fatal("GetLogs(prefixed) returned no logs, want at least 1") + } + bare, err := database.GetLogs(bareDependent, 0) + if err != nil { + t.Fatalf("GetLogs(bare) failed: %v", err) + } + if len(bare) != len(prefixed) { + t.Errorf("bare id returned %d logs, prefixed returned %d; the two must agree", + len(bare), len(prefixed)) + } + }) +} + +// stripPrefix returns the id without its td- prefix, which is what a user types +// when they copy an id out of a listing and drop the prefix. +func stripPrefix(t *testing.T, id string) string { + t.Helper() + if len(id) <= len(idPrefix) || id[:len(idPrefix)] != idPrefix { + t.Fatalf("expected %q to carry the %q prefix", id, idPrefix) + } + return id[len(idPrefix):] +} diff --git a/internal/db/issue_relations.go b/internal/db/issue_relations.go index 0f455ef9..1b34eae3 100644 --- a/internal/db/issue_relations.go +++ b/internal/db/issue_relations.go @@ -402,6 +402,7 @@ func (db *DB) GetIssueDependencyRelations(issueID string) ([]models.IssueDepende // GetDependencies returns what an issue depends on func (db *DB) GetDependencies(issueID string) ([]string, error) { + issueID = NormalizeIssueID(issueID) rows, err := db.conn.Query(` SELECT depends_on_id FROM issue_dependencies WHERE issue_id = ? AND relation_type = 'depends_on' `, issueID) @@ -426,6 +427,7 @@ func (db *DB) GetDependencies(issueID string) ([]string, error) { // GetBlockedBy returns what issues are blocked by this issue func (db *DB) GetBlockedBy(issueID string) ([]string, error) { + issueID = NormalizeIssueID(issueID) rows, err := db.conn.Query(` SELECT issue_id FROM issue_dependencies WHERE depends_on_id = ? AND relation_type = 'depends_on' `, issueID) @@ -699,6 +701,7 @@ func (db *DB) UnlinkFile(issueID, filePath string) error { // GetLinkedFiles returns files linked to an issue func (db *DB) GetLinkedFiles(issueID string) ([]models.IssueFile, error) { + issueID = NormalizeIssueID(issueID) rows, err := db.conn.Query(` SELECT CAST(id AS TEXT), issue_id, file_path, role, linked_sha, linked_at FROM issue_files WHERE issue_id = ? ORDER BY role, file_path