diff --git a/beads/memory.go b/beads/memory.go index 5781a5667a19f57ae749c93c2feb687bad4f2844..98951f1642d0361fdedb48c32f13d0052219bfb8 100644 --- a/beads/memory.go +++ b/beads/memory.go @@ -61,11 +61,12 @@ // Memory is one `bd remember` entry. type Memory struct { Slug string // the config key with the "kv.memory." prefix stripped - Text string // the value, with both newline spellings normalised - // Paragraphs is Text split on blank lines. Values are typed into shell - // strings, so they arrive with a mix of real newlines and literal "\n" - // escapes; a page that dumped the raw value would render either as one blob. - Paragraphs []string + // Text is the value with both newline spellings normalised. It is markdown — + // `bd remember` stores whatever was written, and what is written is the same + // markdown the memory files carry — and it is left as text here: this package + // renders nothing, and the split into paragraphs, lists and code blocks is + // the renderer's, not a projection's. + Text string // Revision is the commit that last changed this key's value, or nil when the // walk could not reach it (see MemoryView.WalkTruncated). Revision *MemoryRevision @@ -222,9 +223,8 @@ view.Memories = make([]Memory, 0, len(tracked)) for key := range tracked { m := Memory{ - Slug: strings.TrimPrefix(key, memoryPrefix), - Text: texts[key], - Paragraphs: memoryParagraphs(texts[key]), + Slug: strings.TrimPrefix(key, memoryPrefix), + Text: texts[key], } if rev, ok := walk.revisions[key]; ok { m.Revision = &rev @@ -394,9 +394,6 @@ // memoryEscapedNewline matches the two-character escapes that reach the value // because the memory was typed into a shell string: "\r\n" and "\n" written out // as backslash sequences rather than as newlines. var memoryEscapedNewline = regexp.MustCompile(`\\r\\n|\\n`) - -// memoryBlankLine splits a memory into paragraphs on runs of blank lines. -var memoryBlankLine = regexp.MustCompile(`\n[ \t]*\n[ \t\n]*`) // normalizeMemoryText brings a stored value's two newline spellings together. // The same memory arrives with real newlines when it was written from a file or @@ -407,24 +404,6 @@ v = strings.ReplaceAll(v, "\r\n", "\n") v = strings.ReplaceAll(v, "\r", "\n") v = memoryEscapedNewline.ReplaceAllString(v, "\n") return strings.TrimSpace(v) -} - -// memoryParagraphs splits normalised text on blank lines. Line breaks *inside* a -// paragraph are kept — memories are full of bullet lists, and joining those into -// a sentence is not rendering, it is losing the structure the author typed. -func memoryParagraphs(text string) []string { - if text == "" { - return nil - } - parts := memoryBlankLine.Split(text, -1) - out := make([]string, 0, len(parts)) - for _, p := range parts { - if strings.TrimSpace(p) == "" { - continue - } - out = append(out, strings.Trim(p, "\n")) - } - return out } // matchesMemorySearch is the ?q= rule: a case-insensitive substring of the slug diff --git a/beads/memory_test.go b/beads/memory_test.go index 516ef4b3ac5887b4e10199b9f9cd7e10431fd8e4..4683f3ddd0c34ff91775e6d9ebba8e29395793b7 100644 --- a/beads/memory_test.go +++ b/beads/memory_test.go @@ -157,7 +157,9 @@ } // A value carries both newline spellings — real ones when it was written from a // file, literal backslash-n when it was typed into a shell string — and both -// have to become the same paragraphs. +// have to become the same text. It is the one shaping this projection does: the +// markdown itself is the renderer's to read, and it cannot read a value whose +// line breaks are still two characters. func TestMemoriesNormaliseBothNewlineSpellings(t *testing.T) { const escaped = `First paragraph.\n\nSecond paragraph, line one.\nLine two.` real := "First paragraph.\n\nSecond paragraph, line one.\nLine two." @@ -174,9 +176,7 @@ } v := buildMemories(t, sess, "") require.Len(t, v.Memories, 3) - want := []string{"First paragraph.", "Second paragraph, line one.\nLine two."} for _, m := range v.Memories { - assert.Equal(t, want, m.Paragraphs, "memory %q", m.Slug) assert.Equal(t, "First paragraph.\n\nSecond paragraph, line one.\nLine two.", m.Text, "memory %q", m.Slug) assert.NotContains(t, m.Text, `\n`, "the literal escape must not survive into the text") diff --git a/beads/prefixes.go b/beads/prefixes.go index 85315f0b0889dcf204b406c367a119b1925ccbc1..8fceab93c6d8fe91bee2ba03e6d5b2d24f6ccf2a 100644 --- a/beads/prefixes.go +++ b/beads/prefixes.go @@ -10,17 +10,26 @@ "sourcecraft.dev/bigbes/sr-ht-dolt/browse" ) -// --- which database owns "-" -------------------------------------- +// --- which database owns "-", and which one holds "[[slug]]" ------ // // A global-tracker issue that says "blocked by artifacts-nex.2" is naming a row // in another database, and without this the reader has to work out which one and // go there by hand. Two levels of tracker only pay for themselves if they point // at each other. +// +// Memories name each other the same way and land the same way: the mirroring +// workflow files a memory by its type, so `[[srht-service-release]]` written in +// one tracker routinely lives in another, and the reference is a dead end +// wherever it is read. Both questions are answered from one read of one table — +// issue_prefix and the kv.memory.* keys are rows of the same config — so they +// are one index and one cached projection rather than two passes over every +// database. // // This file answers the one question that needs several databases: given an -// id-shaped string in some text, which database — of the ones this caller may -// browse — owns it. Whether the answer becomes a link, and what that link looks -// like, is the renderer's business; this package renders nothing. +// id-shaped string or a memory slug in some text, which database — of the ones +// this caller may browse — holds it. Whether the answer becomes a link, and what +// that link looks like, is the renderer's business; this package renders +// nothing. // // Authorization is not here, exactly as it is not in ReadyAcross: handing this // function a database is the statement that the caller may read it. A database @@ -32,32 +41,47 @@ // ("global", "artifacts", "sr-ht-dolt", …). It sits in the same config table the // memories do, beside compact_tier2_days and the rest of the tracker's settings. const prefixKey = "issue_prefix" -// PrefixCache is the prefix index's projection cache: one prefix per database, -// gated on the head hash and expiring on ReadyCacheTTL — /ready's bounds and not -// a second set (see cache.go). The zero value is usable, so a holder that has no -// constructor can carry one as a field. +// PrefixCache is the index's projection cache: one config projection per +// database, gated on the head hash and expiring on ReadyCacheTTL — /ready's +// bounds and not a second set (see cache.go). The zero value is usable, so a +// holder that has no constructor can carry one as a field. type PrefixCache struct { projectionCache[prefixEntry] } -// prefixEntry is one database's cached prefix projection: the prefix its config -// names, and the total that read reported. The total is cached with the prefix -// rather than recomputed because a cache hit reads no row — it must say what -// the read that produced it said, including that the read was partial. +// prefixEntry is one database's cached config projection: the issue prefix its +// config names, the memory slugs it stores, and the total that read reported. +// The total is cached with them rather than recomputed because a cache hit reads +// no row — it must say what the read that produced it said, including that the +// read was partial. type prefixEntry struct { prefix string + // slugs is every kv.memory.* key in that config, prefix stripped, in the + // order the read returned them. It is cached beside the prefix because it + // comes out of the same rows: a second projection over the same table would + // double the reads to answer half the question. + slugs []string // configTotal is the config table's reported total, clipped or not. Over Max - // it means the read stopped short, and an absent prefix is then a row that - // was never reached rather than a row that does not exist. + // it means the read stopped short, and an absent prefix — or an absent + // memory — is then a row that was never reached rather than a row that does + // not exist. configTotal int } -// PrefixIndex is prefix → the database that owns it, over the databases one -// caller may browse. It is built per caller and must not be shared between them: -// what it holds is precisely the set of databases that caller is allowed to know -// exists. +// PrefixIndex is prefix → the database that owns it and memory slug → the +// database that holds it, over the databases one caller may browse. It is built +// per caller and must not be shared between them: what it holds is precisely the +// set of databases that caller is allowed to know exists. type PrefixIndex struct { byPrefix map[string]ReadyDatabase + // byMemory is slug → the first database listed that stores a memory under it. + // First and not "the only one", unlike byPrefix: a prefix two trackers both + // claim is an ambiguity, but the same memory slug in two trackers is normally + // the same memory — the mirroring workflow re-files one by its type, and a + // re-typed memory is left behind in the tracker it moved out of. The caller + // lists the database whose page this is first, so a slug present locally + // always resolves locally, and the fallback is the caller's listing order. + byMemory map[string]ReadyDatabase // Failed lists the databases that could not be read. It is here for the // caller's log and for nothing else: a database that could not be read costs // the ids it owns their link, and a page may not say more than that. @@ -103,15 +127,15 @@ // knows makes it one. Everything else is left as it was written. var idPattern = regexp.MustCompile(`\b[0-9a-z_]+(?:-[0-9a-z_]+)*-[0-9a-z]+(?:\.[0-9a-z]+)*\b`) // PrefixesAcross builds the index over the databases it is handed: for each, the -// issue_prefix its config names. +// issue_prefix its config names and the memory slugs its config stores. // // It is bounded exactly as ReadyAcross is — the same head-hash gate, the same // TTL, the same ceiling — because it is the same read pattern: N stores opened -// for one request. It is cheaper per database, though: the projection is one row -// of a dozen-row table, and the fingerprint is not asked for separately, since a -// config table carrying issue_prefix is itself the statement that this is a bd -// tracker. A database with no such row simply owns no prefix, and that answer is -// cached like any other. +// for one request. It is cheaper per database, though: the projection is a +// dozen-row table read once for both halves, and the fingerprint is not asked +// for separately, since a config table carrying issue_prefix is itself the +// statement that this is a bd tracker. A database with no such row simply owns +// no prefix, and that answer is cached like any other. // // now is the clock the TTL is measured against, passed in rather than read here // for the reason ReadyAcross takes one: this package reads no hidden clock. @@ -125,7 +149,10 @@ open ReadyOpener, cache *PrefixCache, now time.Time, ) *PrefixIndex { - index := &PrefixIndex{byPrefix: map[string]ReadyDatabase{}} + index := &PrefixIndex{ + byPrefix: map[string]ReadyDatabase{}, + byMemory: map[string]ReadyDatabase{}, + } if len(dbs) > ReadyMaxDatabases { // The first Max in the order the caller listed them. The caller puts the @@ -153,6 +180,15 @@ Database: d, ShownOf: entry.configTotal, }) } + // The memory half is recorded before the prefix skip below: a tracker that + // names no issue prefix still stores memories, and its slugs are as + // linkable as any other's. + for _, slug := range entry.slugs { + if _, taken := index.byMemory[slug]; !taken { + index.byMemory[slug] = d + } + } + prefix := entry.prefix if prefix == "" { continue @@ -205,7 +241,7 @@ if entry, ok := cache.lookup(d.ID, head, now); ok { return entry, nil } - entry, err := readPrefix(ctx, sess, ref) + entry, err := readConfigProjection(ctx, sess, ref) if err != nil { return prefixEntry{}, err } @@ -213,14 +249,19 @@ cache.store(d.ID, head, now, entry) return entry, nil } -// readPrefix reads the issue_prefix row out of a database's config table, and -// the total that read reported. A missing table — this is not a bd tracker — is -// no prefix, the treatment every optional table gets here. +// readConfigProjection reads one database's config table down to what the index +// needs of it: the issue_prefix row, the kv.memory.* keys, and the total that +// read reported. A missing table — this is not a bd tracker — is neither a +// prefix nor a memory, the treatment every optional table gets here. // -// The total comes back with the prefix because the two answers this can produce -// are otherwise identical: a config with no issue_prefix row and a config whose -// issue_prefix row was left past Max both arrive here as no prefix at all. -func readPrefix(ctx context.Context, sess BrowseSession, ref string) (prefixEntry, error) { +// The total comes back with them because the answers this can produce are +// otherwise identical: a config with no issue_prefix row and a config whose +// issue_prefix row was left past Max both arrive here as no prefix at all, and +// the same holds of a memory key. +// +// The whole table is walked rather than stopped at the prefix row: the memory +// keys sit anywhere in it, and one pass is what makes this one read. +func readConfigProjection(ctx context.Context, sess BrowseSession, ref string) (prefixEntry, error) { rows, total, err := readRowsOptional(ctx, sess, ref, memoryTable) if err != nil { return prefixEntry{}, err @@ -231,13 +272,20 @@ return entry, nil } cols := indexCols(rows.Columns) for _, r := range rowsOf(rows) { - if cell(cols, r, "key") != prefixKey { - continue + key := cell(cols, r, "key") + switch { + case key == prefixKey: + // Lowercased because that is the case the ids themselves are written in, + // and the index is looked up by what the text says. + entry.prefix = strings.ToLower(strings.TrimSpace(cell(cols, r, "value"))) + case strings.HasPrefix(key, memoryPrefix): + // The slug is stored as written and matched as written: `bd remember + // --key` is case-sensitive, and two memories differing only in case are + // two memories. + if slug := strings.TrimPrefix(key, memoryPrefix); slug != "" { + entry.slugs = append(entry.slugs, slug) + } } - // Lowercased because that is the case the ids themselves are written in, - // and the index is looked up by what the text says. - entry.prefix = strings.ToLower(strings.TrimSpace(cell(cols, r, "value"))) - return entry, nil } return entry, nil } @@ -251,6 +299,32 @@ return ReadyDatabase{}, false } d, ok := ix.byPrefix[prefix] return d, ok +} + +// LookupMemory returns the database holding the memory written under slug. A +// slug no database the caller may browse stores is unknown here, which is what +// keeps a reference to a memory in a database they may not see indistinguishable +// from a reference to one that was never written. +func (ix *PrefixIndex) LookupMemory(slug string) (ReadyDatabase, bool) { + if ix == nil { + return ReadyDatabase{}, false + } + d, ok := ix.byMemory[slug] + return d, ok +} + +// MemorySlugs lists the slugs the index knows, in order. Like Prefixes, it +// exists for the caller's log line and for tests. +func (ix *PrefixIndex) MemorySlugs() []string { + if ix == nil { + return nil + } + out := make([]string, 0, len(ix.byMemory)) + for s := range ix.byMemory { + out = append(out, s) + } + sort.Strings(out) + return out } // Prefixes lists the prefixes the index knows, in order. It exists for the diff --git a/beads/prefixes_test.go b/beads/prefixes_test.go index 6dd34c30315fdd6d049b3cd5e57f749582b3548c..d3afe6d74403e39986d26046dea1c34de807b0bb 100644 --- a/beads/prefixes_test.go +++ b/beads/prefixes_test.go @@ -283,3 +283,127 @@ // So does an empty one. empty := PrefixesAcross(t.Context(), nil, nil, &PrefixCache{}, readyNow) assert.Nil(t, empty.Scan("artifacts-46c.2")) } + +// --- the memory half of the same read ------------------------------------------ + +// memoryTracker is a tracker whose config holds memories beside its settings. +// The prefix is a parameter because a tracker that names none still holds +// memories, and the two halves of the projection are independent. +func memoryTracker(head, prefix string, slugs ...string) *fakeReadyDB { + page := &browse.RowPage{ + Columns: []string{"key", "value"}, + Rows: [][]string{{"compact_tier2_days", "30"}}, + } + if prefix != "" { + page.Rows = append(page.Rows, []string{"issue_prefix", prefix}) + } + for _, s := range slugs { + page.Rows = append(page.Rows, []string{memoryPrefix + s, "the body of " + s}) + } + page.Total = len(page.Rows) + return &fakeReadyDB{ + branches: []browse.Branch{{Name: "main", Head: head}}, + tables: beadsTables(), + rows: map[string]*browse.RowPage{"config": page}, + } +} + +// The slugs come out of the same read the prefix does, and a slug no database +// holds is unknown — which is also the answer for one held by a database this +// caller may not browse, since such a database is never handed to the index. +func TestPrefixesAcrossIndexesMemorySlugs(t *testing.T) { + in := &readyInstance{dbs: map[int]*fakeReadyDB{ + 1: memoryTracker("h-global", "global", "go-vcs-stamp-dirty", "srht-service-release"), + 2: memoryTracker("h-artifacts", "artifacts", "apk-index-cadence"), + }} + dbs := []ReadyDatabase{ + {ID: 1, OwnerName: "bigbes", Name: "beads-global"}, + {ID: 2, OwnerName: "bigbes", Name: "sourcehut-artifacts"}, + } + + index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow) + + assert.Equal(t, + []string{"apk-index-cadence", "go-vcs-stamp-dirty", "srht-service-release"}, + index.MemorySlugs()) + d, ok := index.LookupMemory("go-vcs-stamp-dirty") + require.True(t, ok) + assert.Equal(t, "bigbes/beads-global", d.Slug()) + d, ok = index.LookupMemory("apk-index-cadence") + require.True(t, ok) + assert.Equal(t, "bigbes/sourcehut-artifacts", d.Slug()) + _, ok = index.LookupMemory("never-written") + assert.False(t, ok) + + // One read per database answered both questions. + assert.Equal(t, 1, in.dbs[1].rowReads) + assert.Equal(t, []string{"artifacts", "global"}, index.Prefixes()) +} + +// A slug two trackers hold resolves to the first one listed, unlike a prefix two +// trackers claim, which resolves to neither. The difference is what the +// duplicate means: two trackers claiming one prefix own two different id spaces, +// while the same memory slug in two trackers is normally one memory that was +// re-filed and left a copy behind. The caller lists the page's own database +// first, so this is "the tracker you are reading wins". +func TestPrefixesAcrossResolvesADuplicateSlugToTheFirstDatabase(t *testing.T) { + in := &readyInstance{dbs: map[int]*fakeReadyDB{ + 1: memoryTracker("h1", "here", "shared-note"), + 2: memoryTracker("h2", "there", "shared-note"), + }} + dbs := []ReadyDatabase{ + {ID: 1, OwnerName: "alice", Name: "own"}, + {ID: 2, OwnerName: "bob", Name: "other"}, + } + + index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow) + + d, ok := index.LookupMemory("shared-note") + require.True(t, ok, "a slug two trackers hold is still a link") + assert.Equal(t, "alice/own", d.Slug()) +} + +// A tracker that names no issue prefix still contributes its memories: the two +// halves of the projection do not gate each other. +func TestPrefixesAcrossIndexesMemoriesOfAPrefixlessDatabase(t *testing.T) { + in := &readyInstance{dbs: map[int]*fakeReadyDB{ + 1: memoryTracker("h1", "", "only-a-memory"), + }} + dbs := []ReadyDatabase{{ID: 1, OwnerName: "alice", Name: "no-prefix"}} + + index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow) + + assert.Empty(t, index.Prefixes()) + d, ok := index.LookupMemory("only-a-memory") + require.True(t, ok) + assert.Equal(t, "alice/no-prefix", d.Slug()) +} + +// The cached projection carries the slugs, so a second build behind the +// head-hash gate answers the memory question without reading a row either. +func TestPrefixesAcrossCachesTheSlugs(t *testing.T) { + in := &readyInstance{dbs: map[int]*fakeReadyDB{ + 1: memoryTracker("h1", "here", "cached-slug"), + }} + dbs := []ReadyDatabase{{ID: 1, OwnerName: "alice", Name: "own"}} + cache := &PrefixCache{} + + PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow) + reads := in.dbs[1].rowReads + require.Greater(t, reads, 0) + + second := PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow.Add(time.Second)) + + assert.Equal(t, reads, in.dbs[1].rowReads, "the gate holds for the slugs too") + _, ok := second.LookupMemory("cached-slug") + assert.True(t, ok) +} + +// An index that was never built holds no memory either. +func TestNilPrefixIndexKnowsNoMemory(t *testing.T) { + var index *PrefixIndex + + assert.Nil(t, index.MemorySlugs()) + _, ok := index.LookupMemory("anything") + assert.False(t, ok) +} diff --git a/docs/DESIGN.views.md b/docs/DESIGN.views.md index 10ec782f6d5a0b6d7e9b262f1b0f77b685242c6b..b3ed2bb885c6e11e1dc368c975fffe1a54bf9ff4 100644 --- a/docs/DESIGN.views.md +++ b/docs/DESIGN.views.md @@ -213,6 +213,40 @@ Owner is frequently on metered mobile internet (ZeroTier to the lab, mobile uplink). Calibrate downloads … ``` +### 2.3 The body is markdown — and its references resolve across databases + +*Added after the chapters above shipped; the sketch in 2.2 shows the plain-text +bodies this replaces.* + +A memory's value is markdown and always was: `bd remember` stores what was +typed, and what is typed is the same prose the memory files carry — bold +leaders, code spans, fenced recipes, numbered steps, and `[[slug]]` references +to related memories. Printing it as pre-wrapped paragraphs shows the source +rather than the document, and leaves every reference a dead end. + +- **The renderer** is `web/markdown.go`, one shared `goldmark.Markdown` (GFM, + no unsafe HTML, no dangerous URL schemes). Three departures from stock: + - `[[slug]]` is an **inline parser**, not a text rewrite — which is what makes + a reference written inside a code span stay text by construction. + - **Raw HTML is escaped and shown, not dropped.** goldmark's safe default + omits it, and these memories are full of `` spellings — + `SRHT__VER` — that CommonMark reads as tags. Omitting one rewrites the + sentence silently, which is worse than showing markup. + - **Images render as links.** An `` at another host is a request that + host makes on behalf of whoever opened the page. +- **Issue ids in the prose** link too, through an AST transformer over the text + nodes — skipping code spans, links and wikilinks — and through ch. 5's index. +- **The index** is ch. 5's, extended: `issue_prefix` and the `kv.memory.*` keys + are rows of the same `config` table, so one read per database answers both + questions and one cached projection carries both. Unlike a prefix two trackers + both claim, a slug two trackers both hold resolves to the first listed — the + page's own database — because the same slug in two trackers is normally one + memory that was re-filed and left a copy behind, not two different things. +- **What an unresolved reference looks like** is one rendering for two cases: a + slug nobody wrote, and a slug held by a database this caller may not browse. + A link, a distinct class or a different tooltip would each publish the + existence of a private database. + ## 3. Freshness in the header Nothing on the board says how fresh it is. `bd` pushes with a 30-second debounce @@ -283,7 +317,8 @@ - **The prefix index.** Every beads database stores its own prefix in `config` under `issue_prefix` (verified: `global`, `artifacts`, …). Build an index prefix → repository, warmed lazily and refreshed on the same head-hash / - TTL basis as ch. 4's cache, over the databases the *caller* may browse. A + TTL basis as ch. 4's cache, over the databases the *caller* may browse. (It + carries the memory slugs of the same `config` read as well; see §2.3.) A prefix belonging to a database the caller cannot see is not linked, and the page must not reveal that it exists. - **The pattern.** `-` where prefix is a known one and suffix is diff --git a/go.mod b/go.mod index 639d370d014d3baafb3a90f337ada43ac03ffeb5..5bd6373925ac52ca98c453009ced4b6ceb7202c9 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ github.com/sirupsen/logrus v1.9.4 github.com/stretchr/testify v1.11.1 github.com/vaughan0/go-ini v0.0.0-20130923145212-a98ad7ee00ec github.com/vektah/gqlparser/v2 v2.5.36 + github.com/yuin/goldmark v1.8.2 go.bigb.es/auxilia v0.7.0 google.golang.org/grpc v1.79.3 gopkg.in/go-jose/go-jose.v2 v2.6.3 diff --git a/go.sum b/go.sum index 444b00849b09709496629bb425c9ec8220f303fa..79d36356c39544da79aeeff200b903b577ab2c9a 100644 --- a/go.sum +++ b/go.sum @@ -474,6 +474,8 @@ github.com/xtaci/smux v1.5.56/go.mod h1:IGQ9QYrBphmb/4aTnLEcJby0TNr3NV+OslIOMrX825Q= github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= +github.com/yuin/goldmark v1.8.2 h1:kEGpgqJXdgbkhcOgBxkC0X0PmoPG1ZyoZ117rDVp4zE= +github.com/yuin/goldmark v1.8.2/go.mod h1:ip/1k0VRfGynBgxOz0yCqHrbZXhcjxyuS66Brc7iBKg= github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ= github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= github.com/zeebo/blake3 v0.2.3 h1:TFoLXsjeXqRNFxSbk35Dk4YtszE/MQQGK10BH4ptoTg= diff --git a/web/beads.go b/web/beads.go index f843fd906707fc9dfa7a5b131314732a43e8505c..1432b194cb032de76c9d3aebd1136bcfb5a4fdaa 100644 --- a/web/beads.go +++ b/web/beads.go @@ -126,18 +126,12 @@ // that are already links, and nothing else in this service renders stored prose. // Building the index opens a store per database, so it is asked for by the one // rendering that needs it and never as part of the envelope. // -// The visibility rule is the /ready page's, for the same reason: enumerate with -// ListReposForViewer (the listing rule), then ask core.Allowed/OpBrowse per -// database (the access rule), and do both before a single store is opened. A -// database this caller may not browse never enters the index, so an id belonging -// to it renders as plain text — indistinguishable from one whose prefix matches -// nothing. That indistinguishability is the point: a tooltip, a class or an -// "unknown tracker" marker would each publish the existence of a database this -// caller is not allowed to know about. -// -// A listing that fails costs the page its links and not the page. This is -// decoration on top of an answer, exactly as the freshness line is, and it may -// never be the reason a reader gets a 500 instead of an issue. +// Which databases may enter the index is linkableDatabases' answer and not this +// function's. What that leaves here: a database this caller may not browse never +// enters it, so an id belonging to it renders as plain text — indistinguishable +// from one whose prefix matches nothing. That indistinguishability is the point: +// a tooltip, a class or an "unknown tracker" marker would each publish the +// existence of a database this caller is not allowed to know about. func (a *app) beadCrossLinks(r *http.Request, view View, repo *core.Repo, data any) *beadLinks { bv, ok := view.(*beadsView) if !ok { @@ -148,12 +142,46 @@ if !ok || (d.Mode != beadsModeDetail && d.Mode != beadsModeEpic) { return nil } + dbs, open, ok := a.linkableDatabases(r, repo) + if !ok { + return nil + } + + index := beads.PrefixesAcross(r.Context(), dbs, open, &bv.prefixes, timeNow()) + + // A store that cannot be read is a fact about this deployment and belongs in + // the log with its error. The page says nothing at all: the ids that database + // owns simply stay text. + for _, f := range index.Failed { + slog.Warn("reading a database for the issue link index failed", + "component", "web", "database", f.Database.Slug(), scribe.Err(f.Err)) + } + return &beadLinks{index: index} +} + +// linkableDatabases is the set of databases a cross-database link index may be +// built over for this request, with the opener that reaches them. Both the issue +// links and the memory links ask for it, and they must ask the same way: the +// index decides what a reader is shown, so the rule for what may enter it lives +// in one place rather than in each caller. +// +// The rule is /ready's. Enumerate with ListReposForViewer (the listing rule), +// then ask core.Allowed/OpBrowse per database (the access rule), and do both +// before a single store is opened. +// +// The false return is a listing that failed, and it costs the page its links and +// not the page: this is decoration on top of an answer, exactly as the freshness +// line is, and it may never be the reason a reader gets a 500. +func (a *app) linkableDatabases( + r *http.Request, + repo *core.Repo, +) ([]beads.ReadyDatabase, beads.ReadyOpener, bool) { _, caller := callerOf(r.Context()) repos, err := a.cfg.Repos.ListReposForViewer(r.Context(), caller) if err != nil { - slog.Error("listing databases for the issue link index failed", + slog.Error("listing databases for the cross-database link index failed", "component", "web", scribe.Err(err)) - return nil + return nil, nil, false } // The disk path never reaches beads: it is this service's arrangement of its @@ -168,9 +196,10 @@ } paths[cand.ID] = cand.Path entry := beads.ReadyDatabase{ID: cand.ID, OwnerName: cand.OwnerName, Name: cand.Name} if cand.ID == repo.ID { - // The database whose page this is goes first, so the one prefix the - // page cannot do without — its own, the ids that used to link and must - // keep linking — can never be the one the ceiling drops. + // The database whose page this is goes first. It is the one the ceiling + // may never drop — its own ids have always linked and must keep + // linking — and it is what makes a [[slug]] this tracker holds resolve + // here rather than in whichever other tracker also carries a copy. dbs = append([]beads.ReadyDatabase{entry}, dbs...) continue } @@ -184,15 +213,5 @@ return nil, fmt.Errorf("web: no store path for database %s", d.Slug()) } return a.cfg.Browse.Open(ctx, path) } - - index := beads.PrefixesAcross(r.Context(), dbs, open, &bv.prefixes, timeNow()) - - // A store that cannot be read is a fact about this deployment and belongs in - // the log with its error. The page says nothing at all: the ids that database - // owns simply stay text. - for _, f := range index.Failed { - slog.Warn("reading a database for the issue link index failed", - "component", "web", "database", f.Database.Slug(), scribe.Err(f.Err)) - } - return &beadLinks{index: index} + return dbs, open, true } diff --git a/web/handlers_view.go b/web/handlers_view.go index f379c9d4c2abd2f80e2b13bdca5c408064d1a1e8..acfae643a7af0ff7f1a28c28a7fd66ea0e8036d1 100644 --- a/web/handlers_view.go +++ b/web/handlers_view.go @@ -93,6 +93,13 @@ // beads detail pane — and is nil for every other page here, because // building it opens a store per database the caller may browse. A nil one // still renders text; see beadLinks.Text. Links *beadLinks + // Memories renders a memory's stored markdown, with its [[slug]] + // references and the issue ids in its prose linked to the databases that + // hold them. It is built by the memory view alone, for the reason Links is + // built by the detail pane alone: it opens a store per database this + // caller may browse. A nil one still renders the markdown, with every + // reference left as text; see memoryLinks.Body. + Memories *memoryLinks }{ Page: a.page(r, view.Label()+" — "+repo.OwnerName+"/"+repo.Name), Repo: repo, @@ -102,6 +109,7 @@ Views: applicableViews(a.views, tables), Head: headCommit(r.Context(), sess, ref), Data: data, Links: a.beadCrossLinks(r, view, repo, data), + Memories: a.memoryCrossLinks(r, view, repo), } a.render(w, http.StatusOK, pageName(view.Template()), envelope) } diff --git a/web/markdown.go b/web/markdown.go new file mode 100644 index 0000000000000000000000000000000000000000..5a4d24577cdc8b44c9ee31f03967acc02d685eda --- /dev/null +++ b/web/markdown.go @@ -0,0 +1,385 @@ +package web + +import ( + "bytes" + "html/template" + "net/url" + + "github.com/yuin/goldmark" + "github.com/yuin/goldmark/ast" + "github.com/yuin/goldmark/extension" + "github.com/yuin/goldmark/parser" + "github.com/yuin/goldmark/renderer" + "github.com/yuin/goldmark/renderer/html" + "github.com/yuin/goldmark/text" + "github.com/yuin/goldmark/util" + + "sourcecraft.dev/bigbes/sr-ht-dolt/beads" +) + +// --- memories, rendered as the markdown they are ------------------------------- +// +// A memory's value is markdown and always was: `bd remember` stores what was +// typed, and what is typed is the same prose the memory files carry — bold +// leaders, code spans, fenced blocks, numbered steps. The view used to print it +// as pre-wrapped paragraphs, which is readable but is the source and not the +// document: a bullet list stays a line starting with a hyphen, a recipe stays +// four spaces of indent, and `**Why:**` keeps its asterisks. +// +// Three things this rendering does that a stock markdown filter would not: +// +// 1. `[[slug]]` becomes a link to the memory it names, in whichever database +// holds it (see the wikilink parser below). That is the whole point of the +// notation, and memories reference each other across trackers constantly: +// the mirroring workflow files a memory by its type, so a related memory is +// as likely to be in another tracker as in this one. +// 2. An issue id in the prose links to the issue, through the same +// cross-database index the beads detail pane uses. +// 3. Raw HTML is *escaped and shown*, not dropped. goldmark's safe default +// omits it, and the memory corpus is full of `` spellings — +// `SRHT__VER`, `~/data/home/` — that CommonMark reads as tags. +// Omitting them would silently rewrite `SRHT__VER` to `SRHT__VER`, +// which is worse than showing markup: it is showing a different fact. +// +// Safety is goldmark's default posture, kept: no unsafe HTML, no dangerous URL +// schemes in links, everything that came out of the database escaped on its way +// to the browser. The one thing marked template.HTML is the finished document +// this file produced. + +// memoryLinkIndexKey carries the per-request link index into the parse. A +// goldmark.Markdown is stateless and shared; what varies per request is which +// databases this caller may browse, and that belongs in the parse context rather +// than in a second renderer built per page. +var memoryLinkIndexKey = parser.NewContextKey() + +// memoryMarkdown is the shared renderer. goldmark's own parsers and renderers +// are safe for concurrent use — all per-conversion state lives in the context — +// so this is built once and never rebuilt. +var memoryMarkdown = goldmark.New( + // GFM for the shapes the memories actually use: tables, strikethrough, task + // lists, and linkify — a bare https://dolt.srht.bigb.es/~bigbes/ in the + // prose is a URL the reader wants to follow. + goldmark.WithExtensions(extension.GFM), + goldmark.WithParserOptions( + // Ahead of the link parser (200), behind the task-list marker (0): "[[" is + // a wikilink before it is a link label. Registered as an inline parser and + // not as a text rewrite, so a "[[slug]]" written inside a code span is + // left alone by construction — inline parsers do not run in there. + parser.WithInlineParsers(util.Prioritized(wikilinkParser{}, 150)), + parser.WithASTTransformers(util.Prioritized(issueLinkTransformer{}, 900)), + ), + goldmark.WithRendererOptions( + renderer.WithNodeRenderers(util.Prioritized(memoryNodeRenderer{}, 100)), + ), +) + +// memoryLinks renders memory bodies for one request. It holds the link index — +// prefixes and memory slugs over the databases this caller may browse — and +// nothing else; a nil one still renders markdown, with every reference left as +// text, which is a whole answer and not a degraded one. +type memoryLinks struct { + index *beads.PrefixIndex +} + +// Body renders one memory's stored text as HTML. +// +// It is the only function here that produces template.HTML, and what it marks is +// the document goldmark built: every leaf that came out of the database is +// escaped by the renderer on its way in, including the slug inside a wikilink +// and the href built from it. +func (l *memoryLinks) Body(src string) template.HTML { + var index *beads.PrefixIndex + if l != nil { + index = l.index + } + pc := parser.NewContext() + if index != nil { + pc.Set(memoryLinkIndexKey, index) + } + var buf bytes.Buffer + if err := memoryMarkdown.Convert([]byte(src), &buf, parser.WithContext(pc)); err != nil { + // Convert fails only on a write, which this buffer cannot do. The memory is + // still shown, as the escaped text it was: a rendering that could not run is + // not a reason to answer with a blank pane. + return template.HTML(`

` + + template.HTMLEscapeString(src) + `

`) + } + return template.HTML(buf.String()) +} + +// memoryHref is the address of one memory in one database: the memory view +// narrowed to a single slug, which is the page a reference wants to land on. +func memoryHref(owner, name, slug string) string { + return "/~" + url.PathEscape(owner) + "/" + url.PathEscape(name) + + "/view/memory?key=" + url.QueryEscape(slug) +} + +// --- [[slug]] ------------------------------------------------------------------ + +// wikilink is a resolved or unresolved memory reference. Href is empty when no +// database this caller may browse holds a memory under that slug — which is +// deliberately the same node as one nobody ever wrote, so the rendering cannot +// disclose the existence of a database the caller may not see. +type wikilink struct { + ast.BaseInline + Href string +} + +var kindWikilink = ast.NewNodeKind("MemoryWikilink") + +func (n *wikilink) Kind() ast.NodeKind { return kindWikilink } + +func (n *wikilink) Dump(source []byte, level int) { + ast.DumpHelper(n, source, level, map[string]string{"Href": n.Href}, nil) +} + +// wikilinkParser turns "[[slug]]" into a wikilink node, resolving the slug +// against the request's index as it goes. +type wikilinkParser struct{} + +func (wikilinkParser) Trigger() []byte { return []byte{'['} } + +// Parse reads a wikilink out of the current line, or nothing at all: a "[[" with +// no closing "]]" on the same line, and anything whose slug is not slug-shaped, +// is left to the ordinary link parser and ends up as the text it was. Memory +// slugs are single-token keys — `bd remember --key` takes one — so a reference +// never spans a line. +func (wikilinkParser) Parse(_ ast.Node, block text.Reader, pc parser.Context) ast.Node { + line, _ := block.PeekLine() + if len(line) < 5 || line[0] != '[' || line[1] != '[' { + return nil + } + end := bytes.Index(line, []byte("]]")) + if end < 3 { + return nil + } + slug := string(line[2:end]) + if !isMemorySlug(slug) { + return nil + } + block.Advance(end + 2) + + node := &wikilink{} + if index, ok := pc.Get(memoryLinkIndexKey).(*beads.PrefixIndex); ok { + if d, found := index.LookupMemory(slug); found { + node.Href = memoryHref(d.OwnerName, d.Name, slug) + } + } + // The slug is carried as a string node rather than as a source segment: a + // line the reader hands back can be padded — a list item's continuation + // indent is synthesised, not sliced — and an offset into it is then not an + // offset into the source. The renderer escapes a string node exactly as it + // escapes every other leaf. + node.AppendChild(node, ast.NewString([]byte(slug))) + return node +} + +// memorySlugMax bounds what this will treat as a slug. `bd remember --key` takes +// a short name; a "[[" followed by half a paragraph and a "]]" is prose that +// happens to contain brackets. +const memorySlugMax = 128 + +// isMemorySlug is the shape a memory key has: the characters `bd remember --key` +// and the memory files' `name:` field use, and no others. It is deliberately +// narrower than "anything without brackets" — a bracketed aside is not a +// reference, and the difference has to be decidable without asking the index, +// since a slug nobody holds must render the same way whether or not it is one. +func isMemorySlug(s string) bool { + if s == "" || len(s) > memorySlugMax { + return false + } + for _, r := range s { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + case r == '-', r == '_', r == '.', r == '/': + default: + return false + } + } + return true +} + +// --- issue ids in memory prose ------------------------------------------------- + +// issueLinkTransformer links the issue ids in a memory's prose to the databases +// that own them, reusing the index this render already built for the wikilinks. +// +// It runs after inline parsing, over the text nodes only, and never descends +// into a code span, a link, an autolink or a wikilink: an id inside `code` is +// being shown rather than cited, and an id inside a link label would nest an +// anchor in an anchor. +type issueLinkTransformer struct{} + +func (issueLinkTransformer) Transform(doc *ast.Document, reader text.Reader, pc parser.Context) { + index, ok := pc.Get(memoryLinkIndexKey).(*beads.PrefixIndex) + if !ok || index == nil { + return + } + source := reader.Source() + + // Collected first and rewritten after: replacing a node during the walk that + // found it is how a walk starts stepping over its own edits. + var texts []*ast.Text + _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) { + if !entering { + return ast.WalkContinue, nil + } + switch n.Kind() { + case ast.KindLink, ast.KindImage, ast.KindAutoLink, ast.KindCodeSpan, + ast.KindRawHTML, ast.KindHTMLBlock, ast.KindCodeBlock, + ast.KindFencedCodeBlock, kindWikilink: + return ast.WalkSkipChildren, nil + case ast.KindText: + t := n.(*ast.Text) + // A raw text node is a code span's content, and a padded one carries a + // block indent its segment offsets do not describe. Neither can be cut + // on byte offsets taken from the source. + if !t.IsRaw() && t.Segment.Padding == 0 { + texts = append(texts, t) + } + } + return ast.WalkContinue, nil + }) + + for _, t := range texts { + linkIssueIDs(t, source, index) + } +} + +// linkIssueIDs replaces one text node with the sequence of text and link nodes +// its ids imply. A node with no id in it is left exactly as it was. +func linkIssueIDs(t *ast.Text, source []byte, index *beads.PrefixIndex) { + seg := t.Segment + refs := index.Scan(string(source[seg.Start:seg.Stop])) + if len(refs) == 0 { + return + } + parent := t.Parent() + if parent == nil { + return + } + + var nodes []ast.Node + last := seg.Start + for _, ref := range refs { + start, stop := seg.Start+ref.Start, seg.Start+ref.End + if start > last { + nodes = append(nodes, ast.NewTextSegment(text.NewSegment(last, start))) + } + link := ast.NewLink() + link.Destination = []byte(beadIssueHref( + ref.Database.OwnerName, ref.Database.Name, ref.ID)) + link.AppendChild(link, ast.NewTextSegment(text.NewSegment(start, stop))) + nodes = append(nodes, link) + last = stop + } + + // The tail carries the original node's line-break flags. When the id ran to + // the end of the node the tail is empty and is kept anyway: dropping it drops + // the newline, and the next line's first word would be glued to the id. + tail := ast.NewTextSegment(text.NewSegment(last, seg.Stop)) + tail.SetSoftLineBreak(t.SoftLineBreak()) + tail.SetHardLineBreak(t.HardLineBreak()) + nodes = append(nodes, tail) + + for _, n := range nodes { + parent.InsertBefore(parent, t, n) + } + parent.RemoveChild(parent, t) +} + +// --- the three nodes this rendering does not leave to goldmark ----------------- + +// memoryNodeRenderer registers the wikilink renderer and replaces goldmark's +// handling of raw HTML. +type memoryNodeRenderer struct{} + +func (memoryNodeRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) { + reg.Register(kindWikilink, renderWikilink) + reg.Register(ast.KindRawHTML, renderRawHTMLAsText) + reg.Register(ast.KindHTMLBlock, renderHTMLBlockAsText) + reg.Register(ast.KindImage, renderImageAsLink) +} + +// renderWikilink writes the anchor, or the muted marker for a slug no database +// this caller may browse holds. The marker says only that the reference does not +// resolve *here*; it cannot say more without disclosing what it must not. +func renderWikilink(w util.BufWriter, _ []byte, node ast.Node, entering bool) (ast.WalkStatus, error) { + n := node.(*wikilink) + switch { + case entering && n.Href != "": + _, _ = w.WriteString(``) + case entering: + _, _ = w.WriteString(``) + case n.Href != "": + _, _ = w.WriteString(``) + default: + _, _ = w.WriteString(``) + } + return ast.WalkContinue, nil +} + +// renderImageAsLink renders an image reference as a link to it rather than as an +// . +// +// An pointing at another host is a request that host makes on behalf of +// whoever opened the page, and a memory is prose one account wrote and another +// may read: an image in it would report the reader's address to a server the +// reader never chose to contact. The reference is kept and stays followable; it +// simply is not fetched by opening the page. +func renderImageAsLink(w util.BufWriter, _ []byte, node ast.Node, entering bool) (ast.WalkStatus, error) { + n := node.(*ast.Image) + if !entering { + _, _ = w.WriteString(``) + return ast.WalkContinue, nil + } + _, _ = w.WriteString(``) + return ast.WalkContinue, nil +} + +// renderRawHTMLAsText writes inline raw HTML as the escaped text it reads as. +// +// goldmark's safe default omits it, which is right for a comment feed and wrong +// here: `SRHT__VER` is a placeholder somebody typed, CommonMark sees +// `` as a tag, and omitting it turns the memory into a different sentence. +// Escaping shows what was written and is exactly as safe as omitting it. +func renderRawHTMLAsText(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) { + if !entering { + return ast.WalkSkipChildren, nil + } + n := node.(*ast.RawHTML) + for i := 0; i < n.Segments.Len(); i++ { + seg := n.Segments.At(i) + _, _ = w.Write(util.EscapeHTML(seg.Value(source))) + } + return ast.WalkSkipChildren, nil +} + +// renderHTMLBlockAsText is the same treatment for a block that opened with +// something tag-shaped: shown, escaped, in a paragraph of its own rather than +// dropped. +func renderHTMLBlockAsText(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) { + n := node.(*ast.HTMLBlock) + if !entering { + if n.HasClosure() { + _, _ = w.Write(util.EscapeHTML(n.ClosureLine.Value(source))) + } + _, _ = w.WriteString("

\n") + return ast.WalkContinue, nil + } + _, _ = w.WriteString(`

`) + for i := 0; i < n.Lines().Len(); i++ { + line := n.Lines().At(i) + _, _ = w.Write(util.EscapeHTML(line.Value(source))) + } + return ast.WalkContinue, nil +} diff --git a/web/markdown_test.go b/web/markdown_test.go new file mode 100644 index 0000000000000000000000000000000000000000..4e80c304754c4242b8fee169f696013bd66b1bf9 --- /dev/null +++ b/web/markdown_test.go @@ -0,0 +1,282 @@ +package web + +import ( + "errors" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-dolt/browse" + "sourcecraft.dev/bigbes/sr-ht-dolt/core" +) + +// --- the rendering itself ------------------------------------------------------ +// +// These render with no index at all — the nil receiver, which is what a page +// gets when the database listing failed. Everything asserted here is therefore +// about the markdown and about what is allowed to reach the browser, not about +// the links. + +// renderMemory is one memory body, rendered by a memoryLinks that knows no +// database. +func renderMemory(src string) string { + var noLinks *memoryLinks + return string(noLinks.Body(src)) +} + +// The shapes the memories are actually written in: a bold leader, code spans, a +// bullet list, a fenced recipe, and hard-wrapped prose that reflows into one +// paragraph rather than keeping the width its author's terminal had. +func TestMemoryBodyRendersMarkdown(t *testing.T) { + got := renderMemory("**Why:** the networks are declared `external: true` everywhere,\n" + + "and compose cannot create one.\n" + + "\n" + + "- first, `./prepare.sh`\n" + + "- then `labng push backup`\n" + + "\n" + + "```\n" + + "dolt clone https://dolt.srht.bigb.es/~bigbes/x\n" + + "```\n") + + assert.Contains(t, got, "Why:") + assert.Contains(t, got, "external: true") + assert.Contains(t, got, "

    ") + assert.Contains(t, got, "
  • first, ./prepare.sh
    • ") + assert.Contains(t, got, "
    dolt clone")
+	assert.Contains(t, got, "everywhere,\nand compose cannot create one.",
+		"a hard-wrapped paragraph reflows: the source's line breaks are not the document's")
+	assert.NotContains(t, got, "**", "the asterisks are markup and must not survive as text")
+}
+
+// A memory is stored text and is escaped on its way out, exactly as it was when
+// the page printed paragraphs. Markup written in one renders as the characters
+// somebody typed and never as an element.
+func TestMemoryBodyEscapesStoredMarkup(t *testing.T) {
+	got := renderMemory("Supersedes the memory that asserted otherwise.\n" +
+		"\n" +
+		"\n")
+
+	assert.NotContains(t, got, "otherwise")
+	assert.Contains(t, got, "<b>otherwise</b>")
+	assert.NotContains(t, got, "