diff --git a/internal/links/extract.go b/internal/links/extract.go index 12a28d00..db7a720d 100644 --- a/internal/links/extract.go +++ b/internal/links/extract.go @@ -72,6 +72,21 @@ type WikiLinkRef struct { // not the code-stripped buffer the parser used to find // outside-code matches. Position int + + // RawKey is the untrimmed unescaped key segment (everything + // before the first unescaped pipe, or the whole body if no + // pipe). Populated for `WikiLinkKindRef` so the ref→title + // fallback in the store layer can mirror the renderer's + // untrimmed title lookup at web/src/lib/utils/markdown.ts:541-543 + // — an item literally titled `" TASK-5 "` (with surrounding + // whitespace) resolves via the renderer's untrimmed key but + // would miss a canonical-trimmed `"TASK-5"` lookup. Codex + // round 10 P2. + // + // Empty for other kinds — title kind already preserves the + // untrimmed body in Title, and workspace_ref kinds are + // whitespace-free by construction. + RawKey string } // REF_PATTERN matches a Pad item ref like TASK-5 or BUG-585. Mirrors @@ -447,11 +462,14 @@ func isInRanges(pos int, ranges [][2]int) bool { // any fenced or inline code region, parses each into a WikiLinkRef, // and returns them in source order. // -// Phase 1 of PLAN-1593: only ref-form links (`[[REF-N]]` / -// `[[REF-N|Display]]`) populate the returned slice. Title-form and -// workspace_ref-form links are recognized at parse time (so the -// caller can persist them as `target_kind` placeholders in Phase 2) -// but are NOT emitted today — Phase 2 will flip that switch. +// Phase 2a of PLAN-1593 (TASK-1595): emits ref-form (`[[REF-N]]`) +// AND title-form (`[[Title]]`, `[[collection/Title]]`) links. +// Cross-workspace `[[workspace::REF]]` links are recognized by +// parseBody but still gated out — TASK-1597 (Phase 2b) lifts that +// last gate once the request-independent per-workspace ACL helper +// lands. The two-step rollout is deliberate: cross-ws backlinks +// need visibility plumbing that doesn't exist yet, so emitting the +// rows now would leak indexed-but-unqueryable data. // // Returns an empty slice on empty input. Never returns an error — // any bracket sequence that fails to parse is silently skipped @@ -478,10 +496,14 @@ func ExtractWikiLinks(content string) []WikiLinkRef { continue } ref.Position = linkStart - // Phase 1: only emit ref-form. Drop title and workspace_ref - // rows so the Phase-1 store sees only what Phase 1 promises - // to index. Phase 2 will remove this gate. - if ref.Kind != WikiLinkKindRef { + // Phase 2a (TASK-1595): emit ref AND title kinds. The + // workspace_ref gate stays until TASK-1597 ships the + // per-workspace ACL helper — without it the cross-ws + // inbound query can't honor source-workspace visibility, + // and emitting rows we can't safely query just bloats + // the index. parseBody still RECOGNIZES the kind so + // removing the gate in Phase 2b is a one-line change. + if ref.Kind == WikiLinkKindWorkspaceRef { continue } out = append(out, *ref) @@ -511,23 +533,32 @@ func parseBody(body string) *WikiLinkRef { hasDisplay = true body = key } - // The key/ref side still gets trimmed because refPattern is - // anchored — leading/trailing whitespace would force the whole - // body to fall through to the title kind even though the - // renderer would resolve it as a ref. parseBody's job is to - // recognize the SHAPE; whitespace forgiveness in the key is - // part of that. - body = unescapeWikiBody(strings.TrimSpace(body)) - if body == "" { + // Unescape the post-split key. KEEP UNTRIMMED for the title-kind + // fallthrough so the index mirrors the renderer's whitespace- + // sensitive title resolution: the renderer compares items.title + // against `key` directly with no implicit trim + // (web/src/lib/utils/markdown.ts:541-543). If we trimmed here, + // `[[ Foo ]]` would index a backlink to item "Foo" that the UI + // renders as broken, creating ghost entries in the backlinks + // panel. Codex round 9 P2. + bodyUnescaped := unescapeWikiBody(body) + if bodyUnescaped == "" { return nil } + // Trimmed copy for ref / workspace_ref shape detection only. + // Refs are whitespace-free by construction (`REF-N`), so this + // is forgiveness for `[[ TASK-5 ]]` typed by hand — matches + // the renderer's `key.trim()` at L503/L506. The trimmed value + // is NEVER stored as target_title. + trimmed := strings.TrimSpace(bodyUnescaped) - // Cross-workspace form: `workspace-slug::REF`. The `::` separator - // is unambiguous; if it's present, the workspace + ref must each - // match their patterns or the whole thing falls back to title. - if sep := strings.Index(body, "::"); sep >= 0 { - ws := strings.TrimSpace(body[:sep]) - rest := strings.TrimSpace(body[sep+2:]) + // Cross-workspace form: `workspace-slug::REF`. The `::` + // separator is unambiguous; if it's present, the workspace + + // ref must each match their patterns or the whole thing falls + // back to title. + if sep := strings.Index(trimmed, "::"); sep >= 0 { + ws := strings.TrimSpace(trimmed[:sep]) + rest := strings.TrimSpace(trimmed[sep+2:]) if isWorkspaceSlug(ws) && refPattern.MatchString(rest) { return &WikiLinkRef{ Kind: WikiLinkKindWorkspaceRef, @@ -540,30 +571,33 @@ func parseBody(body string) *WikiLinkRef { // Fall through to title — the renderer's fallback policy. } - // Ref form: a bare REF-N pattern. Normalize the prefix to upper- - // case at this single chokepoint — collection prefixes are - // canonically uppercase in `collections.prefix`, and the - // resolver/backlinks queries compare against that column. The - // renderer accepts mixed case for input convenience; we store - // the canonical form so the index has one shape per (workspace, - // prefix, number) and downstream callers don't need to be - // case-aware (Codex round-1 P2). - if refPattern.MatchString(body) { + // Ref form: a bare REF-N pattern (trimmed-shape check). Normalize + // the prefix to upper-case at this single chokepoint — + // collection prefixes are canonically uppercase in + // `collections.prefix`, and the resolver/backlinks queries + // compare against that column. The renderer accepts mixed case + // for input convenience; we store the canonical form so the + // index has one shape per (workspace, prefix, number) and + // downstream callers don't need to be case-aware (Codex + // round-1 P2). + if refPattern.MatchString(trimmed) { return &WikiLinkRef{ Kind: WikiLinkKindRef, - Ref: canonicalizeRef(body), + Ref: canonicalizeRef(trimmed), + RawKey: bodyUnescaped, // untrimmed, for ref→title fallback Display: display, HasDisplay: hasDisplay, } } // Legacy collection-qualified title: `collection/Title`. We - // treat the whole body as the title for storage; the resolver - // in Phase 2 will split on `/` to bias the lookup. - // Plain legacy title. + // treat the whole UNTRIMMED body as the title for storage; the + // resolver in Phase 2 will split on `/` to bias the lookup. + // Plain legacy title — also untrimmed. Whitespace in the body + // is preserved verbatim per the renderer. return &WikiLinkRef{ Kind: WikiLinkKindTitle, - Title: body, + Title: bodyUnescaped, Display: display, HasDisplay: hasDisplay, } diff --git a/internal/links/extract_test.go b/internal/links/extract_test.go index bb32fc86..0e3ded48 100644 --- a/internal/links/extract_test.go +++ b/internal/links/extract_test.go @@ -61,28 +61,168 @@ func TestExtractWikiLinks_RefForm(t *testing.T) { } } -// TestExtractWikiLinks_NonRefFormsHidden verifies that Phase 1's gate -// suppresses title and workspace_ref kinds from the returned slice — -// even though parseBody recognizes them. Once Phase 2 lands and the -// gate is removed, these inputs must start emitting WikiLinkRefs of -// the corresponding kinds; this test will need updating then. -func TestExtractWikiLinks_NonRefFormsHidden(t *testing.T) { +// TestExtractWikiLinks_TitleForms covers Phase 2a title emission: +// plain `[[Title]]` and `[[collection/Title]]` are now returned with +// kind=title and the body stored verbatim. The collection-qualified +// form is stored AS-WRITTEN (e.g. "docs/Setup") so the resolver can +// try the renderer's order — full-key title match first, `/`-split +// only on miss — without losing information about how the link was +// typed. Codex finding #3 from the planning round. +func TestExtractWikiLinks_TitleForms(t *testing.T) { + t.Run("plain title", func(t *testing.T) { + got := ExtractWikiLinks("Click [[Some Title]] here.") + if len(got) != 1 { + t.Fatalf("expected 1 link, got %d: %+v", len(got), got) + } + if got[0].Kind != WikiLinkKindTitle { + t.Errorf("Kind: got %q, want title", got[0].Kind) + } + if got[0].Title != "Some Title" { + t.Errorf("Title: got %q, want %q", got[0].Title, "Some Title") + } + if got[0].Position != 6 { + t.Errorf("Position: got %d, want 6", got[0].Position) + } + }) + + t.Run("collection-qualified title stored verbatim", func(t *testing.T) { + got := ExtractWikiLinks("Look at [[docs/Setup]] for help.") + if len(got) != 1 { + t.Fatalf("expected 1 link, got %d", len(got)) + } + if got[0].Kind != WikiLinkKindTitle { + t.Errorf("Kind: got %q, want title", got[0].Kind) + } + // Verbatim — DON'T pre-split. An item literally titled + // "docs/Setup" must resolve before the qualified-form + // fallback fires; storing the split here would lose that. + if got[0].Title != "docs/Setup" { + t.Errorf("Title: got %q, want %q", got[0].Title, "docs/Setup") + } + }) + + t.Run("title with display alias", func(t *testing.T) { + got := ExtractWikiLinks("See [[Some Title|the page]] for context.") + if len(got) != 1 { + t.Fatalf("expected 1 link, got %d", len(got)) + } + if got[0].Kind != WikiLinkKindTitle { + t.Errorf("Kind: got %q, want title", got[0].Kind) + } + if got[0].Title != "Some Title" { + t.Errorf("Title: got %q, want %q", got[0].Title, "Some Title") + } + if got[0].Display != "the page" || !got[0].HasDisplay { + t.Errorf("Display: got %q (has=%v), want %q (true)", + got[0].Display, got[0].HasDisplay, "the page") + } + }) + + t.Run("multiple titles in one body", func(t *testing.T) { + got := ExtractWikiLinks("[[First]] and [[Second Title]] and [[third]].") + if len(got) != 3 { + t.Fatalf("expected 3 links, got %d", len(got)) + } + wantTitles := []string{"First", "Second Title", "third"} + for i, w := range wantTitles { + if got[i].Kind != WikiLinkKindTitle { + t.Errorf("[%d] Kind: got %q, want title", i, got[i].Kind) + } + if got[i].Title != w { + t.Errorf("[%d] Title: got %q, want %q", i, got[i].Title, w) + } + } + }) +} + +// TestExtractWikiLinks_TitlePreservesWhitespace regresses Codex +// round 9 P2 against PR #621. The renderer doesn't trim before +// title matching (web/src/lib/utils/markdown.ts:541-543), so an +// item titled "Foo" doesn't match `[[ Foo ]]`. The extractor must +// preserve whitespace in the title kind too, or the index would +// surface backlinks the UI can't actually click on. Ref/workspace_ref +// shape detection still trims (the renderer does the same). +func TestExtractWikiLinks_TitlePreservesWhitespace(t *testing.T) { + t.Run("leading and trailing whitespace preserved in title", func(t *testing.T) { + got := ExtractWikiLinks("See [[ Foo ]] for details.") + if len(got) != 1 { + t.Fatalf("expected 1 link, got %d", len(got)) + } + if got[0].Kind != WikiLinkKindTitle { + t.Errorf("Kind: got %q, want title", got[0].Kind) + } + if got[0].Title != " Foo " { + t.Errorf("Title should preserve whitespace, got %q want %q", got[0].Title, " Foo ") + } + }) + + t.Run("padded ref still parses as ref (whitespace forgiveness)", func(t *testing.T) { + got := ExtractWikiLinks("See [[ TASK-5 ]] please.") + if len(got) != 1 { + t.Fatalf("expected 1 link, got %d", len(got)) + } + if got[0].Kind != WikiLinkKindRef { + t.Errorf("padded ref: Kind got %q want ref", got[0].Kind) + } + if got[0].Ref != "TASK-5" { + t.Errorf("padded ref: Ref got %q want TASK-5", got[0].Ref) + } + }) + + t.Run("internal whitespace preserved in title", func(t *testing.T) { + got := ExtractWikiLinks("See [[Project Goals]] (two spaces inside).") + if len(got) != 1 { + t.Fatalf("expected 1 link, got %d", len(got)) + } + if got[0].Title != "Project Goals" { + t.Errorf("internal whitespace lost: got %q want %q", got[0].Title, "Project Goals") + } + }) +} + +// TestExtractWikiLinks_WorkspaceRefStillHidden documents that Phase 2a +// continues to gate workspace_ref kinds. TASK-1597 lifts this gate +// once the request-independent ACL helper lands — until then, +// emitting these rows would create indexed-but-unqueryable data +// (the cross-ws inbound query can't honor source-workspace +// visibility yet). +func TestExtractWikiLinks_WorkspaceRefStillHidden(t *testing.T) { cases := []string{ - "Click [[Some Title]] here.", // legacy title - "Look at [[docs/Setup]] for help.", // collection-qualified - "Cross [[other-ws::TASK-9]] over.", // workspace_ref - "Cross [[other-ws::TASK-9|over]] too.", // workspace_ref + display + "Cross [[other-ws::TASK-9]] over.", + "Cross [[other-ws::TASK-9|over]] too.", } for _, c := range cases { t.Run(c, func(t *testing.T) { got := ExtractWikiLinks(c) if len(got) != 0 { - t.Errorf("expected 0 links (Phase 1 hides non-ref forms), got %d: %+v", len(got), got) + t.Errorf("expected 0 (Phase 2a still gates workspace_ref), got %d: %+v", + len(got), got) } }) } } +// TestExtractWikiLinks_TitleCodeBlockExclusion ensures Phase 2a +// titles are excluded from fenced / inline code just like refs were +// in Phase 1. The exclusion happens at the gate-independent +// outer scan (linkStart vs ranges), so it's worth a smoke test to +// catch any regression from lifting the gate. +func TestExtractWikiLinks_TitleCodeBlockExclusion(t *testing.T) { + content := "Real: [[Outside Title]]\n" + + "```\n" + + "Fake: [[Inside Title]]\n" + + "```\n" + + "Inline `[[Also Inside]]` then [[Last One]]." + got := ExtractWikiLinks(content) + if len(got) != 2 { + t.Fatalf("expected 2 titles, got %d: %+v", len(got), got) + } + if got[0].Title != "Outside Title" || got[1].Title != "Last One" { + t.Errorf("got titles %q / %q, want %q / %q", + got[0].Title, got[1].Title, "Outside Title", "Last One") + } +} + // TestExtractWikiLinks_CodeBlocksExcluded asserts the headline behavior // decision from PLAN-1593: [[REF]] inside fenced or inline code is NOT // a real link and must be skipped. @@ -378,12 +518,21 @@ func TestExtractWikiLinks_Edge(t *testing.T) { t.Run(c.name, func(t *testing.T) { got := ExtractWikiLinks(c.content) for _, g := range got { - // Anything emitted must at least be a valid ref-form. - if g.Kind != WikiLinkKindRef { - t.Errorf("emitted non-ref kind in Phase 1: %+v", g) - } - if g.Ref == "" { - t.Errorf("emitted ref-kind with empty Ref: %+v", g) + // Phase 2a: emitted kinds are ref or title; cross-ws + // is still gated. Whatever the kind, the matching + // identifier field must be non-empty so downstream + // consumers can rely on it. + switch g.Kind { + case WikiLinkKindRef: + if g.Ref == "" { + t.Errorf("emitted ref-kind with empty Ref: %+v", g) + } + case WikiLinkKindTitle: + if g.Title == "" { + t.Errorf("emitted title-kind with empty Title: %+v", g) + } + default: + t.Errorf("emitted unexpected kind in Phase 2a: %+v", g) } } }) @@ -442,13 +591,20 @@ func TestExtractWikiLinks_RefVsTitleFallback(t *testing.T) { t.Errorf("Ref: got %q want TASK-5", got[0].Ref) } }) - t.Run("non-ref body falls to title and is hidden", func(t *testing.T) { + t.Run("non-ref body falls to title (Phase 2a emits)", func(t *testing.T) { // "5-Task" (number-led) doesn't match REF_PATTERN even // with the relaxed case rule; parseBody returns a - // title-kind ref; Phase 1 gates it out. + // title-kind ref; Phase 2a now emits title kinds, so the + // body is preserved verbatim as the Title field. got := ExtractWikiLinks("[[5-Task]]") - if len(got) != 0 { - t.Errorf("expected 0 (title-kind hidden), got %+v", got) + if len(got) != 1 { + t.Fatalf("expected 1 title-kind row, got %d: %+v", len(got), got) + } + if got[0].Kind != WikiLinkKindTitle { + t.Errorf("Kind: got %q, want title", got[0].Kind) + } + if got[0].Title != "5-Task" { + t.Errorf("Title: got %q, want %q", got[0].Title, "5-Task") } }) } @@ -633,9 +789,9 @@ func TestCanonicalizeRef(t *testing.T) { } // assertLinks compares two WikiLinkRef slices for the fields Phase 1 -// cares about. Doesn't enforce equality on fields not yet emitted -// (Title, WorkspaceSlug) so adding test cases for those in Phase 2 -// won't require rewriting these comparisons. +// + Phase 2a care about. WorkspaceSlug is excluded because Phase 2b +// (TASK-1597) is the first to emit workspace_ref kinds; once that +// lands the helper grows another comparison line. func assertLinks(t *testing.T, got, want []WikiLinkRef) { t.Helper() if len(got) != len(want) { @@ -648,6 +804,9 @@ func assertLinks(t *testing.T, got, want []WikiLinkRef) { if got[i].Ref != want[i].Ref { t.Errorf("[%d] Ref: got %q, want %q", i, got[i].Ref, want[i].Ref) } + if got[i].Title != want[i].Title { + t.Errorf("[%d] Title: got %q, want %q", i, got[i].Title, want[i].Title) + } if got[i].Display != want[i].Display { t.Errorf("[%d] Display: got %q, want %q", i, got[i].Display, want[i].Display) } diff --git a/internal/links/links.go b/internal/links/links.go index e4c7d479..55840fc9 100644 --- a/internal/links/links.go +++ b/internal/links/links.go @@ -1,12 +1,166 @@ package links +import ( + "regexp" + "strings" +) + // ReplaceTitle replaces all [[oldTitle]] with [[newTitle]] in content. +// LEGACY helper used by the document-rename path; case-sensitive, +// no-pipe forms only. For item rename use RewriteWikiTitle below, +// which also handles `[[Title|alias]]`, `[[/Title]]`, and +// `[[/Title|alias]]` and matches case-insensitively to mirror +// the renderer's title resolution. func ReplaceTitle(content, oldTitle, newTitle string) string { old := "[[" + oldTitle + "]]" new := "[[" + newTitle + "]]" return replaceAll(content, old, new) } +// RewriteWikiTitle rewrites the four title-form wiki-link shapes that +// resolve to an item titled `oldTitle` in collection `collSlug`, +// substituting `newTitle` for the title portion and preserving any +// optional display alias verbatim: +// +// [[Old Title]] → [[New Title]] +// [[Old Title|alias]] → [[New Title|alias]] +// [[/Old Title]] → [[/New Title]] +// [[/Old Title|alias]] → [[/New Title|alias]] +// +// The title segment matches case-insensitively because resolveTitleTx +// also resolves titles case-insensitively — a source body that wrote +// `[[old title]]` and resolved to "Old Title" via LOWER() comparison +// must get rewritten by this function or the cascade leaves it broken +// (Codex review of TASK-1595 round 1). +// +// Returns content unchanged if `oldTitle` is empty or equal to +// `newTitle`. Caller is responsible for invoking once per rename; +// repeat application is safe but does no useful work. +// +// Known limitation: titles containing the wiki-link escape characters +// (`]`, `|`, `\`) get stored in source content as `\]`, `\|`, `\\`, +// which the editor's grammar at web/src/lib/utils/markdown.ts:461 +// supports. This rewriter does NOT attempt escape-aware matching on +// the TITLE segment — a title literally containing `]` would be +// stored escaped and would fail to match the regex's `oldTitle` +// literal. The same limitation exists in the legacy ReplaceTitle +// helper above and the document-rename path; items with such titles +// are vanishingly rare in practice (an item titled `My [Plan]` +// would be a stretch). Promotable to a separate task if a real user +// hits it. +func RewriteWikiTitle(content, oldTitle, newTitle, collSlug string) string { + if oldTitle == "" || oldTitle == newTitle { + return content + } + // Build per-rename regex. The capturing groups: + // 1: optional `/` prefix (or empty) + // 2: the title segment (matched case-insensitively) + // 3: optional `|display` suffix including the pipe (or empty) + // + // The display segment uses the same `(?:\\.|[^\]\\])*` grammar as + // the editor (markdown.ts:461) so an alias with escaped `]`/`|` + // inside doesn't end the match early. Inline `(?i:...)` scopes + // the case-insensitivity to the title segment only — the slug + // portion compares against c.slug which is canonically lowercase, + // and we don't want to accidentally fold case on the optional + // display either. + escSlug := regexp.QuoteMeta(collSlug) + escTitle := regexp.QuoteMeta(oldTitle) + pattern := `\[\[((?:` + escSlug + `/)?)(` + `(?i:` + escTitle + `))((?:\|(?:\\.|[^\]\\])*)?)\]\]` + re := regexp.MustCompile(pattern) + return re.ReplaceAllStringFunc(content, func(match string) string { + groups := re.FindStringSubmatch(match) + if len(groups) != 4 { + return match // defensive — shouldn't happen given the literal pattern + } + return "[[" + groups[1] + newTitle + groups[3] + "]]" + }) +} + +// RewriteBracketAt rewrites the wiki-link bracket starting at byte +// position `position` in `content`, replacing the bracket's title +// segment with `newTitle`. Used by the rename cascade to rewrite +// only the specific bracket whose item_wiki_links row resolves to +// the renamed item — avoiding the broad-regex hazard from Codex +// round 7 finding 2, where an unrelated `[[OldTitle|alias]]` +// pointing at a literal-pipe-titled item B was corrupted when +// item A "Old Title" was renamed. +// +// The bracket's optional `/` prefix and `|` +// suffix are preserved verbatim. The function does a defensive +// title-segment check so a position whose bracket no longer +// matches the expected target_title (e.g. due to a prior edit +// that shifted offsets without our index catching up) leaves the +// content unchanged — the caller's replaceWikiLinks re-parse will +// reconcile the index regardless. +// +// Matching cases: +// +// - Bracket body equals targetTitle (case-insensitive) — replace +// the whole body with `newTitle`. +// - Bracket body starts with `targetTitle + "|"` — replace just +// the title segment, preserve the `|display` suffix verbatim. +// - Bracket body equals `/` — replace the +// trailing title segment, preserve the slug prefix. +// - Same with `|display` suffix. +// +// Otherwise the content is returned unchanged. +// +// Like the legacy ReplaceTitle helper, this does NOT distinguish +// code regions from prose. A bracket inside fenced code at the +// recorded position WILL be rewritten — matches the document +// rename path's behavior. +func RewriteBracketAt(content string, position int, targetTitle, newTitle, collSlug string) string { + if position < 0 || position+2 > len(content) { + return content + } + if content[position:position+2] != "[[" { + return content + } + rest := content[position+2:] + closeIdx := strings.Index(rest, "]]") + if closeIdx < 0 { + return content + } + body := rest[:closeIdx] + bracketEnd := position + 2 + closeIdx + 2 // past `]]` + + tLower := strings.ToLower(body) + ttLower := strings.ToLower(targetTitle) + + // Compose the new title segment. If target_title starts with + // `/`, preserve that prefix on output so a qualified + // body like `[[tasks/Old Title]]` becomes `[[tasks/New Title]]` + // — the cascade SELECT already proved this row points at the + // renamed item via stage-2 qualified-fallback resolution, so + // the slug is guaranteed to match collSlug. + newSegment := newTitle + if collSlug != "" { + pfx := collSlug + "/" + if strings.HasPrefix(strings.ToLower(targetTitle), strings.ToLower(pfx)) { + newSegment = pfx + newTitle + } + } + + // Case 1: body equals target_title (case-insensitive) — no + // display segment, replace whole. + if tLower == ttLower { + return content[:position] + "[[" + newSegment + "]]" + content[bracketEnd:] + } + + // Case 2: body starts with target_title + "|" — display segment + // follows. Preserve everything from the pipe onward verbatim. + if strings.HasPrefix(tLower, ttLower+"|") { + displaySuffix := body[len(targetTitle):] // includes the pipe + return content[:position] + "[[" + newSegment + displaySuffix + "]]" + content[bracketEnd:] + } + + // Bracket doesn't match the expected shape — leave it alone. + // (Index drift or a stored full-body title with embedded pipe; + // the trailing replaceWikiLinks call will reconcile.) + return content +} + func replaceAll(s, old, new string) string { // Simple string replacement, not regex-based result := s diff --git a/internal/links/links_test.go b/internal/links/links_test.go new file mode 100644 index 00000000..71fe72a8 --- /dev/null +++ b/internal/links/links_test.go @@ -0,0 +1,159 @@ +package links + +import ( + "strings" + "testing" +) + +// TestRewriteBracketAt covers the position-based per-row cascade +// helper introduced for Codex round 7 finding 2. The cascade SELECT +// returns (position, target_title) per row; this helper rewrites +// exactly the bracket at `position` if its body matches target_title, +// preserving slug prefix and display suffix. +func TestRewriteBracketAt(t *testing.T) { + cases := []struct { + name string + content string + bracket string // for position lookup via strings.Index + target string + newTitle string + slug string + want string + }{ + { + name: "plain bracket no display", + content: "prose [[Old Title]] more", + bracket: "[[Old Title]]", + target: "Old Title", + newTitle: "New Title", + slug: "tasks", + want: "prose [[New Title]] more", + }, + { + name: "bracket with display alias preserved", + content: "prose [[Old Title|see this]] more", + bracket: "[[Old Title|see this]]", + target: "Old Title", + newTitle: "New Title", + slug: "tasks", + want: "prose [[New Title|see this]] more", + }, + { + name: "qualified slug body verbatim target", + content: "see [[tasks/Old Title]] here", + bracket: "[[tasks/Old Title]]", + target: "tasks/Old Title", + newTitle: "New Title", + slug: "tasks", + want: "see [[tasks/New Title]] here", + }, + { + name: "qualified slug with display", + content: "see [[tasks/Old Title|qual]] here", + bracket: "[[tasks/Old Title|qual]]", + target: "tasks/Old Title", + newTitle: "New Title", + slug: "tasks", + want: "see [[tasks/New Title|qual]] here", + }, + { + name: "mixed case body matches case-insensitively", + content: "see [[old title]] here", + bracket: "[[old title]]", + target: "old title", + newTitle: "New Title", + slug: "tasks", + want: "see [[New Title]] here", + }, + { + name: "bracket body doesn't match target — leave alone", + content: "see [[Something Else]] here", + bracket: "[[Something Else]]", + target: "Old Title", + newTitle: "New Title", + slug: "tasks", + want: "see [[Something Else]] here", + }, + { + name: "literal-pipe-title row — whole body matches target", + content: "see [[Old Title|alias]] here", + bracket: "[[Old Title|alias]]", + target: "Old Title|alias", // row stored target_title=full body + newTitle: "New Title", + slug: "tasks", + // Renaming the item titled "Old Title|alias" → whole body becomes "New Title". + want: "see [[New Title]] here", + }, + { + name: "split-key row preserves pipe-suffix", + content: "see [[Old Title|alias]] here", + bracket: "[[Old Title|alias]]", + target: "Old Title", // row stored target_title=split key, display preserved + newTitle: "New Title", + slug: "tasks", + // Renaming "Old Title" rewrites the title segment, preserves |alias. + want: "see [[New Title|alias]] here", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + pos := strings.Index(c.content, c.bracket) + if pos < 0 { + t.Fatalf("bracket %q not found in content %q", c.bracket, c.content) + } + got := RewriteBracketAt(c.content, pos, c.target, c.newTitle, c.slug) + if got != c.want { + t.Errorf("RewriteBracketAt: got %q, want %q", got, c.want) + } + }) + } +} + +// TestRewriteBracketAt_OutOfBoundsNoop covers the defensive guards: +// invalid position returns content unchanged. +func TestRewriteBracketAt_OutOfBoundsNoop(t *testing.T) { + content := "see [[Old]] here" + if got := RewriteBracketAt(content, -1, "Old", "New", ""); got != content { + t.Errorf("negative position: got %q, want unchanged", got) + } + if got := RewriteBracketAt(content, len(content)+10, "Old", "New", ""); got != content { + t.Errorf("past-EOF position: got %q, want unchanged", got) + } + if got := RewriteBracketAt(content, 0, "Old", "New", ""); got != content { + t.Errorf("position not at `[[`: got %q, want unchanged", got) + } +} + +// TestRewriteWikiTitle covers the four title-form shapes the +// item-rename cascade depends on. Case-insensitive title matching +// mirrors resolveTitleTx (and the renderer's title resolution at +// web/src/lib/utils/markdown.ts:543). Display aliases must be +// preserved verbatim — including padding and escaped chars — so +// the renderer's user-facing text doesn't silently change. +func TestRewriteWikiTitle(t *testing.T) { + cases := []struct { + name, in, old, new, slug, want string + }{ + {"plain", "see [[Old Title]] here", "Old Title", "New Title", "tasks", "see [[New Title]] here"}, + {"aliased", "see [[Old Title|click me]]", "Old Title", "New Title", "tasks", "see [[New Title|click me]]"}, + {"qualified", "see [[tasks/Old Title]]", "Old Title", "New Title", "tasks", "see [[tasks/New Title]]"}, + {"qualified aliased", "see [[tasks/Old Title|qual]]", "Old Title", "New Title", "tasks", "see [[tasks/New Title|qual]]"}, + {"mixed case", "see [[old title]] and [[OLD TITLE]]", "Old Title", "New Title", "tasks", "see [[New Title]] and [[New Title]]"}, + {"display preserves padding", "see [[Old Title| padded ]]", "Old Title", "New Title", "tasks", "see [[New Title| padded ]]"}, + {"display preserves escaped pipe", `see [[Old Title|a \| b]]`, "Old Title", "New Title", "tasks", `see [[New Title|a \| b]]`}, + {"non-matching qualified slug untouched", "see [[other/Old Title]]", "Old Title", "New Title", "tasks", "see [[other/Old Title]]"}, + {"only-matching-title untouched", "see [[Different]]", "Old Title", "New Title", "tasks", "see [[Different]]"}, + {"multiple occurrences", "[[Old Title]] then [[Old Title|alias]]", "Old Title", "New Title", "tasks", "[[New Title]] then [[New Title|alias]]"}, + {"empty oldTitle no-op", "see [[Old Title]]", "", "New Title", "tasks", "see [[Old Title]]"}, + {"same title no-op", "see [[Old Title]]", "Old Title", "Old Title", "tasks", "see [[Old Title]]"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := RewriteWikiTitle(c.in, c.old, c.new, c.slug) + if got != c.want { + t.Errorf("RewriteWikiTitle(%q, %q, %q, %q) = %q, want %q", + c.in, c.old, c.new, c.slug, got, c.want) + } + }) + } +} diff --git a/internal/store/items.go b/internal/store/items.go index 5b08b83e..38d58294 100644 --- a/internal/store/items.go +++ b/internal/store/items.go @@ -225,9 +225,34 @@ func (s *Store) tryCreateItem(id, workspaceID, collectionID, slug, ts, fields, t return fmt.Errorf("index wiki links: %w", err) } + // Phase 2a (TASK-1595): flip any pre-existing broken `[[Title]]` + // rows that have been waiting for an item with this title to + // arrive. Cheap when no broken rows match (the common case). + // Without this, sources that mention the new item by title would + // stay broken until either their content is rewritten or the + // next migration-driven backfill — both rare. + collSlug, err := s.getCollectionSlugTx(tx, collectionID) + if err != nil { + return fmt.Errorf("lookup collection slug: %w", err) + } + if err := s.resolveBrokenTitleLinks(tx, id, workspaceID, collSlug, input.Title); err != nil { + return fmt.Errorf("resolve broken titles: %w", err) + } + return tx.Commit() } +// getCollectionSlugTx reads collections.slug for a collection_id +// inside the supplied tx. Tiny helper used by the wiki-link cascade +// hooks that need the slug to build collection-qualified link keys. +func (s *Store) getCollectionSlugTx(tx *sql.Tx, collectionID string) (string, error) { + var slug string + if err := tx.QueryRow(s.q(`SELECT slug FROM collections WHERE id = ?`), collectionID).Scan(&slug); err != nil { + return "", err + } + return slug, nil +} + // isUniqueViolation checks whether an error is a unique constraint violation. // Works for both SQLite (UNIQUE constraint failed) and PostgreSQL (duplicate key). func isUniqueViolation(err error) bool { @@ -1650,15 +1675,54 @@ func (s *Store) UpdateItemWithPreCheck( return nil, fmt.Errorf("update item: %w", err) } + // Title rename — cascade to title-form backlinks. Fires whether + // content changed or not. ORDER MATTERS: cascade runs BEFORE + // replaceWikiLinks(self) so the pre-existing wl rows pointing + // at self via target_item_id=renamedItemID are still present + // when cascade does its SELECT. Codex round 6 finding 2 caught + // the original order (re-index self → cascade) silently + // breaking the self-ref cascade on title+content combined + // updates: re-indexing self first would delete the self-row + // that cascade needs to find. Function early-returns when + // oldTitle == newTitle so title-shaped-but-unchanged updates + // pay nothing. PLAN-1593 / TASK-1595. + if input.Title != nil && *input.Title != existing.Title { + // excludeSelf=true when the caller also supplied new content + // — they're authoritatively rewriting the renamed item's own + // body and the cascade should respect that. Self-refs in + // title-only renames still get cascade-rewritten so stale + // `[[Old Title]]` literals in unmodified content don't go + // broken. Mirrors documents.go::updateLinksInTx's pattern. + excludeSelf := input.Content != nil + if err := s.cascadeTitleRename(tx, id, existing.WorkspaceID, existing.Title, *input.Title, excludeSelf); err != nil { + return nil, fmt.Errorf("cascade title rename: %w", err) + } + } + // Re-index [[...]] wiki-links if the content was part of this // update (regardless of whether the new content equals the old — // the caller already paid the UPDATE cost so the delete-then-insert // is cheap and keeps the index consistent if a previous reparse - // left stale rows). When `input.Content == nil` the content wasn't - // touched, so the existing rows remain valid and we skip work. - // PLAN-1593 / TASK-1594. + // left stale rows). When `input.Content == nil` the content + // wasn't touched, so the existing rows remain valid and we skip + // work. PLAN-1593 / TASK-1594. + // + // Read items.content fresh from the DB instead of using + // *input.Content directly: cascadeTitleRename above may have + // rewritten the renamed item's own content if it contained + // self-references (`[[oldTitle]]` → `[[newTitle]]`), and we + // want the index to reflect that post-cascade state. Without + // the fresh read, this re-index would overwrite the cascade- + // rewritten rows back to whatever the user submitted, undoing + // the cascade's effect. if input.Content != nil { - if err := s.replaceWikiLinks(tx, id, existing.WorkspaceID, *input.Content); err != nil { + currentContent := *input.Content + if input.Title != nil && *input.Title != existing.Title { + if err := tx.QueryRow(s.q(`SELECT content FROM items WHERE id = ?`), id).Scan(¤tContent); err != nil { + return nil, fmt.Errorf("re-read self content after cascade: %w", err) + } + } + if err := s.replaceWikiLinks(tx, id, existing.WorkspaceID, currentContent); err != nil { return nil, fmt.Errorf("index wiki links: %w", err) } } diff --git a/internal/store/migrations/062_repopulate_wiki_links.sql b/internal/store/migrations/062_repopulate_wiki_links.sql new file mode 100644 index 00000000..20cb4254 --- /dev/null +++ b/internal/store/migrations/062_repopulate_wiki_links.sql @@ -0,0 +1,22 @@ +-- Migration 062: wipe item_wiki_links so the next BackfillWikiLinks run +-- repopulates with the Phase 2a extractor vocabulary (refs + titles). +-- PLAN-1593 / TASK-1595. +-- +-- Phase 1 (migration 061 + TASK-1594) populated item_wiki_links only for +-- `[[REF-N]]` forms — title-form links (`[[Title]]`, `[[collection/Title]]`) +-- were recognized by the parser but gated out at the store. Phase 2a lifts +-- that gate, so every existing item needs to be re-parsed under the new +-- extractor to populate title rows. The cheap way to trigger that is to +-- truncate the table here; the startup hook (Store.BackfillWikiLinks) sees +-- empty rows for every item and re-parses each one's content from scratch. +-- +-- Migrations run exactly once per DB, so this fires on the Phase-2a upgrade +-- boot and never again — subsequent boots see populated rows and short- +-- circuit normally via the EXISTS check inside BackfillWikiLinks. +-- +-- Idempotency note: if a workspace somehow had ZERO ref-form links indexed +-- pre-Phase-2a (impossible if migration 061 ran, but a clean install never +-- runs this migration's predecessor in isolation), DELETE FROM is still +-- a no-op on an empty table — safe to apply unconditionally. + +DELETE FROM item_wiki_links; diff --git a/internal/store/pgmigrations/041_repopulate_wiki_links.sql b/internal/store/pgmigrations/041_repopulate_wiki_links.sql new file mode 100644 index 00000000..e93af513 --- /dev/null +++ b/internal/store/pgmigrations/041_repopulate_wiki_links.sql @@ -0,0 +1,10 @@ +-- Postgres mirror of SQLite migration 062 (PLAN-1593 / TASK-1595). +-- See internal/store/migrations/062_repopulate_wiki_links.sql for the full +-- rationale; this file diverges only where engine syntax requires it. +-- +-- Differences from the SQLite migration: +-- * No syntax differences here — DELETE FROM is identical in both engines. +-- Kept as a separate file so the migration numbering stays in lockstep +-- with the rest of the schema. + +DELETE FROM item_wiki_links; diff --git a/internal/store/wiki_links.go b/internal/store/wiki_links.go index f3b12cd6..4692f6d1 100644 --- a/internal/store/wiki_links.go +++ b/internal/store/wiki_links.go @@ -55,8 +55,24 @@ func (s *Store) replaceWikiLinks(tx *sql.Tx, sourceItemID, workspaceID, content number int } resolved := map[refKey]sql.NullString{} + // Same caching story for title resolution — a doc mentioning + // `[[Project Goals]]` six times only hits the DB once. Key is + // the verbatim target_title (case-preserved) because + // resolveTitleTx already collapses case at SQL time via LOWER(); + // caching by lowercase would also work but slightly mismatches + // the verbatim-storage contract that the rename hook relies on. + resolvedTitles := map[string]sql.NullString{} for _, link := range extracted { + // HasDisplay (not Display != "") distinguishes "no pipe" + // from "pipe with empty display." Mirrors the client + // renderer's `displayOverride ?? title` semantics, which + // preserve "". Codex round-12 P3 (Phase 1). + displayText := sql.NullString{} + if link.HasDisplay { + displayText = sql.NullString{String: link.Display, Valid: true} + } + switch link.Kind { case links.WikiLinkKindRef: prefix, number, ok := splitRef(link.Ref) @@ -71,14 +87,85 @@ func (s *Store) replaceWikiLinks(tx *sql.Tx, sourceItemID, workspaceID, content targetID = resolveRefTx(tx, s, workspaceID, prefix, number) resolved[key] = targetID } - // HasDisplay (not Display != "") distinguishes "no - // pipe" from "pipe with empty display." Mirrors the - // client renderer's `displayOverride ?? title` - // semantics, which preserve "". Codex round-12 P3. - displayText := sql.NullString{} - if link.HasDisplay { - displayText = sql.NullString{String: link.Display, Valid: true} + if targetID.Valid { + // Ref resolved — store as a ref-kind row. + if _, err := tx.Exec(s.q(` + INSERT INTO item_wiki_links ( + source_item_id, target_kind, target_workspace_id, + target_item_id, target_ref, target_title, + display_text, position + ) VALUES (?, ?, NULL, ?, ?, NULL, ?, ?) + `), sourceItemID, string(links.WikiLinkKindRef), + targetID, link.Ref, displayText, link.Position); err != nil { + return fmt.Errorf("insert wiki link: %w", err) + } + continue } + // Ref didn't resolve — try title fallback. Mirrors the + // renderer's "If the ref doesn't resolve we FALL THROUGH + // to the legacy title path" at web/src/lib/utils/markdown.ts:513. + // Order matches the renderer: + // (a) If HasDisplay, try FULL body (RawKey+"|"+Display) + // as a title — covers literal-pipe titles like + // "ISO-9001|Spec" matching `[[ISO-9001|Spec]]`. + // Codex round 7 P1. + // (b) Try the raw key string as a title — covers + // `[[ISO-9001]]` matching an item titled "ISO-9001". + // Codex round 6 P1. + // + // Use link.RawKey (untrimmed unescaped key) NOT link.Ref + // (canonical trimmed) for title fallback so an item + // literally titled `" TASK-5 "` resolves the same way + // the renderer does. Codex round 10 P2. + rawKey := link.RawKey + if rawKey == "" { + rawKey = link.Ref // defensive — shouldn't happen for ref kind + } + titleCandidates := []string{rawKey} + storedTitleForFallback := rawKey + storeDisplayForFallback := displayText + if link.HasDisplay { + fullBody := rawKey + "|" + link.Display + titleCandidates = []string{fullBody, rawKey} + } + var titleHit sql.NullString + for i, candidate := range titleCandidates { + cached, ok := resolvedTitles[candidate] + if !ok { + cached = resolveTitleTx(tx, s, workspaceID, candidate) + resolvedTitles[candidate] = cached + } + if cached.Valid { + titleHit = cached + storedTitleForFallback = candidate + // If stage (a) matched (i==0 with pipe), the + // display segment was absorbed into the title. + if i == 0 && link.HasDisplay { + storeDisplayForFallback = sql.NullString{} + } + break + } + } + if titleHit.Valid { + if _, err := tx.Exec(s.q(` + INSERT INTO item_wiki_links ( + source_item_id, target_kind, target_workspace_id, + target_item_id, target_ref, target_title, + display_text, position + ) VALUES (?, ?, NULL, ?, NULL, ?, ?, ?) + `), sourceItemID, string(links.WikiLinkKindTitle), + titleHit, storedTitleForFallback, storeDisplayForFallback, link.Position); err != nil { + return fmt.Errorf("insert wiki link (ref→title fallback): %w", err) + } + continue + } + // Both ref and title missed — store as broken ref-kind + // row (the original interpretation). A future ref-item + // creation will still find this row via the ref-shaped + // target_ref; a future title-item creation won't + // auto-retarget (target_kind='ref' so + // resolveBrokenTitleLinks misses it), which is a known + // limitation v3 can revisit. if _, err := tx.Exec(s.q(` INSERT INTO item_wiki_links ( source_item_id, target_kind, target_workspace_id, @@ -89,11 +176,94 @@ func (s *Store) replaceWikiLinks(tx *sql.Tx, sourceItemID, workspaceID, content targetID, link.Ref, displayText, link.Position); err != nil { return fmt.Errorf("insert wiki link: %w", err) } + + case links.WikiLinkKindTitle: + // Phase 2a (TASK-1595). Target_title is stored VERBATIM + // in WHATEVER FORM RESOLVED so the rename cascade can + // reconstruct the literal bracket string in source + // content. Resolution mirrors the renderer's order at + // web/src/lib/utils/markdown.ts:516-558: + // + // (a) If a pipe was present (HasDisplay), try the + // FULL body (Title + "|" + Display) as a title. + // Handles legacy items whose title literally + // contains a `|` — `[[A|B]]` rendered as a link + // to the item titled "A|B" before falling back + // to the split interpretation. Codex round 3 P1 + // (this PR) caught the parity gap. + // (b) The split key (Title alone) as a title. + // + // Stage (a)'s match stores target_title=fullBody and + // drops the display override (the "display" segment + // was actually part of the title). Stage (b) is the + // common case: target_title=Title, display preserved. + // + // Within each stage, resolveTitleTx applies the + // renderer's two-step lookup (full-key literal first, + // `/`-split fallback on miss) for the SAME case- + // insensitive correctness reasons. + candidates := []string{link.Title} + storeDisplay := displayText + fullBody := link.Title // == split key when !HasDisplay + if link.HasDisplay { + // Stage (a) first, then stage (b). + fullBody = link.Title + "|" + link.Display + candidates = []string{fullBody, link.Title} + } + var resolved sql.NullString + storedTitle := link.Title // default if nothing resolves + for i, candidate := range candidates { + cached, ok := resolvedTitles[candidate] + if !ok { + cached = resolveTitleTx(tx, s, workspaceID, candidate) + resolvedTitles[candidate] = cached + } + if cached.Valid { + resolved = cached + storedTitle = candidate + // If stage (a) matched (i==0 with a pipe in + // the candidate), the "display" was consumed + // into the title — don't double-store it. + if i == 0 && link.HasDisplay { + storeDisplay = sql.NullString{} + } + break + } + } + // Codex round 4: when nothing resolved AND there was a + // pipe, key the broken row on the FULL BODY rather than + // the split key. The renderer's preferred interpretation + // for `[[A|B]]` is "title A|B" (markdown.ts:516); if an + // item literally titled "A|B" arrives later, + // resolveBrokenTitleLinks needs target_title="A|B" to + // find this row. Storing the split key would orphan it + // forever (no path retargets "A" → "A|B"). The + // remaining asymmetry — a broken row keyed on full body + // won't pick up a future split-fallback resolution to a + // new item titled "A" — is documented as a v3-promotable + // limitation. The full-body path is the renderer's + // preferred interpretation, so prioritizing it is the + // right tradeoff in the rare case both paths apply. + if !resolved.Valid && link.HasDisplay { + storedTitle = fullBody + storeDisplay = sql.NullString{} + } + if _, err := tx.Exec(s.q(` + INSERT INTO item_wiki_links ( + source_item_id, target_kind, target_workspace_id, + target_item_id, target_ref, target_title, + display_text, position + ) VALUES (?, ?, NULL, ?, NULL, ?, ?, ?) + `), sourceItemID, string(links.WikiLinkKindTitle), + resolved, storedTitle, storeDisplay, link.Position); err != nil { + return fmt.Errorf("insert wiki link (title): %w", err) + } + default: - // Phase 1 skips title and workspace_ref kinds. The - // parser's gate should already prevent them from - // arriving here; this default branch is a safety - // net for the day Phase 2 lifts the gate. + // Phase 2a still skips workspace_ref kinds. The parser's + // gate prevents them from arriving here; this default + // branch is a safety net for the day TASK-1597 lifts + // the gate. continue } } @@ -141,6 +311,400 @@ func resolveRefTx(tx *sql.Tx, s *Store, workspaceID, prefix string, number int) return sql.NullString{String: id, Valid: true} } +// resolveTitleTx mirrors the renderer's title-resolution logic +// (web/src/lib/utils/markdown.ts:541–558) inside the parse-time +// transaction. Two-stage lookup: +// +// 1. Exact case-insensitive match on items.title against the full +// verbatim title (e.g. "docs/Setup" matches an item literally +// titled "docs/Setup"). +// 2. On miss, if the title contains `/`, split into +// (collection_slug, remainder) and try the collection-qualified +// form (e.g. "docs/Setup" → collection slug "docs", title +// "Setup"). +// +// The order matters for items whose titles legitimately contain `/` +// — they must match in stage 1 before stage 2's split-then-lookup +// could ever take a different interpretation. Codex caught this on +// the planning round (finding #3). +// +// Unresolved titles return NULL so the row persists as a broken +// title-link, matching the same broken-row semantics resolveRefTx +// uses for ref-form lookups. Broken titles are intentional — a +// future broken-links report uses them and the rename hook can +// flip them to resolved as items appear / get renamed. +// +// LOWER() is portable across SQLite and Postgres for ASCII case +// folding. For non-ASCII titles both engines fall back to bytewise +// behavior — matching the renderer's `.toLowerCase()`, which is +// also locale-naive (JS default-locale toLowerCase on the V8 +// runtime in production targets is effectively ASCII for our +// data). If a future requirement demands Unicode-aware folding, +// it lands as a single helper change here + a matching renderer +// fix; the cross-engine baseline doesn't pretend to do more than +// it does. +func resolveTitleTx(tx *sql.Tx, s *Store, workspaceID, title string) sql.NullString { + // Stage 1: full-key exact match. LIMIT 1 because the renderer + // uses Array.find() (first match wins) — we mirror that + // non-determinism rather than introducing our own ordering. + var id string + err := tx.QueryRow(s.q(` + SELECT id FROM items + WHERE workspace_id = ? + AND deleted_at IS NULL + AND LOWER(title) = LOWER(?) + LIMIT 1 + `), workspaceID, title).Scan(&id) + if err == nil { + return sql.NullString{String: id, Valid: true} + } + if err != sql.ErrNoRows { + // Real DB error — return NULL so the row persists as + // unresolved (same conservative posture as resolveRefTx). + return sql.NullString{} + } + + // Stage 2: collection-qualified fallback. Only applies when the + // title contains a `/`. Split on the FIRST `/` because an item's + // title can legitimately contain additional slashes after the + // collection delimiter (e.g. "docs/api/auth-flow"). The renderer + // at markdown.ts:548 uses `key.split('/')` then `rest.join('/')` + // — equivalent to "split once, keep the rest verbatim." + slash := strings.IndexByte(title, '/') + if slash <= 0 || slash >= len(title)-1 { + // No `/`, or empty side — no qualified form to try. + return sql.NullString{} + } + collSlug := title[:slash] + titleRest := title[slash+1:] + err = tx.QueryRow(s.q(` + SELECT i.id FROM items i + JOIN collections c ON c.id = i.collection_id + WHERE i.workspace_id = ? + AND c.slug = ? + AND i.deleted_at IS NULL + AND LOWER(i.title) = LOWER(?) + LIMIT 1 + `), workspaceID, collSlug, titleRest).Scan(&id) + if err != nil { + return sql.NullString{} + } + return sql.NullString{String: id, Valid: true} +} + +// cascadeTitleRename keeps title-form backlinks consistent when an +// item's title changes. Two effects to maintain inside the rename tx: +// +// 1. Sources that ALREADY point at the renamed item via a title-form +// link have `[[oldTitle]]` (or `[[/oldTitle]]`) literal in +// their content. The renderer would no longer resolve those after +// the rename, breaking the user's click target. Rewrite each +// source's content via links.ReplaceTitle (matches the document +// rename behavior — see documents.go::updateLinksInTx), re-stamp +// updated_at + content_flushed_at, bump the workspace seq, and +// re-run replaceWikiLinks so the source's own index rows refresh +// with the new literal AND the new target_item_id resolution. +// +// 2. Sources that ALREADY contain `[[newTitle]]` (or +// `[[/newTitle]]`) in their content but whose index rows +// were stored as broken (target_item_id IS NULL, because at parse +// time no item had that title) need to flip to RESOLVED. No +// content change required — only an UPDATE of item_wiki_links. +// +// Runs inside the rename tx so a single failure rolls back the whole +// rename: either every dependent is consistent with the new title or +// the rename never happened. Self-references (the renamed item links +// to itself by its own title) are filtered out — the renamed item's +// own content gets updated by the surrounding UPDATE statement, not +// by this cascade. +// +// PLAN-1593 / TASK-1595. +// +// `excludeSelf` controls whether the renamed item's own body is part +// of the cascade: +// +// - false (title-only rename) — INCLUDE self. The user didn't +// touch content, so any pre-existing self-ref `[[Old Title]]` +// would go stale without rewrite. Cascade refreshes it. +// - true (title + content rename) — EXCLUDE self. The user is +// actively rewriting content in the same call; their submission +// is authoritative. Mirrors documents.go::updateLinksInTx's +// pattern of leaving the renamed entity's own content alone. +// If their new content contains `[[Old Title]]`, the trailing +// replaceWikiLinks correctly stores it as broken — same as the +// renderer would render it. +func (s *Store) cascadeTitleRename(tx *sql.Tx, renamedItemID, workspaceID, oldTitle, newTitle string, excludeSelf bool) error { + if oldTitle == newTitle { + return nil + } + + // Look up the renamed item's collection_slug so the qualified-form + // matches (`[[/oldTitle]]`) get the same cascade treatment. + // A collection-move during a title rename is impossible in the + // API (UpdateItem doesn't move collections), so reading the slug + // from the current row is safe — it's the slug both before and + // after the rename. + var collSlug string + if err := tx.QueryRow(s.q(` + SELECT c.slug FROM items i + JOIN collections c ON c.id = i.collection_id + WHERE i.id = ? + `), renamedItemID).Scan(&collSlug); err != nil { + return fmt.Errorf("cascade rename: lookup collection slug: %w", err) + } + + // (1) Cascade content rewrites — POSITION-BASED. SELECT each + // individual wl row (not DISTINCT sources) along with its + // position + target_title. Each row tells us EXACTLY which + // bracket in the source content corresponds to a link to the + // renamed item; we rewrite only those brackets, leaving any + // unrelated literal-pipe `[[OldTitle|alias]]` brackets that + // happened to mention items B/C with overlapping titles alone. + // + // Codex round 7 finding 2 caught the prior broad-regex + // approach corrupting unrelated brackets: items A "Old Title" + // and B "Old Title|alias" both referenced from one source, A + // renamed, RewriteWikiTitle's pattern (?i:Old Title) matched + // both A's `[[Old Title]]` row AND B's `[[Old Title|alias]]` + // bracket (even though B's wl row points at B, not A). The + // position-based approach restricts the rewrite to exactly the + // brackets the cascade SELECT actually returned. + // + // target_workspace_id IS NULL filter excludes Phase-2b cross- + // workspace rows (TASK-1597 owns those). Self-references + // INCLUDED so a renamed item's own body stays consistent + // (Codex round 5 finding 2); GetBacklinks filters self-links + // at query time so the panel behavior is unchanged. + // + // ORDER BY source_id, position DESC: descending position so + // rewrites at later byte offsets don't shift the offsets of + // earlier rows in the same source. + selectQuery := ` + SELECT s.id, s.content, s.workspace_id, wl.position, wl.target_title + FROM item_wiki_links wl + JOIN items s ON s.id = wl.source_item_id + WHERE wl.target_kind = 'title' + AND wl.target_workspace_id IS NULL + AND wl.target_item_id = ? + AND s.deleted_at IS NULL` + queryArgs := []interface{}{renamedItemID} + if excludeSelf { + selectQuery += " AND s.id != ?" + queryArgs = append(queryArgs, renamedItemID) + } + selectQuery += " ORDER BY s.id, wl.position DESC" + rows, err := tx.Query(s.q(selectQuery), queryArgs...) + if err != nil { + return fmt.Errorf("cascade rename: scan sources: %w", err) + } + type rowInfo struct { + position int + targetTitle string + } + type sourceWork struct { + id, content, workspaceID string + rows []rowInfo + } + var works []sourceWork + cursor := -1 + for rows.Next() { + var ( + id, content, workspaceID string + position int + targetTitle string + ) + if err := rows.Scan(&id, &content, &workspaceID, &position, &targetTitle); err != nil { + rows.Close() + return fmt.Errorf("cascade rename: scan source row: %w", err) + } + if cursor < 0 || works[cursor].id != id { + works = append(works, sourceWork{id: id, content: content, workspaceID: workspaceID}) + cursor = len(works) - 1 + } + works[cursor].rows = append(works[cursor].rows, rowInfo{position: position, targetTitle: targetTitle}) + } + if err := rows.Err(); err != nil { + rows.Close() + return fmt.Errorf("cascade rename: iterate sources: %w", err) + } + rows.Close() + + // Per source: walk its rows (positions descending), rewrite each + // bracket via links.RewriteBracketAt, then UPDATE the source's + // content + re-parse to refresh the index. + ts := now() + for _, work := range works { + newContent := work.content + mutated := false + for _, r := range work.rows { + rewritten := links.RewriteBracketAt(newContent, r.position, r.targetTitle, newTitle, collSlug) + if rewritten != newContent { + mutated = true + newContent = rewritten + } + } + if !mutated { + // No bracket matched the expected shape at the recorded + // positions — possible if a previous content edit shifted + // the offsets in a way replaceWikiLinks didn't catch, or + // the bracket uses title-segment escape forms the rewriter + // doesn't unescape (documented limitation, parallel to + // RewriteWikiTitle's caveat). Re-parse on existing content + // so the index converges; the row will flip to broken if + // no clickable match remains, matching the renderer's + // behavior on a body that would no longer render as a link. + if err := s.replaceWikiLinks(tx, work.id, work.workspaceID, work.content); err != nil { + return fmt.Errorf("cascade rename: reparse %s: %w", work.id, err) + } + continue + } + if _, err := tx.Exec(s.q(` + UPDATE items + SET content = ?, + updated_at = ?, + content_flushed_at = ?, + seq = `+nextWorkspaceSeqSubquery+` + WHERE id = ? + `), newContent, ts, ts, work.workspaceID, work.id); err != nil { + return fmt.Errorf("cascade rename: update source %s: %w", work.id, err) + } + if err := s.replaceWikiLinks(tx, work.id, work.workspaceID, newContent); err != nil { + return fmt.Errorf("cascade rename: reparse %s: %w", work.id, err) + } + } + + // (2) Flip newly-resolvable broken rows under the NEW title. + return s.resolveBrokenTitleLinks(tx, renamedItemID, workspaceID, collSlug, newTitle) +} + +// resolveBrokenTitleLinks flips item_wiki_links rows in the workspace +// that have target_kind='title' and a target_title matching the +// given (collSlug, title) — case-insensitive — to point at the +// supplied itemID. Called from two places: +// +// - cascadeTitleRename after a title rename, to pick up any +// pre-existing `[[newTitle]]` sources that were stored as broken +// OR as qualified-fallback hits that should now resolve to a +// literal-title match. +// - tryCreateItem after a new item lands, so any pre-existing +// `[[Title]]` sources resolve immediately rather than waiting +// for a backfill run or a content rewrite. +// +// Three UPDATEs cover three distinct cases: +// +// (1) Plain literal flip — target_title=newTitle, target_item_id IS NULL. +// Just resolves previously-broken plain references. +// (2) Qualified literal flip — target_title=collSlug/newTitle, +// target_item_id IS NULL. Resolves previously-broken qualified +// references (no item with this title-in-collection existed). +// (3) Literal-arrival retarget (only when newTitle contains `/`) — +// target_title=newTitle, target_item_id non-NULL and not us. +// Handles the arrival-order case Codex round 2 caught: if a +// row was previously resolved to a stage-2 qualified-fallback +// target and now a literal-title match exists, the renderer's +// stage-1 always wins, so the row flips to us. +// +// Why (3) is gated on title containing `/`: only `[[/Title]]` +// rows could have been resolved via stage-2 fallback (the qualified +// form REQUIRES a `/`). A row with target_title="Foo" (no slash) was +// resolved via stage 1 — if a second item titled "Foo" arrives, the +// renderer's Array.find() is order-dependent (we can't predict which +// the UI shows), so we leave the row pointing at the original +// resolution rather than churning. Codex round 3 P2 caught the +// previous broader UPDATE silently stealing such rows. +func (s *Store) resolveBrokenTitleLinks(tx *sql.Tx, itemID, workspaceID, collSlug, title string) error { + plainTitleNorm := strings.ToLower(title) + qualifiedTitleNorm := strings.ToLower(collSlug + "/" + title) + + // "Broken-in-practice" predicate: a row is eligible for flip + // when target_item_id IS NULL (never resolved) OR its current + // target points at a soft-deleted (or otherwise gone) item. + // The renderer hides deleted-target links from clicks, so the + // index must follow. Codex round 8 P2 caught the prior NULL-only + // constraint missing the soft-delete-then-create-same-title + // case. Subquery against items.deleted_at via NOT EXISTS so the + // row qualifies when its target is missing entirely (defensive + // against hard-delete; FK only cascades on source, not target). + brokenPredicate := `( + target_item_id IS NULL + OR NOT EXISTS ( + SELECT 1 FROM items t + WHERE t.id = item_wiki_links.target_item_id + AND t.deleted_at IS NULL + ) + )` + + // (1) Plain literal flip — broken-in-practice rows. + if _, err := tx.Exec(s.q(` + UPDATE item_wiki_links + SET target_item_id = ? + WHERE target_kind = 'title' + AND target_workspace_id IS NULL + AND `+brokenPredicate+` + AND LOWER(target_title) = ? + AND source_item_id IN ( + SELECT id FROM items WHERE workspace_id = ? AND deleted_at IS NULL + ) + `), itemID, plainTitleNorm, workspaceID); err != nil { + return fmt.Errorf("resolve broken plain titles: %w", err) + } + + // (2) Qualified literal flip — broken-in-practice rows. + if _, err := tx.Exec(s.q(` + UPDATE item_wiki_links + SET target_item_id = ? + WHERE target_kind = 'title' + AND target_workspace_id IS NULL + AND `+brokenPredicate+` + AND LOWER(target_title) = ? + AND source_item_id IN ( + SELECT id FROM items WHERE workspace_id = ? AND deleted_at IS NULL + ) + `), itemID, qualifiedTitleNorm, workspaceID); err != nil { + return fmt.Errorf("resolve broken qualified titles: %w", err) + } + + // (3) Literal-arrival retarget. Only meaningful when our title + // contains a `/` — then a row with this exact target_title might + // have been resolved via stage-2 qualified fallback to a + // different item, and our arrival makes the renderer prefer us + // via stage 1. + // + // Codex round 5 finding 1: the broad UPDATE could steal rows + // resolved via stage-1 literal match if a SECOND item with the + // same slash-containing title arrives. Distinguish stage-1 + // (literal) from stage-2 (qualified-fallback) resolution by + // checking the CURRENT target's title: + // - If target's title equals our literal title (case- + // insensitive), the row resolved via stage 1 to a "twin" + // item — don't steal it. + // - Otherwise the row resolved via stage 2 (target's title is + // just the trailing segment, not the whole `slug/title`) — + // stage 1 now wins, flip the row to us. + if strings.Contains(title, "/") { + if _, err := tx.Exec(s.q(` + UPDATE item_wiki_links + SET target_item_id = ? + WHERE target_kind = 'title' + AND target_workspace_id IS NULL + AND target_item_id IS NOT NULL + AND target_item_id != ? + AND LOWER(target_title) = ? + AND source_item_id IN ( + SELECT id FROM items WHERE workspace_id = ? AND deleted_at IS NULL + ) + AND EXISTS ( + SELECT 1 FROM items t + WHERE t.id = item_wiki_links.target_item_id + AND LOWER(t.title) != ? + ) + `), itemID, itemID, plainTitleNorm, workspaceID, plainTitleNorm); err != nil { + return fmt.Errorf("retarget qualified-fallback to literal: %w", err) + } + } + return nil +} + // splitRef parses "TASK-5" into ("TASK", 5). The trailing - // is required; everything before the last '-' is the prefix. // Returns ok=false on shapes ExtractWikiLinks shouldn't produce diff --git a/internal/store/wiki_links_test.go b/internal/store/wiki_links_test.go index 2f12a77a..9bf918ec 100644 --- a/internal/store/wiki_links_test.go +++ b/internal/store/wiki_links_test.go @@ -547,6 +547,699 @@ func refOf(item *models.Item) string { return item.CollectionPrefix + "-" + itoa(num) } +// -- Phase 2a (TASK-1595): title-form backlinks -- + +// TestWikiLinks_TitleFormIndexed exercises the headline Phase 2a path: +// `[[Title]]` in a source's body produces a backlink for the target +// when the target's title matches (case-insensitive). +func TestWikiLinks_TitleFormIndexed(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + target := createTestItem(t, s, ws.ID, col.ID, "Project Goals", "") + source := createTestItem(t, s, ws.ID, col.ID, "Source", + "Please see [[Project Goals]] for context.") + + got, err := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if err != nil { + t.Fatalf("GetBacklinks: %v", err) + } + if len(got) != 1 { + t.Fatalf("expected 1 backlink, got %d: %+v", len(got), got) + } + if got[0].SourceItemID != source.ID { + t.Errorf("SourceItemID: got %q want %q", got[0].SourceItemID, source.ID) + } +} + +// TestWikiLinks_TitleFormCaseInsensitive — resolveTitleTx uses LOWER() +// to mirror the renderer's `.toLowerCase()` comparison. A source that +// writes `[[project goals]]` must still resolve to an item titled +// "Project Goals". +func TestWikiLinks_TitleFormCaseInsensitive(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + target := createTestItem(t, s, ws.ID, col.ID, "Project Goals", "") + createTestItem(t, s, ws.ID, col.ID, "Source", + "See [[project goals]] for the plan.") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("expected 1 backlink (case-insensitive resolution), got %d", len(got)) + } +} + +// TestWikiLinks_BrokenTitlePersistedThenResolved covers two beats: +// +// - A `[[Title]]` whose target doesn't exist yet persists with +// target_item_id=NULL so the row is queryable later. +// - When the missing item is CREATED, resolveBrokenTitleLinks flips +// the row to point at it — backlinks resolve on next query without +// waiting for a content rewrite or a backfill run. +func TestWikiLinks_BrokenTitlePersistedThenResolved(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + // Source first, target later. At this point [[Future Title]] + // doesn't resolve. + source := createTestItem(t, s, ws.ID, col.ID, "Source", + "Anchor for [[Future Title]] which doesn't exist yet.") + _ = source + + // Now create the target. The create hook should flip the broken + // row to point at the new item. + target := createTestItem(t, s, ws.ID, col.ID, "Future Title", "") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("expected source to resolve to target after creation, got %d backlinks", len(got)) + } +} + +// TestWikiLinks_TitleRenameCascadesContentAndBacklinks is the headline +// Phase 2a behavior: rename an item and (a) sources that referenced +// the old title get their bodies rewritten in-band, (b) their +// index rows refresh, (c) "who mentions me?" still finds them under +// the new title. +func TestWikiLinks_TitleRenameCascadesContentAndBacklinks(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + target := createTestItem(t, s, ws.ID, col.ID, "Old Title", "") + source := createTestItem(t, s, ws.ID, col.ID, "Source", + "Please see [[Old Title]] and again [[Old Title]] here.") + + // Baseline: target has 1 backlink (multiplicity stored but the + // query returns one row per source by snippet position — we get + // 2 rows actually since multiplicity is preserved per PLAN-1593; + // either way both should point at the same source). + bls, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(bls) != 2 { + t.Fatalf("baseline: expected 2 rows (multiplicity preserved), got %d", len(bls)) + } + + // Rename the target. Cascade should rewrite the source's content + // and refresh its index rows. The renamed target still has its + // backlinks queryable. + newTitle := "New Title" + if _, err := s.UpdateItem(target.ID, models.ItemUpdate{Title: &newTitle}); err != nil { + t.Fatalf("UpdateItem rename: %v", err) + } + + // Source's content should have been rewritten in-band. + updatedSource, err := s.GetItem(source.ID) + if err != nil { + t.Fatalf("GetItem source: %v", err) + } + if strings.Contains(updatedSource.Content, "[[Old Title]]") { + t.Errorf("expected [[Old Title]] to be rewritten, source content: %q", updatedSource.Content) + } + if !strings.Contains(updatedSource.Content, "[[New Title]]") { + t.Errorf("expected [[New Title]] in rewritten content, got %q", updatedSource.Content) + } + + // Target's backlinks still find the source via the new title. + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 2 { + t.Errorf("post-rename: expected 2 backlinks, got %d", len(got)) + } + for _, bl := range got { + if bl.SourceItemID != source.ID { + t.Errorf("backlink source should still be %q, got %q", source.ID, bl.SourceItemID) + } + } +} + +// TestWikiLinks_TitleRenameCascadesAliasedForms regresses Codex round 1 +// finding: the cascade must preserve display aliases when rewriting +// title-form links. `[[Old Title|alias]]` AND `[[old title]]` (mixed +// case) both index via target_item_id, so without alias-aware / +// case-insensitive rewrite, the trailing re-parse drops them. +func TestWikiLinks_TitleRenameCascadesAliasedForms(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + target := createTestItem(t, s, ws.ID, col.ID, "Old Title", "") + source := createTestItem(t, s, ws.ID, col.ID, "Source", + "Plain [[Old Title]], aliased [[Old Title|see this]], "+ + "mixed case [[old title]], and qualified [[tasks/Old Title|qual]] all here.") + + // Baseline: 4 backlink rows (multiplicity preserved). + bls, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(bls) != 4 { + t.Fatalf("baseline: expected 4 rows (multiplicity), got %d", len(bls)) + } + + // Rename and verify all 4 forms got rewritten in source body AND + // stayed resolved. + newTitle := "New Title" + if _, err := s.UpdateItem(target.ID, models.ItemUpdate{Title: &newTitle}); err != nil { + t.Fatalf("UpdateItem rename: %v", err) + } + updated, _ := s.GetItem(source.ID) + for _, oldShape := range []string{ + "[[Old Title]]", + "[[Old Title|see this]]", + "[[old title]]", + "[[tasks/Old Title|qual]]", + } { + if strings.Contains(updated.Content, oldShape) { + t.Errorf("expected %q to be rewritten, content: %q", oldShape, updated.Content) + } + } + for _, newShape := range []string{ + "[[New Title]]", + "[[New Title|see this]]", + "[[tasks/New Title|qual]]", + } { + if !strings.Contains(updated.Content, newShape) { + t.Errorf("expected %q in rewritten content, got %q", newShape, updated.Content) + } + } + + // All 4 rows still resolve after rename — none flipped to broken. + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 4 { + t.Errorf("post-rename: expected 4 resolved backlinks, got %d", len(got)) + } +} + +// TestWikiLinks_CollectionQualifiedTitleResolved covers the qualified +// `[[collection_slug/Title]]` form. Stage 1 (full-key match) misses +// because no item is literally titled "tasks/Setup"; stage 2 (split +// fallback) finds the item titled "Setup" in collection "tasks". +func TestWikiLinks_CollectionQualifiedTitleResolved(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") // slug = "tasks" + + target := createTestItem(t, s, ws.ID, col.ID, "Setup", "") + createTestItem(t, s, ws.ID, col.ID, "Source", + "See [[tasks/Setup]] for the install steps.") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("collection-qualified link should resolve, got %d backlinks", len(got)) + } +} + +// TestWikiLinks_FullKeyTitleBeatsQualifiedSplit regresses Codex +// finding #3 from the planning round: an item literally titled +// "tasks/Setup" must win stage 1 BEFORE the resolver splits on `/` +// and looks up by collection slug. If we split first, the wrong item +// would resolve. +func TestWikiLinks_FullKeyTitleBeatsQualifiedSplit(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") // slug = "tasks" + + // Two items: one literally titled "tasks/Setup" (the trick + // case), one titled "Setup" in the tasks collection (the + // fallback-match case). The link `[[tasks/Setup]]` must + // resolve to the FIRST — the literal title beats the qualified- + // split interpretation per renderer order. + literalTitle := createTestItem(t, s, ws.ID, col.ID, "tasks/Setup", "") + fallback := createTestItem(t, s, ws.ID, col.ID, "Setup", "") + createTestItem(t, s, ws.ID, col.ID, "Source", + "Link: [[tasks/Setup]] here.") + + literalBls, _ := s.GetBacklinks(literalTitle.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(literalBls) != 1 { + t.Errorf("literal-title item should win stage 1, got %d backlinks", len(literalBls)) + } + fallbackBls, _ := s.GetBacklinks(fallback.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(fallbackBls) != 0 { + t.Errorf("fallback item should NOT resolve when stage 1 hits, got %d backlinks", len(fallbackBls)) + } +} + +// TestWikiLinks_BrokenPipeInBodyRetargetsOnLiteralArrival regresses +// Codex round 4: a source body `[[A|B]]` written BEFORE any matching +// item exists must store target_title="A|B" (the full body), not the +// split key "A". Otherwise resolveBrokenTitleLinks for a later- +// arriving item titled "A|B" can't find the row — the index goes +// stale while the renderer's full-body interpretation would resolve +// the link correctly. +func TestWikiLinks_BrokenPipeInBodyRetargetsOnLiteralArrival(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + // Source first; nothing matches. + createTestItem(t, s, ws.ID, col.ID, "Source", "Future: [[A|B]] arrives later.") + + // Now create an item literally titled "A|B". + target := createTestItem(t, s, ws.ID, col.ID, "A|B", "") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("expected broken pipe-in-body row to retarget on literal arrival, got %d backlinks", len(got)) + } +} + +// TestWikiLinks_LiteralPipeInTitleResolves regresses Codex round 3 P1: +// an item literally titled "A|B" (pipe in title) must match +// `[[A|B]]` in source content — the renderer's L516-525 tries the +// full body as a title BEFORE splitting on the pipe. Without this, +// the index would split-then-look-up "A" and either resolve to a +// different item or fail to resolve at all, leaving a backlink the +// UI shows but the index can't surface. +func TestWikiLinks_LiteralPipeInTitleResolves(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + // Item literally titled with a pipe character. Pad allows this. + target := createTestItem(t, s, ws.ID, col.ID, "A|B", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "See [[A|B]] for the doc.") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("expected literal-pipe title to resolve, got %d backlinks", len(got)) + } +} + +// TestWikiLinks_LiteralPipeInTitleFallsThroughToSplit covers the +// complement of the above: when no item is literally titled "A|B" +// but an item titled "A" exists, the split interpretation still +// kicks in (title="A", display="B"). The candidate-order in +// replaceWikiLinks tries full body first, then falls through. +func TestWikiLinks_LiteralPipeInTitleFallsThroughToSplit(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + target := createTestItem(t, s, ws.ID, col.ID, "A", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "See [[A|B]] for the doc.") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("split fallback should resolve `[[A|B]]` to item titled 'A', got %d backlinks", len(got)) + } +} + +// TestWikiLinks_SecondItemSameTitleDoesNotStealBacklinks regresses +// Codex round 3 P2: dropping the IS NULL constraint on the stage-1 +// UPDATE made the broken-row flip too aggressive. A row already +// resolved to via stage-1 literal match must stay there +// when a SECOND item titled "Foo-v1" is created — titles aren't +// unique, the renderer's first-match is array-order-dependent, and +// the index churning would silently move backlinks without warning. +// Stage 3 (literal-arrival retarget) is gated to titles containing +// `/` for exactly this reason. +func TestWikiLinks_SecondItemSameTitleDoesNotStealBacklinks(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + original := createTestItem(t, s, ws.ID, col.ID, "Foo", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "Mentions [[Foo]] here.") + bls, _ := s.GetBacklinks(original.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(bls) != 1 { + t.Fatalf("baseline: expected 1 backlink, got %d", len(bls)) + } + + // Create a second item with the same title. + duplicate := createTestItem(t, s, ws.ID, col.ID, "Foo", "") + + // Original should retain its backlink — no theft. + originalBls, _ := s.GetBacklinks(original.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(originalBls) != 1 { + t.Errorf("original `Foo` lost its backlink to duplicate creation: got %d", len(originalBls)) + } + // Duplicate should NOT have inherited any backlinks. + dupBls, _ := s.GetBacklinks(duplicate.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(dupBls) != 0 { + t.Errorf("duplicate stole backlinks, got %d", len(dupBls)) + } +} + +// TestWikiLinks_LiteralTitleArrivalRetargetsQualifiedFallback regresses +// Codex round 2 P2: a row resolved via qualified-fallback (stage 2) +// must flip to point at a later-arriving literal-title match (stage 1 +// always wins per renderer order at markdown.ts:541). +// +// Step 1: source writes `[[tasks/Setup]]`. No literal "tasks/Setup" +// item exists; item "Setup" exists in collection "tasks". +// Row resolves via qualified fallback to . +// Step 2: item literally titled "tasks/Setup" is created. +// Row MUST retarget to — the literal match. +// +// Without the stage-1 NULL-constraint drop, the index would stay +// stale until the source's content was rewritten. +func TestWikiLinks_LiteralTitleArrivalRetargetsQualifiedFallback(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") // slug = "tasks" + + // Step 1: fallback resolution. + fallback := createTestItem(t, s, ws.ID, col.ID, "Setup", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "See [[tasks/Setup]].") + fallbackBls, _ := s.GetBacklinks(fallback.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(fallbackBls) != 1 { + t.Fatalf("step 1: fallback should resolve, got %d backlinks", len(fallbackBls)) + } + + // Step 2: literal arrival should win stage 1 and steal the row. + literal := createTestItem(t, s, ws.ID, col.ID, "tasks/Setup", "") + + got, _ := s.GetBacklinks(literal.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("literal arrival should retarget qualified-fallback row, got %d backlinks", len(got)) + } + fallbackBls, _ = s.GetBacklinks(fallback.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(fallbackBls) != 0 { + t.Errorf("fallback should lose the row after literal arrival, got %d backlinks", len(fallbackBls)) + } +} + +// TestWikiLinks_TitleRenameResolvesPreExistingBrokenRows covers the +// other half of cascadeTitleRename: a source that wrote `[[New Title]]` +// BEFORE any item had that title — so its row stored as broken — +// should resolve when an existing item gets renamed TO "New Title". +// No content rewrite needed; only the target_item_id flip. +func TestWikiLinks_TitleRenameResolvesPreExistingBrokenRows(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + // Source mentions "New Title" before any such item exists. + createTestItem(t, s, ws.ID, col.ID, "Source", + "I expect a [[New Title]] item to exist someday.") + + // Now rename an existing item TO "New Title". The cascade's + // stage 2 should flip the broken row. + target := createTestItem(t, s, ws.ID, col.ID, "Placeholder", "") + newTitle := "New Title" + if _, err := s.UpdateItem(target.ID, models.ItemUpdate{Title: &newTitle}); err != nil { + t.Fatalf("UpdateItem rename: %v", err) + } + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("expected pre-existing broken row to resolve after rename, got %d backlinks", len(got)) + } +} + +// TestWikiLinks_TitleRenameNoChangeIsNoOp — calling UpdateItem with +// the SAME title (or with no title field at all) must not trigger +// the cascade. Cheap path that paid nothing pre-Phase-2a should pay +// nothing now either. +func TestWikiLinks_TitleRenameNoChangeIsNoOp(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + target := createTestItem(t, s, ws.ID, col.ID, "Stable Title", "") + createTestItem(t, s, ws.ID, col.ID, "Source", + "See [[Stable Title]] for details.") + + // Update with same title — cascade should early-return on + // oldTitle == newTitle. + same := "Stable Title" + if _, err := s.UpdateItem(target.ID, models.ItemUpdate{Title: &same}); err != nil { + t.Fatalf("UpdateItem with same title: %v", err) + } + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("backlinks unaffected by no-op rename, got %d", len(got)) + } +} + +// TestWikiLinks_TitleRenameRewritesSelfReferences — Codex round 5 +// finding 2 caught: when the renamed item mentions ITSELF by its +// (now-old) title in its body, the cascade MUST also rewrite the +// self-reference. Otherwise a title-only rename leaves the body's +// `[[Old Title]]` bracket pointing at a now-non-existent title +// while the index still records a working backlink — index and +// renderer drift apart. Self-link visibility filtering happens at +// GetBacklinks query time, so the panel still hides it. +func TestWikiLinks_TitleRenameRewritesSelfReferences(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + // Item that mentions itself by title. + item := createTestItem(t, s, ws.ID, col.ID, "Self Title", "") + body := "I mention myself: [[Self Title]]." + if _, err := s.UpdateItem(item.ID, models.ItemUpdate{Content: &body}); err != nil { + t.Fatalf("seed self-ref content: %v", err) + } + + // Title-only rename. Cascade INCLUDES self, so the self-ref + // gets rewritten in-band — body becomes `[[Renamed Self]]` + // pointing at this same item (now titled "Renamed Self"). + newTitle := "Renamed Self" + if _, err := s.UpdateItem(item.ID, models.ItemUpdate{Title: &newTitle}); err != nil { + t.Fatalf("rename self: %v", err) + } + + post, _ := s.GetItem(item.ID) + if strings.Contains(post.Content, "[[Self Title]]") { + t.Errorf("self-reference should have been rewritten by cascade, got %q", post.Content) + } + if !strings.Contains(post.Content, "[[Renamed Self]]") { + t.Errorf("expected `[[Renamed Self]]` in cascade-rewritten content, got %q", post.Content) + } + + // Self still hidden from own backlinks panel — GetBacklinks + // filters self-links at query time independent of indexing. + got, _ := s.GetBacklinks(item.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 0 { + t.Errorf("self-link must stay hidden in panel, got %d", len(got)) + } +} + +// TestWikiLinks_RefShapedWithWhitespaceFallsThroughToTitle regresses +// Codex round 10 P2: a ref-shaped body with surrounding whitespace +// (`[[ TASK-5 ]]`) that doesn't match any actual ref falls through +// to title lookup using the UNTRIMMED key — matches the renderer's +// `key.toLowerCase()` (no trim) at markdown.ts:541-543. An item +// literally titled " TASK-5 " (with spaces) must resolve via the +// fallback when no real TASK-5 ref exists. +func TestWikiLinks_RefShapedWithWhitespaceFallsThroughToTitle(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") // prefix "TASKS" + + // Item literally titled with whitespace and ref shape. There's + // no TASKS collection prefix variant matching "TASK-5", so the + // ref resolution misses and title fallback must use the raw + // (untrimmed) key. + target := createTestItem(t, s, ws.ID, col.ID, " TASK-5 ", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "See [[ TASK-5 ]] anyway.") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("expected ref-fallback to use untrimmed key, got %d backlinks", len(got)) + } +} + +// TestWikiLinks_SoftDeletedTargetRetargetsOnNewItem regresses Codex +// round 8 P2: when item A "Foo" resolves a backlink and is then +// soft-deleted, a NEW item B titled "Foo" must flip the row to +// point at B. The renderer ignores deleted-target links, so an +// index that keeps pointing at deleted A means GetBacklinks(B) +// misses the link the UI would actually render to B. +func TestWikiLinks_SoftDeletedTargetRetargetsOnNewItem(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + a := createTestItem(t, s, ws.ID, col.ID, "Foo", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "Mention [[Foo]].") + aBls, _ := s.GetBacklinks(a.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(aBls) != 1 { + t.Fatalf("baseline: A should have 1 backlink, got %d", len(aBls)) + } + + // Soft-delete A. + if err := s.DeleteItem(a.ID); err != nil { + t.Fatalf("delete A: %v", err) + } + + // Create B with same title — should flip the row. + b := createTestItem(t, s, ws.ID, col.ID, "Foo", "") + + bBls, _ := s.GetBacklinks(b.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(bBls) != 1 { + t.Errorf("B should inherit the backlink after A's soft-delete + B's creation, got %d", len(bBls)) + } +} + +// TestWikiLinks_CascadeDoesNotCorruptLiteralPipeNeighbor regresses +// Codex round 7 finding 2: when items A "Old Title" and B +// "Old Title|alias" are both referenced from one source, renaming A +// must NOT rewrite the B reference. The prior broad-regex cascade +// matched `[[Old Title|alias]]` against the A-rename pattern, +// corrupting the B link. Position-based per-row cascade fixes this +// — only the brackets whose wl row resolves to A get rewritten. +func TestWikiLinks_CascadeDoesNotCorruptLiteralPipeNeighbor(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + a := createTestItem(t, s, ws.ID, col.ID, "Old Title", "") + b := createTestItem(t, s, ws.ID, col.ID, "Old Title|alias", "") + createTestItem(t, s, ws.ID, col.ID, "Source", + "Mentions [[Old Title]] (to A) and [[Old Title|alias]] (to B).") + + // Rename only A. + newTitle := "New Title" + if _, err := s.UpdateItem(a.ID, models.ItemUpdate{Title: &newTitle}); err != nil { + t.Fatalf("rename A: %v", err) + } + + // A's reference rewritten; B's reference left intact. + // Look up the source item by querying via items that mention A. + aBacklinksProbe, _ := s.GetBacklinks(a.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(aBacklinksProbe) == 0 { + t.Fatalf("expected at least one backlink to A so we can locate source") + } + srcItem, err := s.GetItem(aBacklinksProbe[0].SourceItemID) + if err != nil { + t.Fatalf("GetItem source: %v", err) + } + srcContent := srcItem.Content + if !strings.Contains(srcContent, "[[New Title]]") { + t.Errorf("A's reference should be rewritten to [[New Title]], got: %q", srcContent) + } + if !strings.Contains(srcContent, "[[Old Title|alias]]") { + t.Errorf("B's reference [[Old Title|alias]] should be untouched, got: %q", srcContent) + } + if strings.Contains(srcContent, "[[New Title|alias]]") { + t.Errorf("B's reference was incorrectly rewritten via A's cascade, got: %q", srcContent) + } + + // A's backlinks should include the source (via the rewritten + // [[New Title]] bracket). + aBls, _ := s.GetBacklinks(a.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(aBls) != 1 { + t.Errorf("A should have 1 backlink post-rename, got %d", len(aBls)) + } + // B's backlinks should still include the source (untouched). + bBls, _ := s.GetBacklinks(b.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(bBls) != 1 { + t.Errorf("B should still have 1 backlink (its bracket was untouched), got %d", len(bBls)) + } +} + +// TestWikiLinks_RefShapedFallsThroughToTitle regresses Codex round 6 +// finding 1. A ref-shaped body like `[[ISO-9001]]` should resolve to +// an item literally titled "ISO-9001" when no ISO-9001 ref-item +// exists — the renderer falls through (markdown.ts:513), so the +// index must too. Without the fallback, GetBacklinks would never +// find the backlink even though the renderer renders the link. +func TestWikiLinks_RefShapedFallsThroughToTitle(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") // prefix "TASKS" + + // Item literally titled with a ref shape that does NOT match + // any existing collection's prefix-number. There's no ISO + // collection in this workspace, so the body fails the ref + // resolution and must fall through to title. + target := createTestItem(t, s, ws.ID, col.ID, "ISO-9001", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "See [[ISO-9001]] for the standard.") + + got, _ := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 1 { + t.Errorf("expected ref-shaped body to fall through to title, got %d backlinks", len(got)) + } +} + +// TestWikiLinks_TitleAndContentRenameLeavesUserContentVerbatim +// documents the behavior choice for combined title+content updates: +// the user's just-submitted content is authoritative, so the cascade +// EXCLUDES self in this case (excludeSelf=true). If their new +// content contains `[[Old Title]]`, it stays as written and the +// index correctly stores it as broken — matching what the renderer +// would render. This mirrors documents.go::updateLinksInTx, which +// also leaves the renamed entity's own content alone. +// +// The title-only case (input.Content == nil) is the complement: +// cascade INCLUDES self because there's no fresh user input — +// stale `[[Old Title]]` would otherwise go broken. Covered by +// TestWikiLinks_TitleRenameRewritesSelfReferences. +func TestWikiLinks_TitleAndContentRenameLeavesUserContentVerbatim(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + item := createTestItem(t, s, ws.ID, col.ID, "Old Title", "") + body := "I mention myself: [[Old Title]] in old content." + if _, err := s.UpdateItem(item.ID, models.ItemUpdate{Content: &body}); err != nil { + t.Fatalf("seed self-ref content: %v", err) + } + + // Combined title + content rename. New content still contains + // `[[Old Title]]` because the user wrote it that way. Cascade + // must respect the user's submission — DO NOT auto-rewrite. + newTitle := "New Title" + newBody := "Now updated: [[Old Title]] still mentioned in new content." + if _, err := s.UpdateItem(item.ID, models.ItemUpdate{ + Title: &newTitle, + Content: &newBody, + }); err != nil { + t.Fatalf("combined rename + content update: %v", err) + } + + post, _ := s.GetItem(item.ID) + // User content stands as written. + if !strings.Contains(post.Content, "[[Old Title]]") { + t.Errorf("combined update: user's [[Old Title]] should remain verbatim, got %q", post.Content) + } + // Backlinks-of-self should be empty: index stores the bracket + // as broken (no item titled "Old Title" exists), self-link + // filter in GetBacklinks would hide anyway. Either way: 0. + got, _ := s.GetBacklinks(item.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(got) != 0 { + t.Errorf("self/broken refs should produce 0 backlinks, got %d", len(got)) + } +} + +// TestWikiLinks_DuplicateSlashTitleNoTheft — Codex round 5 finding 1 +// regression. When item "tasks/Setup" exists and a source has a +// backlink resolved to it via stage 1 (literal match), creating a +// SECOND item titled "tasks/Setup" must NOT steal the row. The +// stage-3 retarget UPDATE has an EXISTS clause that scopes the flip +// to rows whose current target is NOT titled the same as us — +// i.e. rows resolved via stage-2 qualified fallback to a different +// item, not via stage-1 to a literal twin. +func TestWikiLinks_DuplicateSlashTitleNoTheft(t *testing.T) { + s := testStore(t) + ws := createTestWorkspace(t, s, "Test") + col := createTestCollection(t, s, ws.ID, "Tasks") + + original := createTestItem(t, s, ws.ID, col.ID, "tasks/Setup", "") + createTestItem(t, s, ws.ID, col.ID, "Source", "See [[tasks/Setup]].") + bls, _ := s.GetBacklinks(original.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(bls) != 1 { + t.Fatalf("baseline: stage-1 literal should resolve, got %d", len(bls)) + } + + // Create a second item with the same slash-containing title. + duplicate := createTestItem(t, s, ws.ID, col.ID, "tasks/Setup", "") + + // Original keeps its backlink — no theft from a literal twin. + stillBls, _ := s.GetBacklinks(original.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(stillBls) != 1 { + t.Errorf("original slash-title item lost backlink to duplicate creation, got %d", len(stillBls)) + } + // Duplicate has no backlinks — nothing was stolen. + dupBls, _ := s.GetBacklinks(duplicate.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true}) + if len(dupBls) != 0 { + t.Errorf("duplicate stole backlinks, got %d", len(dupBls)) + } +} + // itoa is strconv.Itoa renamed to keep test bodies readable when // they're already heavy on ref-formatting. func itoa(n int) string {