diff --git a/modules/mesh-issues/README.md b/modules/mesh-issues/README.md index 71aa3743..3c527096 100644 --- a/modules/mesh-issues/README.md +++ b/modules/mesh-issues/README.md @@ -22,16 +22,66 @@ what it refuses. | `move` | `number`, `status`, and what that status needs | changes the status under the development cycle's rules | | `note` | `number`, `text` | appends a note; a note is never edited | | `link` | `number`, `kind`, `target`, optionally `remove` | adds or removes a typed link | +| `import` | optionally `dry-run`, `commit` | hq's issue folders, read through the records module, each written as an issue under its own number; see below | -**The cycle's rules, at the write** (hq `00-META/checks/cycle.py`): a status is one of `open`, +**The cycle's rules, at the write** (hq playbook 03, which `00-META/checks/cycle.py` enforced on the +folders until the import): a status is one of `open`, `diagnosing`, `located`, `resolved`, `wontfix`; `located` needs `located-in`; `resolved` needs `located-in` and `fixed-by`; a core issue (located in the controller, the node-engine, the tool runner, the SDK, or the catalogue's bus, forge or build agent) resolves with `replay` or `replay-none`; `wontfix` needs a `reason`. A refused change changes nothing and says which rule refused it. -**An issue with a folder in hq** (`record` at open: the folder's name, without the number) keeps its -status in that folder's report until the import of slice 3. The tracker holds its number, title, folder, -notes and links, and `move` refuses it, saying where its status lives. +**An issue with a folder in hq** (`record` at open: the folder's name, without the number) is whole in the +tracker like any other: the folder holds its narrative, the tracker its status. Until the import of +slice 3 ran, such an issue kept its status in the folder and `move` refused it; the import filled it. + +## The import of hq's issues (slice 3) + +`issue-tracker.import` reads every `04-ISSUES/NNN-slug/00-report.md` of hq **through the records module +over the bus** (`records_status`, `records_list`, `records_read`, called as this module through the SDK's +`Ask`), at the commit the records module's checkout stands at, and writes each as an issue **at its own +number**: `record` is the folder, the title is the report's heading, the report is the text under it, and +`status`, `opened`, `located-in`, `fixed-by`, `amended-design`, `replay` and `replay-none` are the +frontmatter's, as written — the status one to one, no cycle rule applied, because the folder's status is +the snapshot hq kept. The issues and decision records the report names (`issue 155`, `ADR 0275`, +`04-ISSUES/203-…`, `02-DECISIONS/0019-…`) become `relates-to` and `decision` links. + +It is a verb on the seat, callable by any agent, because it can do only this: + +- **It only creates an issue that is not there** (`create` on the key, refused when the key exists) **or + fills the status of one the tracker holds without a status** — an issue opened with a folder before this + slice, whose folder kept its status; one whose folder hq does not hold at the commit becomes `open`. +- **It never overwrites.** An issue the tracker holds differently is reported as `differs`, naming the + fields, and left alone: the tracker is the truth. An issue it holds the same is `same`. So a second run + changes nothing and says so. +- **It never touches the counter**, and refuses the whole run when a folder is numbered past the counter. + A folder numbered at or past the setting `first-number` without an issue in the tracker is refused on its + own: a folder from the tracker's time carries a number the tracker gave. +- **It cannot forge an issue's history.** An issue it writes is `opened-by: import`, its `opened` the date + the report gave, and it carries `imported: {at, by, commit, from}` — when, who ran the import (the bus + credential, `unproven` until the runtime proves it), the commit of hq and the document. Each is an + `imported` event. What it writes comes from hq's `main` as the records module holds it, which is reviewed + in pull requests. +- **What it keeps is bounded** as every verb's text is: the title 200 bytes, the report 64 KiB, one issue + 256 KiB; a report that trips a credential's shape is refused. An entry of `located-in` or `fixed-by` hq + wrote is prose at times, so it is taken within a reason's bound (2000 bytes), not a line's. +- **An owner hq named that the registered list does not know is kept as written and marked**, in + `located-in-unverified` on the issue and `unverified-owners` in the answer, never refused: the list is + the setting's problem (hq issue 366), and a `move` that gives `located-in` anew checks every entry and + clears the mark. +- **With `dry-run`** it reads everything, writes nothing, and answers the same rows with `would-` on each + action. **With `commit`** it refuses a checkout standing elsewhere; it refuses a run during which the + checkout moved. + +The answer is a row per folder — number, action (`create`, `fill`, `same`, `differs`, `refused`, with why), +status, title — the counts, the counts by status, what was odd (a title numbered otherwise than its +folder, a frontmatter key not imported, an `amended-design` that is no design document) and the unverified +owners. A call that outlives the tool runner's wait is still running: the import goes on, and the answer +is read back through the controller's `calls` verb by the call's id. + +Before the module is rolled out, `mesh-issues dry-run ` at the terminal runs +the same dry run over a checkout on disk against an empty tracker, and prints the answer. It is not a way +to import: only the verb writes, and it reads the record over the bus. **Every argument is read strictly.** One a verb does not take is refused, never dropped. Every field has a size limit, a one-line field takes no control character, a folder's name is no path, and text shaped like @@ -64,8 +114,8 @@ Two states, declared in the manifest and made by the controller; the module neve (ADR 0201): `issues`, one key per issue by number, keeping 8 values per key; `counters`, one value per key. Neither expires. One issue is kept in at most 256 KiB; a note that would pass it is refused. -Every change is an event: `mesh-issues.opened`, `.moved`, `.noted`, `.linked`, each with the number, what -changed, who changed it and when. The events are the audit trail. A change whose event the bus did not take +Every change is an event: `mesh-issues.opened`, `.moved`, `.noted`, `.linked`, `.imported`, each with the +number, what changed, who changed it and when. The events are the audit trail. A change whose event the bus did not take is kept, and its answer says the event was not said (`announced: false`). ## Who acted @@ -85,7 +135,9 @@ verb takes an argument naming a caller, and none can be given one. ``` - `first-number` — the number the first issue gets: the one after the highest issue number on hq's main - and every open branch at the moment the module is switched on. Read once. + and every open branch at the moment the module is switched on. Read once for the counter; the import + reads it as the boundary between the folders hq numbered before the tracker and the numbers the tracker + gave. - `owners` — the registered list `located-in` may name: repositories, and modules as `mesh-catalog modules/`. An entry of `located-in` is an owner, or an owner followed by a space or a slash and where in it (`mesh-controller internal/broker`, `mesh-catalog modules/nats/module.json`). A module @@ -102,3 +154,10 @@ the test, two instances of this bundle (the test binary started again) speaking test against the buckets the manifest declares, fifty opens at once, fifty distinct numbers from the first in a row. It counts the writes lost to another opener, so it fails when the openers never collided. With the counter written without its revision check, it fails. + +The import is tested over `testdata/hq`, copies of seven real reports of hq chosen for their shapes (a +bracketed list with commas inside parentheses, quoted block items, an apostrophe in prose, a `resolved:` +date, a replay, an empty list, a wontfix): the frontmatter read as written, each issue created at its own +number with the counter untouched, a second run changing nothing, an issue the tracker holds differently +left alone, a folder-backed issue filled once, a folder past the counter or past the first number refused, +a dry run writing nothing, and a report that cannot be read refused alone with why. diff --git a/modules/mesh-issues/cmd/mesh-issues/import.go b/modules/mesh-issues/cmd/mesh-issues/import.go new file mode 100644 index 00000000..5c6e6f95 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/import.go @@ -0,0 +1,683 @@ +package main + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "regexp" + "sort" + "strings" + "sync" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" +) + +// The import of slice 3 (novox/hq ADR 0275 §8, to-be 53 §8): every issue folder in hq, read through the +// records module over the bus, written as an issue under its own number. It only ever creates an issue +// that is not there, or fills the status of one the tracker holds without a status (opened with a folder +// before this slice); it never overwrites, never advances the counter, and run twice changes nothing and +// says so. A dry run reads everything, writes nothing, and says what it would do. + +// Records is the record as the records module serves it (to-be 53 §8: "read through the records +// module"): where its checkout stands, what a folder holds, and one document whole. +type Records interface { + // Standing is the commit the checkout is at. + Standing() (string, error) + // List is a folder's sub-folders and documents. + List(folder string) (folders, documents []string, err error) + // Read is one document and the commit it was read at. + Read(path string) (content, commit string, err error) +} + +// busRecords is Records over the bus: the records module's tools, called as this module (SDK Ask). +type busRecords struct{} + +func (busRecords) Standing() (string, error) { + raw, err := stdio.Ask("records.records_status", map[string]any{}) + if err != nil { + return "", err + } + var s struct { + Commit string `json:"commit"` + LastError string `json:"lastError"` + } + if err := json.Unmarshal(raw, &s); err != nil { + return "", fmt.Errorf("records_status answered something else: %v", err) + } + if s.Commit == "" { + return "", fmt.Errorf("the record's checkout stands at no commit yet (%s)", s.LastError) + } + return s.Commit, nil +} + +func (busRecords) List(folder string) ([]string, []string, error) { + raw, err := stdio.Ask("records.records_list", map[string]any{"folder": folder}) + if err != nil { + return nil, nil, err + } + var f struct { + Folders []string `json:"folders"` + Documents []string `json:"documents"` + } + if err := json.Unmarshal(raw, &f); err != nil { + return nil, nil, fmt.Errorf("records_list answered something else: %v", err) + } + return f.Folders, f.Documents, nil +} + +func (busRecords) Read(path string) (string, string, error) { + raw, err := stdio.Ask("records.records_read", map[string]any{"path": path}) + if err != nil { + return "", "", err + } + var d struct { + Content string `json:"content"` + Commit string `json:"commit"` + Truncated bool `json:"truncated"` + } + if err := json.Unmarshal(raw, &d); err != nil { + return "", "", fmt.Errorf("records_read answered something else: %v", err) + } + if d.Truncated { + return "", "", fmt.Errorf("%s is longer than the records module answers whole", path) + } + return d.Content, d.Commit, nil +} + +// dirRecords is Records over a checkout on disk: the terminal's dry run before the module is rolled out, +// and the tests. Its commit is `dir:` and the directory, as no forge stands behind it. +type dirRecords struct{ dir string } + +func (d dirRecords) Standing() (string, error) { + if st, err := os.Stat(d.dir); err != nil || !st.IsDir() { + return "", fmt.Errorf("%s is not a checkout", d.dir) + } + if out, err := os.ReadFile(filepath.Join(d.dir, ".git", "HEAD")); err == nil { + ref := strings.TrimSpace(string(out)) + if strings.HasPrefix(ref, "ref: ") { + if sha, err := os.ReadFile(filepath.Join(d.dir, ".git", strings.TrimPrefix(ref, "ref: "))); err == nil { + return strings.TrimSpace(string(sha)), nil + } + } else if ref != "" { + return ref, nil + } + } + return "dir:" + d.dir, nil +} + +func (d dirRecords) List(folder string) ([]string, []string, error) { + entries, err := os.ReadDir(filepath.Join(d.dir, filepath.FromSlash(folder))) + if err != nil { + return nil, nil, err + } + var folders, docs []string + for _, e := range entries { + switch { + case e.IsDir(): + folders = append(folders, e.Name()) + case strings.HasSuffix(e.Name(), ".md"): + docs = append(docs, e.Name()) + } + } + return folders, docs, nil +} + +func (d dirRecords) Read(path string) (string, string, error) { + raw, err := os.ReadFile(filepath.Join(d.dir, filepath.FromSlash(path))) + if err != nil { + return "", "", err + } + commit, _ := d.Standing() + return string(raw), commit, nil +} + +// IssuesFolder is where hq keeps its issue folders. +const IssuesFolder = "04-ISSUES" + +// readers is how many documents are read from the record at once. +const readers = 8 + +// commitPrefix is what the commit argument may be: a commit's id, or a prefix of at least seven digits. +var commitPrefix = regexp.MustCompile(`^[0-9a-f]{7,40}$`) + +// errRecordMoved is the record's checkout moving to another commit while the import read it: the folders +// read would be of two commits, so nothing is written. +var errRecordMoved = errors.New("the record moved while it was read") + +// EventImported is the event said for each issue the import wrote or filled. +const EventImported = "imported" + +// ImportAsk is what an import carries. +type ImportAsk struct { + // Commit, when given, is the commit of hq the record must stand at; the import refuses another. + Commit string + // DryRun reads everything, writes nothing, and says what it would do. + DryRun bool +} + +// The actions an import takes on one folder. +const ( + ActionCreate = "create" // the tracker holds no issue under the number: written + ActionFill = "fill" // the tracker holds the issue without a status: its status and fields taken + ActionSame = "same" // the tracker holds the issue as the folder has it: nothing to do + ActionDiffers = "differs" // the tracker holds the issue differently: the tracker is the truth, nothing done + ActionRefused = "refused" // the folder could not be imported, and why + ActionWould = "would-" // prefixed to create and fill in a dry run +) + +// ImportRow is what the import did, or would do, with one folder. +type ImportRow struct { + Number uint64 `json:"number"` + Action string `json:"action"` + Status string `json:"status,omitempty"` + Title string `json:"title,omitempty"` + Folder string `json:"folder"` + Why string `json:"why,omitempty"` +} + +// Imported is the answer to an import. +type Imported struct { + DryRun bool `json:"dry-run"` + Commit string `json:"commit"` + Folders int `json:"folders"` + Created int `json:"created"` + Filled int `json:"filled"` + Same int `json:"same"` + Differs int `json:"differs"` + Refused int `json:"refused"` + ByStatus map[string]int `json:"by-status"` + Rows []ImportRow `json:"rows"` + Odd []string `json:"odd,omitempty"` + // UnverifiedOwners is each entry of located-in that named no registered owner, with the issues it is in. + UnverifiedOwners map[string][]uint64 `json:"unverified-owners,omitempty"` + Announced int `json:"announced"` + Said string `json:"said"` + Warning string `json:"warning,omitempty"` +} + +// candidate is one folder read and checked, before the tracker is asked what it holds. +type candidate struct { + folder string + number uint64 + report *Report + issue *Issue + // unverified is the entries of located-in that named no registered owner. + unverified []string + err error +} + +// Import reads every issue folder of hq through the record and writes each as an issue under its own +// number: create-if-absent on the key, the counter untouched. See the file's comment for what it never +// does. +func (t *Tracker) Import(a ImportAsk, records Records) (*Imported, error) { + settings, err := t.Settings() + if err != nil { + return nil, fmt.Errorf("the settings cannot be read (%v); nothing was imported", err) + } + if settings.FirstNumber < 1 { + return nil, errors.New("the setting first-number is not set: it is the boundary between the folders numbered " + + "before the tracker and the issues the tracker numbered itself. Nothing was imported") + } + owners := Owners(settings.Owners) + commit, err := records.Standing() + if err != nil { + return nil, fmt.Errorf("the record cannot be read (%v); nothing was imported", err) + } + want := strings.TrimSpace(a.Commit) + if want != "" && !commitPrefix.MatchString(want) { + return nil, errors.New("commit is a commit's id or a prefix of it, 7 to 40 hex digits; nothing was imported") + } + if want != "" && !strings.HasPrefix(commit, want) { + return nil, fmt.Errorf("the record stands at hq commit %s, not %s: bring it up to date (records_sync), or "+ + "name the commit it stands at. Nothing was imported", short(commit), want) + } + folders, _, err := records.List(IssuesFolder) + if err != nil { + return nil, fmt.Errorf("the record's %s cannot be listed (%v); nothing was imported", IssuesFolder, err) + } + out := &Imported{DryRun: a.DryRun, Commit: commit, ByStatus: map[string]int{}, Rows: []ImportRow{}} + at := t.now() + by := t.by() + + // Every folder's number, and no two folders sharing one. + var cands []*candidate + byNumber := map[uint64][]string{} + for _, f := range folders { + n, ok := folderNumber(f) + if !ok { + out.Odd = append(out.Odd, fmt.Sprintf("%s/%s is not an issue folder (NNN-slug) and was skipped", IssuesFolder, f)) + continue + } + byNumber[n] = append(byNumber[n], f) + cands = append(cands, &candidate{folder: f, number: n}) + } + out.Folders = len(cands) + for _, c := range cands { + if len(byNumber[c.number]) > 1 { + c.err = fmt.Errorf("two folders are numbered %d: %s — a number names one issue", c.number, strings.Join(byNumber[c.number], " and ")) + } + } + + // The issues a report may link: the folders read, and the issues the tracker keeps. + exists := map[uint64]bool{} + for _, c := range cands { + exists[c.number] = true + } + if keys, err := t.Store.Keys(IssuesState); err == nil { + for _, k := range keys { + if n, ok := numberOfKey(k); ok { + exists[n] = true + } + } + } else { + return nil, fmt.Errorf("the issues kept cannot be listed (%v); nothing was imported", err) + } + + // Read and check every report, a few at a time. + var wg sync.WaitGroup + sem := make(chan struct{}, readers) + for _, c := range cands { + if c.err != nil { + continue + } + wg.Add(1) + sem <- struct{}{} + go func(c *candidate) { + defer wg.Done() + defer func() { <-sem }() + path := IssuesFolder + "/" + c.folder + "/00-report.md" + text, readAt, err := records.Read(path) + if err != nil { + c.err = fmt.Errorf("%s could not be read: %v", path, err) + return + } + if readAt != commit { + c.err = fmt.Errorf("%w: it stood at %s and %s was read at %s. Run the import again", + errRecordMoved, short(commit), path, short(readAt)) + return + } + c.report, c.err = ParseReport(c.folder, text) + if c.err != nil { + return + } + c.issue, c.unverified, c.err = t.issueOf(c.report, owners, commit, at, by, exists) + }(c) + } + wg.Wait() + for _, c := range cands { + if errors.Is(c.err, errRecordMoved) { + return nil, fmt.Errorf("%v; nothing was imported", c.err) + } + } + + // The counter is never passed: a number the counter has not handed out is the counter's to hand out. + var counter uint64 + if e, err := t.Store.Get(CountersState, IssueCounter); err != nil { + return nil, fmt.Errorf("the issue counter cannot be read (%v); nothing was imported", err) + } else if e != nil { + if err := json.Unmarshal(e.Value, &counter); err != nil { + return nil, fmt.Errorf("the issue counter holds %s, not a number; nothing was imported", e.Value) + } + for _, c := range cands { + if c.err == nil && c.number > counter { + return nil, fmt.Errorf("the folder %s is numbered past the counter, which stands at %d: a number the counter "+ + "has not handed out is not a folder's to take. Nothing was imported", c.folder, counter) + } + } + } + + sort.Slice(cands, func(i, j int) bool { return cands[i].number < cands[j].number }) + unverified := map[string][]uint64{} + for _, c := range cands { + row := ImportRow{Number: c.number, Folder: IssuesFolder + "/" + c.folder} + if c.report != nil { + row.Status, row.Title = c.report.Status, c.report.Title + for _, o := range c.report.Odd { + out.Odd = append(out.Odd, fmt.Sprintf("%d: %s", c.number, o)) + } + } + if c.err == nil { + c.err = t.importOne(c, settings.FirstNumber, a.DryRun, &row, out) + } + if c.err != nil { + row.Action, row.Why = ActionRefused, c.err.Error() + } + switch row.Action { + case ActionRefused: + out.Refused++ + case ActionSame: + out.Same++ + case ActionDiffers: + out.Differs++ + default: + if strings.HasSuffix(row.Action, ActionFill) { + out.Filled++ + } else { + out.Created++ + } + out.ByStatus[row.Status]++ + for _, u := range c.unverified { + unverified[u] = append(unverified[u], c.number) + } + } + out.Rows = append(out.Rows, row) + } + if len(unverified) > 0 { + out.UnverifiedOwners = unverified + } + + // An issue the tracker holds without a status, whose folder hq does not hold at this commit (opened with + // a folder before slice 3, the folder not merged yet): it is open, so no issue is left without a status. + folderOf := map[uint64]bool{} + for _, c := range cands { + folderOf[c.number] = true + } + keys, err := t.Store.Keys(IssuesState) + if err != nil { + return nil, fmt.Errorf("the issues kept cannot be listed (%v); what was imported before this is kept", err) + } + for _, k := range keys { + n, ok := numberOfKey(k) + if !ok || folderOf[n] { + continue + } + is, _, err := read(t.Store, n) + if err != nil || is.Status != "" { + continue + } + row := ImportRow{Number: n, Action: ActionWould + ActionFill, Status: StatusOpen, Title: is.Title, Folder: is.Record, + Why: "the tracker holds it without a status and hq holds no folder for it at this commit: open"} + if !a.DryRun { + row.Action = ActionFill + if err := t.fillOpen(n, commit, at, by); err != nil { + row.Action, row.Why = ActionRefused, err.Error() + out.Refused++ + out.Rows = append(out.Rows, row) + continue + } + } + out.Filled++ + out.ByStatus[StatusOpen]++ + out.Rows = append(out.Rows, row) + } + verb := "created" + if a.DryRun { + verb = "would create" + } + out.Said = fmt.Sprintf("Read %d issue folders of hq at commit %s: %s %d, filled %d, the same %d, differing %d, refused %d.", + out.Folders, short(commit), verb, out.Created, out.Filled, out.Same, out.Differs, out.Refused) + if a.DryRun { + out.Said += " Nothing was written: a dry run." + } + if out.Folders > 0 && out.Created == 0 && out.Filled == 0 && !a.DryRun { + if out.Refused == 0 { + out.Said += " Nothing changed: every folder is already in the tracker." + } else { + out.Said += " Nothing changed: every folder not refused is already in the tracker." + } + } + if !a.DryRun && out.Announced < out.Created+out.Filled { + out.Warning = fmt.Sprintf("%d of %d changes are kept, and their %s event could not be said on the bus: what watches "+ + "the events did not hear of them", out.Created+out.Filled-out.Announced, out.Created+out.Filled, EventImported) + } + return out, nil +} + +// issueOf is the issue a report becomes: the fields as the folder has them, checked for size and shape as +// `open` and `move` check them — but located-in is kept as written even when it names no registered +// owner, marked unverified, and no cycle rule is applied: the status is the folder's, as it stood. +func (t *Tracker) issueOf(r *Report, owners Owners, commit string, at time.Time, by string, exists map[uint64]bool) (*Issue, []string, error) { + title, err := singleLine("title", r.Title, MostTitle) + if err != nil { + return nil, nil, err + } + body := r.Body + if strings.TrimSpace(body) == "" { + body = "(the report says nothing beyond its title)" + } + report, err := multiLine("report", body, MostReport) + if err != nil { + return nil, nil, err + } + var unverified []string + // An entry hq wrote is prose at times, longer than one typed at a verb: it is taken whole, within a reason's bound. + located, err := entries("located-in", r.LocatedIn, func(e string) (string, error) { + v, err := singleLine("an entry of located-in", e, MostReason) + if err != nil { + return "", err + } + if owners.owns(v) == "" { + unverified = append(unverified, v) + } + return v, nil + }) + if err != nil { + return nil, nil, err + } + fixed, err := entries("fixed-by", r.FixedBy, func(e string) (string, error) { return singleLine("an entry of fixed-by", e, MostReason) }) + if err != nil { + return nil, nil, err + } + designs, err := entries("amended-design", r.AmendedDesign, func(e string) (string, error) { + v, err := singleLine("an entry of amended-design", e, MostReason) + if err != nil { + return "", err + } + if !designPath.MatchString(v) { + r.Odd = append(r.Odd, fmt.Sprintf("amended-design %q is not a design document's path; kept as written", v)) + } + return v, nil + }) + if err != nil { + return nil, nil, err + } + var replay, replayNone string + if r.Replay != "" { + if replay, err = singleLine("replay", r.Replay, MostLine); err != nil { + return nil, nil, err + } + if !replayID.MatchString(replay) { + r.Odd = append(r.Odd, fmt.Sprintf("replay %q names no replay id (R and a number); kept as written", replay)) + } + } + if r.ReplayNone != "" { + if replayNone, err = multiLine("replay-none", r.ReplayNone, MostReason); err != nil { + return nil, nil, err + } + } + is := &Issue{Number: r.Number, Title: title, Status: r.Status, Report: report, Opened: at, OpenedBy: ImportedBy, + LocatedIn: located, LocatedInUnverified: unverified, FixedBy: fixed, AmendedDesign: designs, + Replay: replay, ReplayNone: replayNone, Record: r.Folder, Opening: "import@" + short(commit), + Imported: &Import{At: at, By: by, Commit: commit, From: r.Path}} + if d, err := time.Parse("2006-01-02", r.Opened); err == nil { + is.Opened = d + } else { + r.Odd = append(r.Odd, "no opened date: the import's time is taken") + } + if d, err := time.Parse("2006-01-02", r.Closed); err == nil { + is.Closed = &d + } + for _, other := range r.Issues { + if !exists[other] { + r.Odd = append(r.Odd, fmt.Sprintf("names issue %d, which is neither a folder nor an issue kept; not linked", other)) + continue + } + is.Links = append(is.Links, Link{Kind: LinkRelatesTo, Target: keyOf(other), By: ImportedBy, At: at}) + } + for _, d := range r.Decisions { + is.Links = append(is.Links, Link{Kind: LinkDecision, Target: d, By: ImportedBy, At: at}) + } + if len(is.Links) > MostLinks { + is.Links = is.Links[:MostLinks] + r.Odd = append(r.Odd, fmt.Sprintf("names more than %d issues and decisions; the first %d are linked", MostLinks, MostLinks)) + } + raw, err := json.Marshal(is) + if err != nil { + return nil, nil, err + } + if len(raw) > MostIssueBytes { + return nil, nil, fmt.Errorf("the issue would be %d bytes; one issue is kept in at most %d", len(raw), MostIssueBytes) + } + return is, unverified, nil +} + +// importOne does what the tracker's state asks for one folder: create, fill, nothing, or refuse. +func (t *Tracker) importOne(c *candidate, first uint64, dry bool, row *ImportRow, out *Imported) error { + kept, _, err := read(t.Store, c.number) + if err != nil && !errors.Is(err, ErrNoIssue) { + return err + } + prefix := "" + if dry { + prefix = ActionWould + } + switch { + case kept == nil && c.number >= first: + return fmt.Errorf("numbered %d, at or past the tracker's first number %d, and the tracker holds no issue %d: "+ + "a folder from the tracker's time carries a number the tracker gave, so this one was made without it", + c.number, first, c.number) + case kept == nil: + row.Action = prefix + ActionCreate + if dry { + return nil + } + return t.importCreate(c.issue, out) + case kept.Status == "" && kept.Record == c.issue.Record: + row.Action = prefix + ActionFill + if kept.Title != c.issue.Title { + out.Odd = append(out.Odd, fmt.Sprintf("%d: the tracker's title %q and the folder's %q differ; the tracker's is kept", c.number, kept.Title, c.issue.Title)) + } + if dry { + return nil + } + return t.importFill(c.issue, out) + case kept.Status == "": + row.Action = ActionDiffers + row.Why = fmt.Sprintf("the tracker holds issue %d without a status under the folder %s, and this folder is %s: "+ + "nothing was changed. If they are one issue, move it in the tracker; its status is not taken from a folder "+ + "that is not its own", c.number, kept.Record, c.issue.Record) + return nil + default: + differs := differences(kept, c.issue) + if len(differs) == 0 { + row.Action = ActionSame + return nil + } + row.Action = ActionDiffers + row.Why = "the tracker holds it differently in " + strings.Join(differs, ", ") + "; the tracker is the truth, nothing was changed" + return nil + } +} + +// differences is the fields in which the tracker's issue and the folder's differ: the fields the import +// sets, for an issue the import wrote; only the status and what it needs, for one the tracker opened. +func differences(kept, folder *Issue) []string { + var d []string + same := func(name string, a, b any) { + if !reflect.DeepEqual(a, b) { + d = append(d, name) + } + } + if kept.OpenedBy == ImportedBy { + same("title", kept.Title, folder.Title) + same("report", kept.Report, folder.Report) + same("record", kept.Record, folder.Record) + } + same("status", kept.Status, folder.Status) + same("located-in", kept.LocatedIn, folder.LocatedIn) + same("fixed-by", kept.FixedBy, folder.FixedBy) + same("amended-design", kept.AmendedDesign, folder.AmendedDesign) + same("replay", kept.Replay, folder.Replay) + same("replay-none", kept.ReplayNone, folder.ReplayNone) + return d +} + +// importCreate writes a new issue under its number, never over one. +func (t *Tracker) importCreate(is *Issue, out *Imported) error { + _, err := t.Store.Create(IssuesState, keyOf(is.Number), is) + if errors.Is(err, stdio.ErrChanged) { + return fmt.Errorf("an issue %d appeared while the import ran; nothing was overwritten. Run the import again", is.Number) + } + if err != nil { + // A write that timed out may still have landed: read back before saying it did not. + if there, _, rerr := read(t.Store, is.Number); rerr == nil && there.Opening == is.Opening { + t.announceImport(is, out) + return nil + } + return fmt.Errorf("issue %d could not be written: %v", is.Number, err) + } + t.announceImport(is, out) + return nil +} + +// importFill takes the status and the fields of an issue the tracker holds without a status: opened with a +// folder before slice 3, whose status lived in that folder. Its title, report, opening, notes and links +// stay; links the report names are added. +func (t *Tracker) importFill(from *Issue, out *Imported) error { + is, err := t.change(from.Number, func(is *Issue) error { + if is.Status != "" { + return fmt.Errorf("issue %d took a status while the import ran; nothing was changed", is.Number) + } + is.Status = from.Status + is.Closed = from.Closed + is.LocatedIn, is.LocatedInUnverified = from.LocatedIn, from.LocatedInUnverified + is.FixedBy, is.AmendedDesign = from.FixedBy, from.AmendedDesign + is.Replay, is.ReplayNone = from.Replay, from.ReplayNone + is.Imported = from.Imported + for _, l := range from.Links { + has := false + for _, k := range is.Links { + if k.Kind == l.Kind && k.Target == l.Target { + has = true + } + } + if !has && len(is.Links) < MostLinks { + is.Links = append(is.Links, l) + } + } + return nil + }) + if err != nil { + return err + } + t.announceImport(is, out) + return nil +} + +func (t *Tracker) announceImport(is *Issue, out *Imported) { + ok, _ := t.emit(EventImported, map[string]any{"number": is.Number, "title": is.Title, "status": is.Status, + "record": is.Record, "commit": is.Imported.Commit, "by": is.Imported.By, "at": is.Imported.At}) + if ok { + out.Announced++ + } +} + +// fillOpen gives an issue the tracker holds without a status — opened with a folder before slice 3, whose +// folder hq does not hold at the import's commit — the status open, so no issue is left without one. +func (t *Tracker) fillOpen(n uint64, commit string, at time.Time, by string) error { + is, err := t.change(n, func(is *Issue) error { + if is.Status != "" { + return fmt.Errorf("issue %d took a status while the import ran; nothing was changed", is.Number) + } + is.Status = StatusOpen + is.Imported = &Import{At: at, By: by, Commit: commit} + return nil + }) + if err != nil { + return err + } + _, _ = t.emit(EventImported, map[string]any{"number": is.Number, "title": is.Title, "status": is.Status, + "record": is.Record, "commit": commit, "by": by, "at": at}) + return nil +} + +func short(commit string) string { + if len(commit) > 12 && !strings.HasPrefix(commit, "dir:") { + return commit[:12] + } + return commit +} diff --git a/modules/mesh-issues/cmd/mesh-issues/import_test.go b/modules/mesh-issues/cmd/mesh-issues/import_test.go new file mode 100644 index 00000000..46b73449 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/import_test.go @@ -0,0 +1,381 @@ +package main + +import ( + "errors" + "os" + "path/filepath" + "reflect" + "strings" + "sync" + "testing" +) + +// The import of slice 3 (to-be 53 §8) over the real reports in testdata/hq, against the tracker kept in +// this process: it creates each issue at its own number, touches no counter, never overwrites, and run +// again changes nothing and says so. + +// sampleRecords is the record played from testdata/hq, at a commit the test names. +type sampleRecords struct { + dirRecords + commit string + // moveTo, when set, is the commit every read after the first answers: the record moving mid-import. + moveTo string + mu sync.Mutex + reads int +} + +func newSample(t *testing.T) *sampleRecords { + t.Helper() + return &sampleRecords{dirRecords: dirRecords{dir: filepath.Join("testdata", "hq")}, commit: "1ea678c6bd48f62a45db3b5e4b21b9c46867d31a"} +} + +func (s *sampleRecords) Standing() (string, error) { return s.commit, nil } +func (s *sampleRecords) Read(path string) (string, string, error) { + content, _, err := s.dirRecords.Read(path) + if err != nil { + return "", "", err + } + s.mu.Lock() + defer s.mu.Unlock() + s.reads++ + if s.moveTo != "" && s.reads > 1 { + return content, s.moveTo, nil + } + return content, s.commit, nil +} + +var sampleNumbers = []uint64{7, 43, 80, 150, 236, 345, 362} + +func TestTheImportCreatesEveryFolderAtItsOwnNumberAndLeavesTheCounterAlone(t *testing.T) { + h := newHarness(t) + rec := newSample(t) + out, err := h.Import(ImportAsk{Commit: "1ea678c"}, rec) + if err != nil { + t.Fatal(err) + } + if out.Folders != 7 || out.Created != 7 || out.Filled+out.Same+out.Differs+out.Refused != 0 || out.DryRun { + t.Fatalf("%+v", out) + } + if !reflect.DeepEqual(out.ByStatus, map[string]int{StatusResolved: 5, StatusDiagnosing: 1, StatusWontfix: 1}) { + t.Fatalf("by status %v", out.ByStatus) + } + if out.Announced != 7 || len(h.said()) != 7 || h.said()[0].Event != EventImported { + t.Fatalf("announced %d, said %v", out.Announced, h.said()) + } + for i, n := range sampleNumbers { + if out.Rows[i].Number != n || out.Rows[i].Action != ActionCreate { + t.Fatalf("row %d: %+v", i, out.Rows[i]) + } + } + if h.mem.counter(t) != 0 { + t.Fatalf("the import moved the counter to %d", h.mem.counter(t)) + } + // A report names only issues that exist: 362's report names issues the sample does not hold, which are + // said and not linked; its links to decisions stay. + if is, _ := h.Get(362); len(is.Links) == 0 { + t.Fatalf("362 has no links: %+v", is.Links) + } + for _, l := range (func() *Issue { is, _ := h.Get(362); return is })().Links { + if l.Kind == LinkRelatesTo { + n, _ := numberOfKey(l.Target) + if _, err := h.Get(n); err != nil { + t.Fatalf("362 links issue %d, which does not exist", n) + } + } + } + // The acceptance: get shows what the folder said, and who/when is the import, with the commit. + is, err := h.Get(362) + if err != nil { + t.Fatal(err) + } + if is.Status != StatusResolved || is.Record != "04-ISSUES/362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person" || + len(is.LocatedIn) != 2 || is.OpenedBy != ImportedBy || is.Opened.Format("2006-01-02") != "2026-10-10" || + is.Imported == nil || is.Imported.Commit != rec.commit || is.Imported.From != is.Record+"/00-report.md" || + is.Imported.By != Unproven || is.Imported.At != h.now() || !strings.HasPrefix(is.Report, "## Symptom") { + t.Fatalf("362 kept as %+v", is) + } + if is, _ := h.Get(362); is.Replay != "R362" || !reflect.DeepEqual(is.FixedBy, []string{"mesh-catalog#171, mesh-controller#197"}) { + t.Fatalf("362 kept as %+v", is) + } + if is, _ := h.Get(236); is.Replay != "R236" || is.Closed != nil { + t.Fatalf("236 kept as %+v", is) + } + if is, _ := h.Get(43); is.Closed == nil || is.Closed.Format("2006-01-02") != "2026-09-12" { + t.Fatalf("43 kept as %+v", is) + } + // The owners hq named that the registered list does not know are kept, and marked. + if is, _ := h.Get(7); !reflect.DeepEqual(is.LocatedInUnverified, []string{"hal (the instance)"}) || len(is.LocatedIn) != 3 { + t.Fatalf("7 kept as %+v", is) + } + if !reflect.DeepEqual(out.UnverifiedOwners["hal (the instance)"], []uint64{7}) { + t.Fatalf("unverified %v", out.UnverifiedOwners) + } + // The next open takes the number after the setting, as if nothing had been imported below it. + if got := h.open(t, "after the import"); got != 365 { + t.Fatalf("opened %d after the import, want 365", got) + } + l, _ := h.List(ListAsk{Status: StatusResolved}) + if l.Matched != 5 || l.Rows[0].Number != 362 { + t.Fatalf("listed %+v", l) + } +} + +func TestASecondImportChangesNothingAndSaysSo(t *testing.T) { + h := newHarness(t) + if _, err := h.Import(ImportAsk{}, newSample(t)); err != nil { + t.Fatal(err) + } + before := h.mem.snapshot(IssuesState) + said := len(h.said()) + out, err := h.Import(ImportAsk{}, newSample(t)) + if err != nil { + t.Fatal(err) + } + if out.Created != 0 || out.Filled != 0 || out.Same != 7 || out.Differs != 0 || out.Refused != 0 || !strings.Contains(out.Said, "Nothing changed") { + t.Fatalf("%+v", out) + } + if !reflect.DeepEqual(h.mem.snapshot(IssuesState), before) || len(h.said()) != said { + t.Fatal("a second import changed an issue or said an event") + } +} + +func TestAnIssueTheTrackerHoldsDifferentlyIsNeverOverwritten(t *testing.T) { + h := newHarness(t) + if _, err := h.Import(ImportAsk{}, newSample(t)); err != nil { + t.Fatal(err) + } + // The tracker is the truth once imported: 345 moves on, 80 takes a note. + if _, err := h.Move(MoveAsk{Number: 345, Status: StatusLocated, LocatedIn: []string{"mesh-catalog modules/mesh-issues"}}); err != nil { + t.Fatal(err) + } + if _, err := h.Note(80, "a note"); err != nil { + t.Fatal(err) + } + before := h.mem.snapshot(IssuesState) + out, err := h.Import(ImportAsk{}, newSample(t)) + if err != nil { + t.Fatal(err) + } + if out.Differs != 1 || out.Same != 6 || out.Created != 0 { + t.Fatalf("%+v", out) + } + var row ImportRow + for _, r := range out.Rows { + if r.Number == 345 { + row = r + } + } + if row.Action != ActionDiffers || !strings.Contains(row.Why, "status, located-in") || !strings.Contains(row.Why, "the tracker is the truth") { + t.Fatalf("row %+v", row) + } + if !reflect.DeepEqual(h.mem.snapshot(IssuesState), before) { + t.Fatal("the import changed an issue the tracker holds") + } +} + +func TestAFolderBackedIssueTheTrackerOpenedTakesItsStatusFromItsFolderOnce(t *testing.T) { + h := newHarness(t) + // Before slice 3, an issue opened with a folder had no status in the tracker; its folder kept it. + h.mem.put(IssuesState, "362", Issue{Number: 362, Title: "opened through the tracker", Report: "r", OpenedBy: Unproven, + Record: "04-ISSUES/362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person", Opening: "x", + Notes: []Note{{By: Unproven, Text: "kept"}}}) + h.settings.FirstNumber = 300 // so 362 is a number the tracker gave + out, err := h.Import(ImportAsk{}, newSample(t)) + if err != nil { + t.Fatal(err) + } + // 345 is past the first number too, with no issue in the tracker: refused, as a folder made without it. + if out.Filled != 1 || out.Created != 5 || out.Refused != 1 { + t.Fatalf("%+v", out) + } + is, _ := h.Get(362) + if is.Status != StatusResolved || is.Title != "opened through the tracker" || is.Report != "r" || len(is.Notes) != 1 || + is.OpenedBy != Unproven || is.Imported == nil || len(is.LocatedIn) != 2 || len(is.Links) == 0 { + t.Fatalf("362 filled as %+v", is) + } + if !strings.Contains(strings.Join(out.Odd, "\n"), "362: the tracker's title") { + t.Fatalf("odd %v", out.Odd) + } + out, _ = h.Import(ImportAsk{}, newSample(t)) + if out.Filled != 0 || out.Same != 6 || out.Refused != 1 { + t.Fatalf("second: %+v", out) + } +} + +func TestATrackerIssueWithoutAStatusAndWithoutAFolderBecomesOpen(t *testing.T) { + h := newHarness(t) + h.mem.put(IssuesState, "375", Issue{Number: 375, Title: "t", Report: "r", OpenedBy: Unproven, Record: "04-ISSUES/375-not-merged", Opening: "x"}) + out, err := h.Import(ImportAsk{DryRun: true}, newSample(t)) + if err != nil { + t.Fatal(err) + } + if out.Filled != 1 || out.Rows[len(out.Rows)-1].Action != ActionWould+ActionFill || out.Rows[len(out.Rows)-1].Number != 375 { + t.Fatalf("dry: %+v", out) + } + if is, _ := h.Get(375); is.Status != "" { + t.Fatal("a dry run wrote a status") + } + if _, err := h.Import(ImportAsk{}, newSample(t)); err != nil { + t.Fatal(err) + } + if is, _ := h.Get(375); is.Status != StatusOpen || is.Imported == nil || is.Imported.From != "" { + t.Fatalf("375 as %+v", is) + } +} + +func TestADryRunReadsEverythingAndWritesNothing(t *testing.T) { + h := newHarness(t) + out, err := h.Import(ImportAsk{DryRun: true}, newSample(t)) + if err != nil { + t.Fatal(err) + } + if !out.DryRun || out.Created != 7 || out.Rows[0].Action != ActionWould+ActionCreate || !strings.Contains(out.Said, "would create 7") || + !strings.Contains(out.Said, "Nothing was written") { + t.Fatalf("%+v", out) + } + if keys, _ := h.mem.Keys(IssuesState); len(keys) != 0 || len(h.said()) != 0 || h.mem.counter(t) != 0 { + t.Fatal("a dry run wrote") + } +} + +func TestTheImportRefusesWhatItMustNotWrite(t *testing.T) { + // A folder numbered past the counter: the counter's number, not a folder's to take. Nothing is imported. + h := newHarness(t) + h.mem.put(CountersState, IssueCounter, 100) + if _, err := h.Import(ImportAsk{}, newSample(t)); err == nil || !strings.Contains(err.Error(), "past the counter") { + t.Fatalf("past the counter: %v", err) + } + if keys, _ := h.mem.Keys(IssuesState); len(keys) != 0 { + t.Fatal("imported past the counter") + } + + // A folder at or past the first number the tracker holds no issue for: made without the tracker. + h = newHarness(t) + h.settings.FirstNumber = 300 + out, err := h.Import(ImportAsk{}, newSample(t)) + if err != nil { + t.Fatal(err) + } + if out.Refused != 2 || out.Created != 5 { + t.Fatalf("%+v", out) + } + for _, r := range out.Rows { + if (r.Number == 345 || r.Number == 362) && (r.Action != ActionRefused || !strings.Contains(r.Why, "made without it")) { + t.Fatalf("row %+v", r) + } + } + + // The record standing elsewhere than asked. + h = newHarness(t) + if _, err := h.Import(ImportAsk{Commit: "abcdef0"}, newSample(t)); err == nil || !strings.Contains(err.Error(), "stands at hq commit 1ea678c6bd48") { + t.Fatalf("another commit: %v", err) + } + if _, err := h.Import(ImportAsk{Commit: "main"}, newSample(t)); err == nil || !strings.Contains(err.Error(), "hex digits") { + t.Fatalf("a commit that is no commit: %v", err) + } + + // A status-less issue the tracker holds under another folder's name is left alone and said. + h = newHarness(t) + h.settings.FirstNumber = 300 + h.mem.put(IssuesState, "345", Issue{Number: 345, Title: "t", Report: "r", OpenedBy: Unproven, Record: "04-ISSUES/345-another-name", Opening: "x"}) + out, err = h.Import(ImportAsk{}, newSample(t)) + if err != nil { + t.Fatal(err) + } + for _, r := range out.Rows { + if r.Number == 345 && (r.Action != ActionDiffers || !strings.Contains(r.Why, "not its own")) { + t.Fatalf("row %+v", r) + } + } + if is, _ := h.Get(345); is.Status != "" { + t.Fatal("a status was taken from a folder that is not the issue's") + } + + // The record moving while it is read. + h = newHarness(t) + rec := newSample(t) + rec.moveTo = "ffffffffffff" + if _, err := h.Import(ImportAsk{}, rec); err == nil || !strings.Contains(err.Error(), "moved while it was read") { + t.Fatalf("a moving record: %v", err) + } + if keys, _ := h.mem.Keys(IssuesState); len(keys) != 0 { + t.Fatal("imported from a moving record") + } + + // An issue that appears under the number while the import runs is never overwritten. + h = newHarness(t) + h.mem.fail[IssuesState] = errors.New("the bus is away") + out, err = h.Import(ImportAsk{}, newSample(t)) + if err != nil { + t.Fatal(err) + } + if out.Refused != 1 || out.Created != 6 || !strings.Contains(out.Rows[0].Why, "could not be written") { + t.Fatalf("%+v", out) + } +} + +func TestAReportThatCannotBeReadIsRefusedAloneAndSaidWhy(t *testing.T) { + dir := t.TempDir() + write := func(folder, text string) { + if err := os.MkdirAll(filepath.Join(dir, IssuesFolder, folder), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, IssuesFolder, folder, "00-report.md"), []byte(text), 0o644); err != nil { + t.Fatal(err) + } + } + write("001-good", "---\nstatus: open\nopened: 2026-10-01\nlocated-in: [mesh-controller]\n---\n# 1. Good\n\nbody\n") + write("002-sixth-status", "---\nstatus: fixed\n---\n# 2. t\n\nbody\n") + write("003-no-title", "---\nstatus: open\n---\nbody\n") + write("004-a-secret", "---\nstatus: open\n---\n# 4. t\n\nthe key is sk-ant-api03-abcdefghijklmnopqrstuvwxyz\n") + write("005-twice", "---\nstatus: open\n---\n# 5. t\n\nbody\n") + write("005-again", "---\nstatus: open\n---\n# 5. t\n\nbody\n") + write("006-no-report", "") + if err := os.Remove(filepath.Join(dir, IssuesFolder, "006-no-report", "00-report.md")); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(dir, IssuesFolder, "README"), 0o755); err != nil { + t.Fatal(err) + } + h := newHarness(t) + out, err := h.Import(ImportAsk{}, dirRecords{dir: dir}) + if err != nil { + t.Fatal(err) + } + if out.Folders != 7 || out.Created != 1 || out.Refused != 6 { + t.Fatalf("%+v", out) + } + why := map[uint64]string{} + for _, r := range out.Rows { + why[r.Number] = r.Why + } + for n, want := range map[uint64]string{2: "not one of", 3: "no title", 4: "an Anthropic key", 5: "two folders are numbered 5", 6: "could not be read"} { + if !strings.Contains(why[n], want) { + t.Errorf("%d: %q lacks %q", n, why[n], want) + } + } + if !strings.Contains(strings.Join(out.Odd, "\n"), "README is not an issue folder") { + t.Fatalf("odd %v", out.Odd) + } + if is, err := h.Get(1); err != nil || is.Status != StatusOpen || is.Title != "Good" { + t.Fatalf("1: %+v %v", is, err) + } +} + +func TestTheImportVerbReadsItsArgumentsStrictly(t *testing.T) { + h := newHarness(t) + h.Records = newSample(t) + if _, err := h.call(t, "import", map[string]any{"path": "/tmp"}); err == nil || !strings.Contains(err.Error(), "does not take path") { + t.Fatalf("an unknown argument: %v", err) + } + if _, err := h.call(t, "import", map[string]any{"dry-run": "yes"}); err == nil { + t.Fatal("dry-run as text") + } + out, err := h.call(t, "import", map[string]any{"dry-run": true, "commit": "1ea678c"}) + if err != nil { + t.Fatal(err) + } + if got := out.(*Imported); !got.DryRun || got.Created != 7 { + t.Fatalf("%+v", got) + } +} diff --git a/modules/mesh-issues/cmd/mesh-issues/issue.go b/modules/mesh-issues/cmd/mesh-issues/issue.go index f0a0aebf..7c54e473 100644 --- a/modules/mesh-issues/cmd/mesh-issues/issue.go +++ b/modules/mesh-issues/cmd/mesh-issues/issue.go @@ -42,18 +42,22 @@ const Unproven = "unproven" type Issue struct { Number uint64 `json:"number"` Title string `json:"title"` - // Status is one of the five; empty for an issue whose folder in hq keeps its status (to-be 53 §3). + // Status is one of the five. It was empty for an issue opened with a folder in hq before the import of + // slice 3, whose folder kept its status; the import fills it (to-be 53 §8). Status string `json:"status,omitempty"` Report string `json:"report"` Opened time.Time `json:"opened"` OpenedBy string `json:"opened-by"` Closed *time.Time `json:"closed,omitempty"` // LocatedIn names registered owners, each optionally followed by where in it. - LocatedIn []string `json:"located-in,omitempty"` - FixedBy []string `json:"fixed-by,omitempty"` - AmendedDesign []string `json:"amended-design,omitempty"` - Replay string `json:"replay,omitempty"` - ReplayNone string `json:"replay-none,omitempty"` + LocatedIn []string `json:"located-in,omitempty"` + // LocatedInUnverified is the entries of LocatedIn that named no registered owner when the import took + // them from hq as written; a move that gives located-in anew clears it, as a move checks every entry. + LocatedInUnverified []string `json:"located-in-unverified,omitempty"` + FixedBy []string `json:"fixed-by,omitempty"` + AmendedDesign []string `json:"amended-design,omitempty"` + Replay string `json:"replay,omitempty"` + ReplayNone string `json:"replay-none,omitempty"` // Reason is why it will not be fixed: what a wontfix needs. Reason string `json:"reason,omitempty"` // Record is the issue's folder in hq, when it has one. @@ -62,6 +66,23 @@ type Issue struct { Links []Link `json:"links,omitempty"` // Opening is the id of the call that opened it, so a write whose answer was lost is known as its own. Opening string `json:"opening"` + // Imported says that the issue, or its status, was taken from its folder in hq by the import of + // slice 3: when, by whom, at which commit of hq, from which document. An issue the import wrote has + // OpenedBy ImportedBy, and its Opened is the date its report gave. + Imported *Import `json:"imported,omitempty"` +} + +// ImportedBy is who opened an issue the import wrote: not a caller of `open`, and not a name anybody typed. +const ImportedBy = "import" + +// Import is how an issue, or its status, came from hq. +type Import struct { + At time.Time `json:"at"` + // By is who ran the import, as the runtime proves it (Unproven until it does). + By string `json:"by"` + Commit string `json:"commit"` + // From is the document read, empty for a status filled without one. + From string `json:"from,omitempty"` } // Note is one note on an issue: appended, never edited. diff --git a/modules/mesh-issues/cmd/mesh-issues/main.go b/modules/mesh-issues/cmd/mesh-issues/main.go index 8b8c7682..de5e3824 100644 --- a/modules/mesh-issues/cmd/mesh-issues/main.go +++ b/modules/mesh-issues/cmd/mesh-issues/main.go @@ -22,14 +22,15 @@ import ( const Seat = "issue-tracker" // Verbs are the seat's verbs, as the manifest declares and claims them. -var Verbs = []string{"open", "get", "list", "move", "note", "link"} +var Verbs = []string{"open", "get", "list", "move", "note", "link", "import"} func logf(format string, a ...any) { fmt.Fprintf(os.Stderr, format+"\n", a...) } // Settings are the operator's values for this module, merged by the mesh into one JSON file. type Settings struct { - // FirstNumber is the number the first issue gets: read once, when the counter is started, and never - // again. The counter never starts below an issue already kept. + // FirstNumber is the number the first issue gets: read once for the counter, when it is started, and + // never again for it. The counter never starts below an issue already kept. The import reads it as the + // boundary between the folders hq numbered before the tracker and the issues the tracker numbered. FirstNumber uint64 `json:"first-number"` // Owners is the registered list `located-in` may name (to-be 53 §3). Owners []string `json:"owners"` @@ -67,6 +68,9 @@ func main() { Logf: logf, Sleep: time.Sleep, } + if len(os.Args) > 1 && os.Args[1] == "dry-run" { + os.Exit(dryRunAtTheTerminal(t, os.Args[2:])) + } if _, err := t.Settings(); err != nil { logf("[mesh-issues] the settings cannot be read (%v): every open and move will say so", err) } @@ -76,6 +80,43 @@ func main() { } } +// dryRunAtTheTerminal is `mesh-issues dry-run `: the import's dry run over a +// checkout on disk, against no tracker, before the module is rolled out. It reads, writes nothing, and +// prints what the import would create from that checkout, as the verb's dry run would answer against an +// empty tracker. Not a way to import: only the verb writes, and it reads the record over the bus. +func dryRunAtTheTerminal(t *Tracker, args []string) int { + if len(args) != 2 { + logf("usage: mesh-issues dry-run ") + return 2 + } + t.Store = emptyStore{} + t.Settings = func() (Settings, error) { return readSettings(args[1]) } + out, err := t.Import(ImportAsk{DryRun: true}, dirRecords{dir: args[0]}) + if err != nil { + logf("%v", err) + return 1 + } + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + _ = enc.Encode(out) + return 0 +} + +// emptyStore is a Store that holds nothing and takes nothing: the terminal's dry run. +type emptyStore struct{} + +func (emptyStore) Get(string, string) (*Entry, error) { return nil, nil } +func (emptyStore) Create(string, string, any) (uint64, error) { + return 0, errors.New("a dry run at the terminal writes nothing") +} +func (emptyStore) Update(string, string, any, uint64) (uint64, error) { + return 0, errors.New("a dry run at the terminal writes nothing") +} +func (emptyStore) Keys(string) ([]string, error) { return nil, nil } +func (emptyStore) Increment(string, string, int) (uint64, error) { + return 0, errors.New("a dry run at the terminal writes nothing") +} + // args reads one call's arguments strictly: an argument the verb does not take is refused, never ignored, // so nothing a caller sends is silently dropped — and no argument can say who the caller is. type args struct { @@ -285,8 +326,7 @@ func Tools(t *Tracker) []stdio.Tool { "development cycle's rules: located needs located-in, owners on the registered list; resolved needs " + "located-in and fixed-by (the pull requests or commits), and an issue in the mesh's core also replay " + "(the replay's id, such as R361) or replay-none (why no replay is possible); wontfix needs a reason. " + - "A refused move changes nothing and says which rule refused it. An issue with a folder in hq keeps its " + - "status there and is refused here.", + "A refused move changes nothing and says which rule refused it.", Input: schema([]string{"number", "status"}, map[string]any{ "number": number("the issue's number"), "status": map[string]any{"type": "string", "enum": Statuses, "description": "the status it moves to"}, @@ -379,5 +419,35 @@ func Tools(t *Tracker) []stdio.Tool { } return t.Link(ask) }}, + {Name: Seat + ".import", + Description: "Import hq's issue folders (04-ISSUES/NNN-slug/00-report.md), read through the records module at " + + "the commit its checkout stands at, each as an issue under its own number with the status, owners, fix " + + "and replay its report gives. It only ever creates an issue that is not there, or fills the status of one " + + "the tracker holds without a status; it never overwrites an issue, never advances the counter, refuses a " + + "folder numbered past the counter or past the tracker's first number without an issue, and run again " + + "changes nothing and says so. With dry-run it writes nothing and says what it would do; with commit it " + + "refuses a checkout standing elsewhere. The answer is a row per folder and the counts.", + Input: schema(nil, map[string]any{ + "dry-run": map[string]any{"type": "boolean", "description": "read everything, write nothing, say what would be done"}, + "commit": text("the commit of hq the record must stand at (a prefix is enough); refused when it stands elsewhere"), + }), + Run: func(m map[string]any) (any, error) { + a, err := readArgs("import", m, "dry-run", "commit") + if err != nil { + return nil, err + } + var ask ImportAsk + if ask.DryRun, err = a.flag("dry-run"); err != nil { + return nil, err + } + if ask.Commit, err = a.str("commit"); err != nil { + return nil, err + } + records := t.Records + if records == nil { + records = busRecords{} + } + return t.Import(ask, records) + }}, } } diff --git a/modules/mesh-issues/cmd/mesh-issues/manifest_test.go b/modules/mesh-issues/cmd/mesh-issues/manifest_test.go index 515f190f..5f5feb17 100644 --- a/modules/mesh-issues/cmd/mesh-issues/manifest_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/manifest_test.go @@ -81,7 +81,7 @@ func TestItDeclaresAndHoldsTheIssueTrackerSeatOnceForTheMesh(t *testing.T) { if m.Module != "mesh-issues" || !reflect.DeepEqual(m.Seats, want) || !reflect.DeepEqual(m.Claims, want) { t.Fatalf("module %q, seats %+v, claims %+v", m.Module, m.Seats, m.Claims) } - if !reflect.DeepEqual(m.Emits, []string{EventOpened, EventMoved, EventNoted, EventLinked}) { + if !reflect.DeepEqual(m.Emits, []string{EventOpened, EventMoved, EventNoted, EventLinked, EventImported}) { t.Fatalf("emits %v", m.Emits) } // Two states, neither expiring: issues keep a short history per key, the counter one value. diff --git a/modules/mesh-issues/cmd/mesh-issues/memory_test.go b/modules/mesh-issues/cmd/mesh-issues/memory_test.go index 59bf55d3..71ed5e7b 100644 --- a/modules/mesh-issues/cmd/mesh-issues/memory_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/memory_test.go @@ -217,3 +217,14 @@ func (h *harness) call(t *testing.T, verb string, args map[string]any) (any, err t.Fatalf("no verb %s", verb) return nil, nil } + +// snapshot is every value of a state, by key, as kept now. +func (m *memory) snapshot(state string) map[string]string { + m.mu.Lock() + defer m.mu.Unlock() + out := map[string]string{} + for k, e := range m.data[state] { + out[k] = string(e.Value) + } + return out +} diff --git a/modules/mesh-issues/cmd/mesh-issues/report.go b/modules/mesh-issues/cmd/mesh-issues/report.go new file mode 100644 index 00000000..c25598f7 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/report.go @@ -0,0 +1,339 @@ +package main + +import ( + "fmt" + "regexp" + "sort" + "strconv" + "strings" + "unicode" +) + +// An issue report in hq, `04-ISSUES/NNN-slug/00-report.md`, as the import of slice 3 reads it (novox/hq +// to-be 53 §8): a YAML frontmatter of a few keys, a heading that is the title, and the report. The +// frontmatter is read as hq's own checks read it (00-META/checks/cycle.py), a little more carefully: a +// list in brackets may run over lines and hold commas inside parentheses or quotes, a block list's items +// may be quoted, and a long value may wrap onto an indented line. + +// Report is one issue report as read from its folder. +type Report struct { + Number uint64 + // Folder is the issue's folder, `04-ISSUES/NNN-slug`. + Folder string + // Path is the report, `/00-report.md`. + Path string + Title string + Status string + // Opened and Closed are dates as the frontmatter gives them, YYYY-MM-DD, or empty. + Opened string + Closed string + LocatedIn []string + FixedBy []string + AmendedDesign []string + Replay string + ReplayNone string + // Body is the report without its frontmatter and its title. + Body string + // Issues and Decisions are the other issues and the decision records the report names. + Issues []uint64 + Decisions []string + // Odd is what was odd about the report and did not stop it from being read. + Odd []string +} + +var ( + issueFolder = regexp.MustCompile(`^([0-9]{1,4})-([a-z0-9]+(?:-[a-z0-9]+)*)$`) + frontKey = regexp.MustCompile(`^([A-Za-z][A-Za-z0-9-]*):(.*)$`) + blockItem = regexp.MustCompile(`^\s+-\s*(.*)$`) + continued = regexp.MustCompile(`^\s+(\S.*)$`) + heading = regexp.MustCompile(`(?m)^#\s+(.+?)\s*$`) + numbered = regexp.MustCompile(`^([0-9]{1,4})\s*([.:]|—|–|-)?\s*(.*)$`) + dateShape = regexp.MustCompile(`^[0-9]{4}-[0-9]{2}-[0-9]{2}$`) + namesIssue = regexp.MustCompile(`(?i)\bissue ([0-9]{1,4})\b|04-ISSUES/([0-9]{1,4})-`) + namesRecord = regexp.MustCompile(`\bADR ([0-9]{4})\b|02-DECISIONS/([0-9]{4})-`) +) + +// folderNumber is the number an issue folder's name carries, or false for a name that is not one. +func folderNumber(name string) (uint64, bool) { + m := issueFolder.FindStringSubmatch(name) + if m == nil || len(m[2]) > 120 { + return 0, false + } + n, err := strconv.ParseUint(m[1], 10, 64) + if err != nil || n == 0 { + return 0, false + } + return n, true +} + +// frontmatter is the YAML block between the first two `---` lines as {key: values}, and the text after +// it. A scalar is one value; a list, in brackets or as block items, its items; an empty value no values. +// A key given twice keeps the last. False when the text has no frontmatter. +func frontmatter(text string) (map[string][]string, string, bool) { + text = strings.ReplaceAll(text, "\r\n", "\n") + if !strings.HasPrefix(text, "---\n") { + return nil, "", false + } + rest := text[4:] + end := -1 + if strings.HasPrefix(rest, "---\n") || rest == "---" { + end = 0 + } else if i := strings.Index(rest, "\n---\n"); i >= 0 { + end = i + 1 + } else if strings.HasSuffix(rest, "\n---") { + end = len(rest) - 3 + } + if end < 0 { + return nil, "", false + } + block := rest[:end] + body := strings.TrimPrefix(rest[end:], "---") + body = strings.TrimPrefix(body, "\n") + + out := map[string][]string{} + key := "" + const ( + none = iota + scalar + list + flow + ) + mode := none + flowText := "" + closeFlow := func() { + out[key] = splitFlow(flowText) + mode = list + } + for _, line := range strings.Split(block, "\n") { + if strings.TrimSpace(line) == "" || strings.HasPrefix(strings.TrimSpace(line), "#") { + continue + } + if mode == flow { + flowText += " " + strings.TrimSpace(line) + if flowClosed(flowText) { + closeFlow() + } + continue + } + if m := frontKey.FindStringSubmatch(line); m != nil && !strings.HasPrefix(line, " ") { + key = m[1] + value := strings.TrimSpace(m[2]) + switch { + case value == "": + out[key] = nil + mode = list + case strings.HasPrefix(value, "["): + flowText = value + mode = flow + if flowClosed(flowText) { + closeFlow() + } + default: + out[key] = []string{unquote(value)} + mode = scalar + } + continue + } + if key == "" { + continue + } + if m := blockItem.FindStringSubmatch(line); m != nil && mode == list { + out[key] = append(out[key], unquote(strings.TrimSpace(m[1]))) + continue + } + if m := continued.FindStringSubmatch(line); m != nil && len(out[key]) > 0 { + last := len(out[key]) - 1 + out[key][last] = strings.TrimSpace(out[key][last] + " " + strings.TrimSpace(m[1])) + } + } + if mode == flow { + // Never closed: what was read of it, without the bracket. + out[key] = splitFlow(flowText + "]") + } + return out, body, true +} + +// flowClosed says whether a bracketed list has its closing bracket, outside quotes. A quote opens an item +// only at the item's start: an apostrophe inside prose ("the provisioner's ACL") is no quote. +func flowClosed(s string) bool { + depth, quote, atStart := 0, rune(0), true + for _, r := range s { + switch { + case quote != 0: + if r == quote { + quote = 0 + } + continue + case (r == '"' || r == '\'') && atStart: + quote = r + case r == '[': + depth++ + case r == ']': + depth-- + if depth == 0 { + return true + } + } + atStart = r == '[' || r == ',' || atStart && unicode.IsSpace(r) + } + return false +} + +// splitFlow is a bracketed list's items: split on the commas outside parentheses, brackets and quotes, +// a quote opening an item only at its start. +func splitFlow(s string) []string { + s = strings.TrimSpace(s) + s = strings.TrimPrefix(s, "[") + s = strings.TrimSuffix(s, "]") + var items []string + var cur strings.Builder + depth, quote, atStart := 0, rune(0), true + flush := func() { + if v := unquote(strings.TrimSpace(cur.String())); v != "" { + items = append(items, v) + } + cur.Reset() + } + for _, r := range s { + switch { + case quote != 0: + if r == quote { + quote = 0 + } + case (r == '"' || r == '\'') && atStart: + quote = r + case r == '(' || r == '[': + depth++ + case r == ')' || r == ']': + if depth > 0 { + depth-- + } + case r == ',' && depth == 0: + flush() + atStart = true + continue + } + cur.WriteRune(r) + atStart = atStart && unicode.IsSpace(r) + } + flush() + return items +} + +// unquote strips one pair of matching quotes around a value, and the escapes a double-quoted YAML +// scalar uses for its own quote. +func unquote(v string) string { + if len(v) >= 2 && (v[0] == '"' && v[len(v)-1] == '"' || v[0] == '\'' && v[len(v)-1] == '\'') { + inner := v[1 : len(v)-1] + if v[0] == '"' { + inner = strings.ReplaceAll(inner, `\"`, `"`) + } else { + inner = strings.ReplaceAll(inner, `''`, `'`) + } + return strings.TrimSpace(inner) + } + return v +} + +// ParseReport reads one issue report. The folder is its name, `NNN-slug`; the text is the report whole. +// What makes a report unreadable is an error: no frontmatter, no title, no status, or a status that +// is not one of the five. What is odd but readable is said in Odd. +func ParseReport(folder, text string) (*Report, error) { + n, ok := folderNumber(folder) + if !ok { + return nil, fmt.Errorf("%q is not an issue folder's name: NNN-slug, lower-case words joined by hyphens", folder) + } + front, body, ok := frontmatter(text) + if !ok { + return nil, fmt.Errorf("%s/00-report.md has no frontmatter", folder) + } + r := &Report{Number: n, Folder: "04-ISSUES/" + folder, Path: "04-ISSUES/" + folder + "/00-report.md"} + one := func(key string) string { + if v := front[key]; len(v) > 0 { + if len(v) > 1 { + r.Odd = append(r.Odd, fmt.Sprintf("%s: is a list of %d; one value was expected, and they were joined", key, len(v))) + return strings.Join(v, "; ") + } + return v[0] + } + return "" + } + r.Status = one("status") + if r.Status == "" { + return nil, fmt.Errorf("%s/00-report.md has no status", folder) + } + if !known(r.Status) { + return nil, fmt.Errorf("%s/00-report.md has the status %q, not one of %s", folder, r.Status, strings.Join(Statuses, ", ")) + } + r.Opened = one("opened") + if r.Opened != "" && !dateShape.MatchString(r.Opened) { + r.Odd = append(r.Odd, fmt.Sprintf("opened: %q is not a date", r.Opened)) + r.Opened = "" + } + for _, key := range []string{"resolved", "closed"} { + if v := one(key); v != "" { + if dateShape.MatchString(v) { + r.Closed = v + } else { + r.Odd = append(r.Odd, fmt.Sprintf("%s: %q is not a date", key, v)) + } + } + } + r.LocatedIn = front["located-in"] + r.FixedBy = front["fixed-by"] + r.AmendedDesign = front["amended-design"] + r.Replay = one("replay") + r.ReplayNone = one("replay-none") + imported := map[string]bool{"status": true, "opened": true, "resolved": true, "closed": true, "located-in": true, + "fixed-by": true, "amended-design": true, "replay": true, "replay-none": true} + var unknown []string + for k := range front { + if !imported[k] { + unknown = append(unknown, k) + } + } + if len(unknown) > 0 { + sort.Strings(unknown) + r.Odd = append(r.Odd, "frontmatter keys not imported: "+strings.Join(unknown, ", ")) + } + + m := heading.FindStringSubmatchIndex(body) + if m == nil { + return nil, fmt.Errorf("%s/00-report.md has no title: no `# ` heading", folder) + } + title := body[m[2]:m[3]] + // "362. The title", "043 — The title": the number and its separator are the folder's, not the title's. A + // title that starts with a bare number ("2 nodes …") keeps it, unless the number is the folder's. + if nm := numbered.FindStringSubmatch(title); nm != nil { + heads, _ := strconv.ParseUint(nm[1], 10, 64) + if nm[2] != "" || heads == n { + title = nm[3] + if heads != n && title != "" { + r.Odd = append(r.Odd, fmt.Sprintf("the title is numbered %s; the folder is %d", nm[1], n)) + } + } + } + r.Title = strings.TrimSpace(title) + if r.Title == "" { + return nil, fmt.Errorf("%s/00-report.md has an empty title", folder) + } + r.Body = strings.TrimSpace(body[:m[0]] + body[m[1]:]) + + seen := map[uint64]bool{n: true} + for _, mm := range namesIssue.FindAllStringSubmatch(text, -1) { + if other, err := strconv.ParseUint(mm[1]+mm[2], 10, 64); err == nil && !seen[other] { + seen[other] = true + r.Issues = append(r.Issues, other) + } + } + sort.Slice(r.Issues, func(i, j int) bool { return r.Issues[i] < r.Issues[j] }) + records := map[string]bool{} + for _, mm := range namesRecord.FindAllStringSubmatch(text, -1) { + if d := mm[1] + mm[2]; !records[d] { + records[d] = true + r.Decisions = append(r.Decisions, d) + } + } + sort.Strings(r.Decisions) + return r, nil +} diff --git a/modules/mesh-issues/cmd/mesh-issues/report_test.go b/modules/mesh-issues/cmd/mesh-issues/report_test.go new file mode 100644 index 00000000..ef321366 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/report_test.go @@ -0,0 +1,198 @@ +package main + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// The reports in testdata/hq are copies of real issue reports of novox/hq, as they stood on 2026-10-10, +// chosen for their shapes: a bracketed list with commas inside parentheses, quoted block items, an +// apostrophe in prose inside a bracketed list, a `resolved:` date, a replay, an empty list, a wontfix. + +func readSample(t *testing.T, folder string) string { + t.Helper() + raw, err := os.ReadFile(filepath.Join("testdata", "hq", IssuesFolder, folder, "00-report.md")) + if err != nil { + t.Fatal(err) + } + return string(raw) +} + +func TestTheFrontmatterOfRealReportsIsReadAsWritten(t *testing.T) { + cases := []struct { + folder string + want Report + }{ + {"362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person", Report{ + Number: 362, Status: StatusResolved, Opened: "2026-10-10", + Title: "Two catalogue merges seconds apart left their delivery waiting for a person", + // A bracketed list whose entries hold commas inside parentheses: two entries, not four. + LocatedIn: []string{"mesh-catalog modules/mesh-delivery (holder.go deliveryWalking)", + "mesh-controller cmd/mesh-controller (upgrades.go SourceMoved, release_plan.go laterOnTheBranch)"}, + // A scalar is one entry, commas and all, as hq's own check reads it. + FixedBy: []string{"mesh-catalog#171, mesh-controller#197"}, + AmendedDesign: []string{"03-DESIGN/01-to-be/47-delivery-from-commit-to-delivered.md"}, + Replay: "R362", + }}, + {"007-an-installed-package-is-not-a-capability", Report{ + Number: 7, Status: StatusResolved, Opened: "2026-08-23", + Title: "An installed package is not an available capability", + LocatedIn: []string{"hal (the instance)", "mesh-host internal/bootstrap (preflight asks the daemon)", "mesh-controller internal/catalogue (capabilities)"}, + // Quoted block items, each holding colons and commas, taken whole and unquoted. + FixedBy: []string{ + "the instance only: PR #962 — incus hook. Merged, ran on one node in 6s of a 60s budget; pipeline #6832 green in 48s. The class remains open.", + "partly, on the mesh: mesh-host 73c010e, 9d8239a — preflight and the profile detectors ask the daemon, not the package (installed-but-broken reports absent)", + "the class, on the mesh: a manifest declares capabilities, an action declares verify (ADR 0005), and a module's own assertions are the lab's verdict (design 01). The old arrangement is replaced module by module, so its count is never taken", + }, + AmendedDesign: []string{"03-DESIGN/01-to-be/01-end-to-end-testing.md"}, + }}, + {"080-a-cache-grant-lets-the-consumer-flush-the-server", Report{ + Number: 80, Status: StatusResolved, Opened: "2026-09-22", + Title: "A cache grant lets the consumer flush the server", + // An apostrophe inside prose in a bracketed list is no quote: the list closes on its own line. + LocatedIn: []string{"mesh-catalog modules/redis (the provisioner's ACL)"}, + FixedBy: []string{"mesh-catalog multiple-fixes (the consumer's ACL user loses the dangerous command category); proven by " + + "the grant end-to-end bed, which now asserts a write outside the consumer's keys and FLUSHALL are refused"}, + }}, + {"043-a-module-cannot-be-given-the-meshs-own-queues", Report{ + Number: 43, Status: StatusResolved, Opened: "2026-09-12", Closed: "2026-09-12", + Title: "A module cannot be given an account for the mesh's own queues", + LocatedIn: []string{"mesh-controller (the verb existed)"}, + FixedBy: []string{"nothing — the capability already existed and the wrong verb was used"}, + }}, + {"236-the-catalogue-check-passes-a-manifest-the-host-refuses", Report{ + Number: 236, Status: StatusResolved, Opened: "2026-10-04", + Title: "The catalogue check passes a manifest the host refuses", + LocatedIn: []string{"mesh-controller cmd/mesh-controller (the merge gate)"}, + FixedBy: []string{"novox/mesh-controller PR #96 (bbd442c, the merge gate, ADR 0237)"}, + Replay: "R236", + }}, + {"345-a-reminder-window-shows-transparent-and-empty", Report{ + Number: 345, Status: StatusDiagnosing, Opened: "2026-10-09", + Title: "A calendar reminder shows as a transparent, empty window, and the mesh had no tool to look at it", + }}, + {"150-a-route-is-contributed-before-its-module-is-taken", Report{ + Number: 150, Status: StatusWontfix, Opened: "2026-09-29", + Title: "A route is contributed before its module is taken", + LocatedIn: []string{"mesh-controller internal/catalogue/declaration.go (contributions are emitted for an assigned module, taken or not)"}, + }}, + } + for _, c := range cases { + got, err := ParseReport(c.folder, readSample(t, c.folder)) + if err != nil { + t.Fatalf("%s: %v", c.folder, err) + } + if got.Folder != IssuesFolder+"/"+c.folder || got.Path != got.Folder+"/00-report.md" { + t.Fatalf("%s: folder %q path %q", c.folder, got.Folder, got.Path) + } + if got.Body == "" || strings.Contains(got.Body, "\n# ") || strings.HasPrefix(got.Body, "# ") || strings.HasPrefix(got.Body, "---") { + t.Fatalf("%s: the body keeps its frontmatter or title: %.80q", c.folder, got.Body) + } + if len(got.Odd) != 0 { + t.Fatalf("%s: odd %v", c.folder, got.Odd) + } + // The title's number and separator are stripped, whatever the separator: "362. ", "043 — ". + got.Folder, got.Path, got.Body, got.Issues, got.Decisions = "", "", "", nil, nil + if !reflect.DeepEqual(*got, c.want) { + t.Fatalf("%s:\n got %+v\n want %+v", c.folder, *got, c.want) + } + } +} + +func TestAReportNamesTheIssuesAndDecisionsItCites(t *testing.T) { + r, err := ParseReport("362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person", + readSample(t, "362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person")) + if err != nil { + t.Fatal(err) + } + for _, n := range r.Issues { + if n == 362 { + t.Fatal("an issue names itself") + } + } + for _, d := range r.Decisions { + if len(d) != 4 { + t.Fatalf("decision %q", d) + } + } + text := "---\nstatus: open\nopened: 2026-10-10\n---\n# 9. t\n\nSee issue 155, 04-ISSUES/203-x/00-report.md and ADR 0275 " + + "(02-DECISIONS/0019-how.md), and issue 155 again, and issue 9 itself." + r, err = ParseReport("009-t", text) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(r.Issues, []uint64{155, 203}) || !reflect.DeepEqual(r.Decisions, []string{"0019", "0275"}) { + t.Fatalf("issues %v decisions %v", r.Issues, r.Decisions) + } +} + +func TestWhatMakesAReportUnreadableAndWhatIsOnlyOdd(t *testing.T) { + refused := map[string]string{ + "no frontmatter": "# 9. t\n\nbody", + "no status": "---\nopened: 2026-10-10\n---\n# 9. t\n", + "a sixth status": "---\nstatus: fixed\n---\n# 9. t\n", + "no title": "---\nstatus: open\n---\nbody only\n", + "an empty title": "---\nstatus: open\n---\n# 9.\n", + "no folder shape": "", + } + for what, text := range refused { + folder := "009-t" + if what == "no folder shape" { + folder, text = "nine-t", "---\nstatus: open\n---\n# t\n" + } + if _, err := ParseReport(folder, text); err == nil { + t.Errorf("%s was read", what) + } + } + r, err := ParseReport("009-t", "---\nstatus: open\nopened: yesterday\nseverity: high\nreplay: R1\nreplay: R2\n---\n# 12 — t\n\nbody") + if err != nil { + t.Fatal(err) + } + if r.Opened != "" || r.Replay != "R2" || r.Title != "t" { + t.Fatalf("%+v", r) + } + if r, _ := ParseReport("009-t", "---\nstatus: open\n---\n# 2 nodes share a name\n"); r.Title != "2 nodes share a name" { + t.Fatalf("a title starting with a number: %q", r.Title) + } + if r, _ := ParseReport("009-t", "---\nstatus: open\n---\n# 9 nodes share a name\n"); r.Title != "nodes share a name" { + t.Fatalf("a title starting with the folder's number: %q", r.Title) + } + odd := strings.Join(r.Odd, "\n") + for _, want := range []string{"opened: \"yesterday\" is not a date", "frontmatter keys not imported: severity", "numbered 12; the folder is 9"} { + if !strings.Contains(odd, want) { + t.Fatalf("odd %q lacks %q", odd, want) + } + } +} + +func TestTheFrontmatterShapesHqUses(t *testing.T) { + front, body, ok := frontmatter("---\n" + + "a: [x, \"y, z\", (p, q) r]\n" + + "b:\n - one\n - \"two: 2\"\n" + + "c: plain scalar\n wrapped onto a line\n" + + "d:\n" + + "e: []\n" + + "f: [over,\n two lines]\n" + + "g: 'it''s'\n" + + "---\nbody\n") + if !ok || body != "body\n" { + t.Fatalf("ok %v body %q", ok, body) + } + want := map[string][]string{ + "a": {"x", "y, z", "(p, q) r"}, + "b": {"one", "two: 2"}, + "c": {"plain scalar wrapped onto a line"}, + "d": nil, + "e": nil, + "f": {"over", "two lines"}, + "g": {"it's"}, + } + for k, v := range want { + if !reflect.DeepEqual(front[k], v) { + t.Errorf("%s: %q, want %q", k, front[k], v) + } + } +} diff --git a/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/007-an-installed-package-is-not-a-capability/00-report.md b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/007-an-installed-package-is-not-a-capability/00-report.md new file mode 100644 index 00000000..3892ddba --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/007-an-installed-package-is-not-a-capability/00-report.md @@ -0,0 +1,119 @@ +--- +status: resolved +opened: 2026-08-23 +located-in: [hal (the instance), mesh-host internal/bootstrap (preflight asks the daemon), mesh-controller internal/catalogue (capabilities)] +fixed-by: + - "the instance only: PR #962 — incus hook. Merged, ran on one node in 6s of a 60s budget; pipeline #6832 green in 48s. The class remains open." + - "partly, on the mesh: mesh-host 73c010e, 9d8239a — preflight and the profile detectors ask the daemon, not the package (installed-but-broken reports absent)" + - "the class, on the mesh: a manifest declares capabilities, an action declares verify (ADR 0005), and a module's own assertions are the lab's verdict (design 01). The old arrangement is replaced module by module, so its count is never taken" +amended-design: 03-DESIGN/01-to-be/01-end-to-end-testing.md +--- + +# 007 — An installed package is not an available capability + +## Symptom + +A module declares the virtualisation package the lab needs. The package is installed — +version 7.3.0-1, recorded as explicitly installed. The client binary runs. + +The capability does not exist: + +| Checked | State | +|---|---| +| `incus.service` | disabled, inactive | +| `incus.socket` | disabled, inactive | +| `incus-user.socket` | disabled, inactive | +| the operator's group membership | not a member of any incus group | +| the client | reports **`Server version: unreachable`** | + +Nothing failed. Nothing reported anything. The declaration was satisfied exactly as written, +and the thing it was declared for cannot be used. + +## Why this is not issue 001 again + +[Issue 001](../001-failed-package-install-reports-success/00-report.md) is *the install failed +and the job reported success*. This is the opposite and arguably worse: **the install +succeeded, and success was not the point.** + +A package is a set of files. A capability is a running service, an enabled socket, and an +identity permitted to reach it. The module model declares the first and has no vocabulary for +the second, so the gap between them is invisible — there is no state in which the mesh believes +this node has virtualisation and is wrong, because the mesh was never asked to believe it. + +The distance between the two is the same one the delivery layer already has a name for: +**transport versus effect.** A package install reports that files arrived, which is transport. + +## Why it matters now + +This is the first requirement of the lab +([ADR 0016](../../02-DECISIONS/0016-the-lab.md)), which is +phase 0 of the entire migration. The first capability the new work depends on is present, +declared, and unusable — and would have stayed unusable silently. + +It also generalises. Every module that declares a package needing a unit enabled, a group +joined, a kernel module loaded, or a socket activated has this gap. Post-install work lives in +hooks, and hooks have their own recorded failure mode: thirteen were found that had never run. + +## Evidence + +- Package recorded as installed 2026-08-23 00:02, explicitly. +- Both units disabled and inactive; the operator in no incus group; client reports the server + unreachable. + +## Open questions + +- Should a module be able to declare a **capability** — a unit that must be enabled, a group + the operator must be in — rather than only the package that provides it? +- If that is what hooks are for, why is the gap invisible when a hook does not run? A hook that + never fires and a hook that fires and does nothing are indistinguishable today. +- Is this what the verify stage should be asserting? It exists, and a module's own assertions + are meant to test outcomes rather than steps — "the socket accepts a connection" is exactly + that shape. +- How many other declared packages are in this state? Nothing currently reports it, which means + the answer is unknown rather than zero. + +## The instance is fixed; the class is what this issue is now about + +*Updated 2026-08-23.* A hook now does the post-install work — group membership, subordinate id +ranges, enabling both units, creating the storage pool, the bridge, and the default profile. +Verified independently afterwards: the group exists with the operator in it, both id files +carry the range, the service is active, and the storage pool reports `CREATED`. + +**So the mechanism was never missing.** Hooks are exactly the right place for this, and used +properly they work. The gap is narrower and worse than "there is no way to do it": + +> The hook did six things. **Six checks were then performed by a human, by hand.** Nothing in +> the pipeline asserted any of them, and a pipeline that dispatched a hook which silently never +> fired would have been green in the same 48 seconds. + +That is not hypothetical — a hook named for a feature its module does not carry is skipped +without complaint, and thirteen such hooks were found at once in the past. The distance between +*the hook ran and did six things* and *the hook was dispatched* is invisible from the outside, +and it is the whole of this issue. + +### The verification that was done by hand is the assertion set + +The six checks performed after the merge are, almost word for word, what the module's own +verification should assert — outcomes, not steps, exactly as the lab design requires: + +| Asserted by hand | As a module assertion | +|---|---| +| operator in the admin group | the group exists and contains the operator | +| subordinate id range in both files | both files carry the range | +| both units enabled, service active | the service is active, and still active shortly after | +| storage pool present | the pool exists and reports created | +| bridge present with an address | the bridge exists and holds its address | +| default profile wired to both | the profile references the bridge and the pool | + +They currently live in a chat message. Moved into the module, they would run on every delivery +to every node, and the difference between a hook that worked and a hook that was merely +dispatched would stop being something a person has to notice. + +### What this issue now asks + +- Does a module gain a way to declare a **capability** — the outcome — separately from the + package that provides it, or is "write assertions" the whole answer? +- The verify stage exists and asserts almost nothing. Is this simply the first module that + should use it properly, making the issue a coverage problem rather than a design gap? +- **How many other declared packages are in the state this one was in?** Still unknown, still + unreported by anything, and now demonstrably worth asking. diff --git a/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/043-a-module-cannot-be-given-the-meshs-own-queues/00-report.md b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/043-a-module-cannot-be-given-the-meshs-own-queues/00-report.md new file mode 100644 index 00000000..7ad17c06 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/043-a-module-cannot-be-given-the-meshs-own-queues/00-report.md @@ -0,0 +1,53 @@ +--- +status: resolved +opened: 2026-09-12 +resolved: 2026-09-12 +located-in: [mesh-controller (the verb existed)] +fixed-by: nothing — the capability already existed and the wrong verb was used +amended-design: +--- + +# 043 — A module cannot be given an account for the mesh's own queues + +**Withdrawn the day it was opened. The premise was wrong.** Kept rather than deleted, because the +mistake is repeatable and the reason is worth reading. + +## What was claimed + +That a build machine could not be given access to the mesh's build queue, because a broker account +is scoped to what a module `emits` and `consumes`, and the build queue is not a module event. The +evidence was a builder that authenticated and was then refused: + +``` +ACCESS_REFUSED - User 'anchor-builder' doesn't have permissions to queue 'builds' +``` + +## Why it was wrong + +**The capability exists and is reachable from the command line.** There are two verbs, and they +create different things: + +| Verb | Creates | Scoped to | +|---|---|---| +| `module issue --node ` | a module's account | what that module emits and consumes | +| `builder issue [--node ]` | a build machine's account | the build queue, and the mesh exchange | + +The refusal was produced by using the first for a job the second exists to do. Running +`builder issue` and pushing produced a builder that starts and takes work — no change to any +manifest, any code, or the account mechanism. + +## The part worth keeping + +**The wrong verb succeeds, and says so.** `module issue` reported *"broker account created, scoped +to what it emits and consumes"* for a module that emits and consumes nothing, producing an account +that authenticates and can do nothing. The failure then appears one layer away, in the module's own +log, as a permissions error against a queue — which reads like a missing capability rather than a +misused command. + +That is a small, real sharp edge, and it is the whole of what this issue found. Whether it is worth +anything — a refusal when a module with no events asks for an account, or a note in the builder +module pointing at the verb that fits it — is a judgement, not a defect. + +**And a lesson that is not about the mesh:** the capability was three lines away in the same file as +the code being read, under a name that says exactly what it does. The issue was written before +looking for it. diff --git a/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/080-a-cache-grant-lets-the-consumer-flush-the-server/00-report.md b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/080-a-cache-grant-lets-the-consumer-flush-the-server/00-report.md new file mode 100644 index 00000000..5eae272c --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/080-a-cache-grant-lets-the-consumer-flush-the-server/00-report.md @@ -0,0 +1,29 @@ +--- +status: resolved +opened: 2026-09-22 +located-in: [mesh-catalog modules/redis (the provisioner's ACL)] +fixed-by: mesh-catalog multiple-fixes (the consumer's ACL user loses the dangerous command category); proven by the grant end-to-end bed, which now asserts a write outside the consumer's keys and FLUSHALL are refused +--- + +# 080 — A cache grant lets the consumer flush the server + +## Symptom + +The cache provider's provisioner creates each consumer an ACL user confined to keys under its own +login and allowed every command. A key pattern confines only commands that name keys. `FLUSHALL`, +`FLUSHDB`, `CONFIG`, `SHUTDOWN` and the rest of the dangerous category name none, so a consumer +granted "its own keys" could wipe every other consumer's, or stop the server. + +Found by carrying the large mesh bed's retired tenancy assertions into the grant end-to-end bed: +`FLUSHALL` as the consumer answered `OK`. + +## Why it matters beyond the instance + +A grant is the mesh's promise that a consumer gets what it asked for and nothing else. The +promise was checked on the key pattern and never on the command set, and the one bed that had +asked was retired before it was run against the catalogue's module. + +## What would close it + +The ACL user is allowed the ordinary command set minus the dangerous category, and the grant bed +asserts a write outside the consumer's keys and a `FLUSHALL` are both refused. diff --git a/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/150-a-route-is-contributed-before-its-module-is-taken/00-report.md b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/150-a-route-is-contributed-before-its-module-is-taken/00-report.md new file mode 100644 index 00000000..7d8a8958 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/150-a-route-is-contributed-before-its-module-is-taken/00-report.md @@ -0,0 +1,58 @@ +--- +status: wontfix +opened: 2026-09-29 +located-in: + - mesh-controller internal/catalogue/declaration.go (contributions are emitted for an assigned module, taken or not) +fixed-by: +--- + +# 150 — A route is contributed before its module is taken + +## What was observed + +ace is adopted and runs `route-adapter` beside the predecessor's traefik (ADR 0104). The first web +module migrated there was searxng. `assign ace searxng` + `push ace` — the step that is supposed to +change nothing on an adopted machine (ADR 0100, hq 125: assign holds, take replaces) — took +`searxng.zurag.be` down: + +``` +https://searxng.zurag.be/ → 502 +``` + +for about five minutes, until the module was unassigned again. + +## Why + +Assign held everything it found on the machine — the predecessor's `searxng` container, its +directories — exactly as designed. But the module's **route contribution** is not a resource on the +machine, so nothing held it: it reached `route-adapter` at once, which wrote +`mesh-searxng.zurag.be.yml` into traefik's file provider pointing at the mesh's assigned machine port +(`http://ace.internal:20000`) — where nothing listened, because the container that would was held. +traefik's file router for the name then won over the predecessor's docker-label router for the same +name, and the name served a dead backend. + +On this occasion the window was lengthened by an unrelated failure (the machine's docker address +pools were exhausted, so the module's network could not be created), but the fault does not depend on +it: **between assign and take, every routed module's public name points at a backend the mesh has +deliberately not started.** The runbook's §6 step 2 ("check the preparation — HAL's service still +serves") is false for every routed module on a node running route-adapter. + +## What the operator did + +Rolled back per runbook (unassign before take), then re-ran as assign → push → wait for the node to +report its holds → take → push, keeping the window to the ~10 s between the two pushes. A workaround, +not a fix: a module whose data must move between assign and take (runbook §6 step 3) cannot shrink +its window this way. + +## What would be right + +A contribution from a module that is assigned but **not taken** on an adopted node should be held like +the module's resources are — withheld from the provider until take — or the provider should be told +the contributor is held and leave the predecessor's route alone. Either keeps "assign changes nothing" +true for routed modules. + +## Won't fix — 2026-10-08 + +The arrangement it is about no longer exists: every machine is converged, no machine runs the +predecessor's proxy or the `route-adapter` beside it, and the predecessor it was adopted from is +retired, so there is no adopted machine on which an assigned but untaken module can contribute a route. diff --git a/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/236-the-catalogue-check-passes-a-manifest-the-host-refuses/00-report.md b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/236-the-catalogue-check-passes-a-manifest-the-host-refuses/00-report.md new file mode 100644 index 00000000..5301bee7 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/236-the-catalogue-check-passes-a-manifest-the-host-refuses/00-report.md @@ -0,0 +1,44 @@ +--- +status: resolved +opened: 2026-10-04 +located-in: [mesh-controller cmd/mesh-controller (the merge gate)] +fixed-by: novox/mesh-controller PR #96 (bbd442c, the merge gate, ADR 0237) +replay: R236 +amended-design: +--- + +# 236 — The catalogue check passes a manifest the host refuses + +## What was observed + +2026-10-04. A login-manager module passed `mesh-controller module check`, was registered, built and +assigned. The first push of its declaration was refused whole by the host: + +``` +refused a declaration: this declaration is refused, and none of it was applied: + - resource "lemurs.service": a service that omits state leaves the unit's lifecycle to the + machine, and boot and takes-over are both its lifecycle +``` + +The rule is the host's declaration validation. The controller's check never applies it, so a manifest +can pass every check the catalogue has and still fail on the first machine that receives it. + +## Why it matters beyond this instance + +A refusal is whole, so one module with this fault blocks every other change for that node until the +module is fixed, rebuilt and pushed again. The fault is static: it is in the manifest, and could be +found before a module is merged. Today it is found by assigning the module to a live machine. + +## Open questions + +1. Should the host's declaration validation be importable, so that `module check` (and registration) + runs it over each resource the manifest declares? +2. Or should the controller validate the composed declaration before it sends it, and refuse to send + one the host would refuse? +3. Which other host-side rules are not visible to the catalogue check today? + +## Resolved — 2026-10-08 + +The merge gate (mesh-controller PR #96) composes each machine with the change and runs the node-engine's +own declaration validation over it, failing the pull request with the machine and the module. R236 in +mesh-lab's replays register holds it. diff --git a/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/345-a-reminder-window-shows-transparent-and-empty/00-report.md b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/345-a-reminder-window-shows-transparent-and-empty/00-report.md new file mode 100644 index 00000000..44fbcfb1 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/345-a-reminder-window-shows-transparent-and-empty/00-report.md @@ -0,0 +1,64 @@ +--- +status: diagnosing +opened: 2026-10-09 +located-in: [] +fixed-by: +amended-design: +--- + +# 345. A calendar reminder shows as a transparent, empty window, and the mesh had no tool to look at it + +## Symptom + +On 2026-10-09 the operator reported that on the workstation, when an Outlook (Exchange) meeting is +due, the mail client's reminder window appears, but it is transparent and shows no content: no title, +no time, no Dismiss or Snooze. The mail client is Betterbird (a Thunderbird fork, installed from the +AUR), with the Exchange calendar through the Owl add-on. The workstation runs i3 and the `picom` +compositor (glx backend, vsync, fading). No module owned Betterbird. + +The operator's view of done: *on the workstation, when an Outlook meeting is due, the reminder window +shows the meeting (title, time, Dismiss/Snooze) on a normal background, not a transparent empty window, +and the mesh has tools to see and fix Betterbird.* + +## Reported history + +The operator reports (2026-10-09) earlier trouble with picom on the workstations, never recorded: a chat +window (Telegram) froze, not redrawn, after a change of i3 workspace. `records_search` for picom finds +nothing about it. It is the same kind of failure as this one, a window the compositor shows without its +content, and it makes picom the first suspect here. + +## Why it is an issue + +Two gaps, and the symptom itself. + +1. **A desktop application the operator uses every day ran outside the mesh.** Nothing could say which + window the reminder is, what the compositor makes of it, or which preferences the mail client runs + with. A change to it could only be made by hand in its profile, which the mesh's rules forbid and + which nothing would keep. +2. **The tools that look at a screen could not look at this window.** The window manager's list + (`node-display-session.windows`) leaves out every window it does not tile, the reminder among them, + and says nothing a compositor decides by (a window's depth, type, opacity). The screenshot tool of the + display server failed for one window with `X Error BadColor (invalid Colormap parameter) + X_QueryColors`, because it dumped the window by its id. +3. **The symptom's cause is not known yet.** A reminder that shows nothing is a missed meeting. + +The diagnosis is in [01-diagnosis.md](01-diagnosis.md). + +## Impact + +- The operator cannot read or dismiss a meeting reminder: the window is there but empty. +- Until now, the next session that looked at it had to work around the mesh to see a window. + +## What is done + +- The display server's module gains a window list that includes unmanaged windows with their depth, + type, state, role and own opacity, and its screenshot of one window is cut from the screen as the + compositor shows it (catalogue pull request, linked from this record's pull request). +- A `betterbird` module: status, named preferences without credentials or accounts, a user.js declared + through the module's setting, its log, restart, check, and a reminder on demand from a throw-away + profile, so a fix can be proved without touching the operator's calendar. + +- The `picom` module's backend and damage tracking become settings of each machine (defaults unchanged: + `glx`, `use-damage` true), so the candidate fixes can be set and proved without a hand edit. + +What is not done: the cause, and so the fix. See the diagnosis. diff --git a/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person/00-report.md b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person/00-report.md new file mode 100644 index 00000000..5b5a2e3c --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/testdata/hq/04-ISSUES/362-two-merges-seconds-apart-left-their-delivery-waiting-for-a-person/00-report.md @@ -0,0 +1,79 @@ +--- +status: resolved +opened: 2026-10-10 +located-in: [mesh-catalog modules/mesh-delivery (holder.go deliveryWalking), mesh-controller cmd/mesh-controller (upgrades.go SourceMoved, release_plan.go laterOnTheBranch)] +fixed-by: mesh-catalog#171, mesh-controller#197 +replay: R362 +amended-design: 03-DESIGN/01-to-be/47-delivery-from-commit-to-delivered.md +--- + +# 362. Two catalogue merges seconds apart left their delivery waiting for a person + +## Symptom + +Shortly after midnight on 2026-10-10 (local time) two pull requests of the catalogue were merged +18 seconds apart: the claude-code change at 00:20:52, then the dunst change at 00:21:09. Both had passed +their checks. + +`mesh-delivery.deliveries` then showed every delivery of the dunst pull request `superseded`, and the +claude-code delivery `held`, waiting for "a person: merged, and the controller opened no walk for it within +10m0s — nothing it holds follows that branch, or the merge was not heard: a person decides". The +controller's `plans` said of the walk for the catalogue's head, which carried both merges: "published; its +walk waits for mesh-delivery's word — `plans go … --why …` starts it by hand". + +Both reasons were wrong. A walk had been opened, and it contained the held merge. Nothing gave it the word. +The operator's change sat undelivered for 58 minutes, until the operator's hand-act `mesh-controller.plans +go` started the walk. **No condition asked anyone to act in that time**: the hold is bounded at 24 hours +before `mesh-delivery.stalled` lists it, and the walk's own wait raised only a warning, at 00:51, which said +there was nothing to do and pointed at the ended delivery (the diagnosis's second entry; this sentence said +before that the walk's wait raised nothing). + +## Why it is an issue + +A merge that passed its checks must reach its nodes without a person, and a delivery that cannot must say +so loudly and truly. Here both failed: two ordinary merges close together, which happens whenever the +operator merges a batch, stopped the mesh's delivery silently, and the reason it gave pointed away from the +cause. + +## Where it lives + +The diagnosis ([01-diagnosis.md](01-diagnosis.md)) traces it to two parts that each behave as written and +disagree on what a walk's commit means: + +- The controller plans a merge it hears after a later merge of the same branch at the later merge's commit, + and supersedes the earlier walk (the fix for + [issue 349](../349-a-merge-acted-on-late-superseded-the-merge-after-it-and-built-the-older-commit/00-report.md)). + So the claude-code merge got a walk at the dunst merge's commit. +- mesh-delivery finds the delivery a walk walks by its walk id or by the commit it was merged as. The new + walk matched the dunst delivery, which its first walk's supersession had already ended, so no live + delivery held the walk and nothing gave it its turn. The claude-code delivery, merged as a different + commit, never got a walk and was held after ten minutes. + +## Open + +- How mesh-delivery learns which merges a walk carries: the controller's walk record naming every merge it + answers (its commit and the commits planned into it), or mesh-delivery reading containment itself, as it + already does to decide *covered*. +- A superseded walk whose successor matches only final deliveries: whether the successor should be adopted + as a delivery of its own, so that something always holds a waiting walk. +- A walk waiting for mesh-delivery's word past a bound, and a delivery held because no walk was opened, + raise a condition to the operator instead of waiting silently (failure must be loud). + +## Decided + +The operator decided the model that removes this shape, not only its bookkeeping: +[ADR 0276](../../02-DECISIONS/0276-merges-are-assembled-in-a-rolling-window-and-each-batch-is-delivered-by-one-walk-with-one-commit-per-repository.md). Merges are collected in a rolling merge window and each batch is walked once, with at most one +commit per repository, one walk at a time. The bookkeeping that tied a walk to an ended delivery is repaired as +well, and every wait of this shape has a bound and a condition. It is designed in to-be 47, *The merge window +and the batch*. + +## Resolved + +2026-10-10. mesh-delivery's side is mesh-catalog#171 (merged): a walk is tied to every live delivery whose +merge it answers, never to one that has ended, and a waiting walk no live delivery holds is listed by `stalled` +at once. The controller's side is mesh-controller#197: the merge window and the batch, one walk per batch with +one commit per repository, the walk naming every merge it answers, and the bounds of decision 9. The replay +R362 is registered in mesh-lab's replays register (mesh-lab#75), proved to fail on the commit before the +mesh-delivery fix and pass on it. The build raised one rule the record had not settled, decided during the +build and written into ADR 0276: a merge on the controller's own path never shares a batch with a catalogue +merge. diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker.go b/modules/mesh-issues/cmd/mesh-issues/tracker.go index 96222c49..c875544b 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker.go @@ -26,6 +26,8 @@ type Tracker struct { Logf func(format string, a ...any) // Sleep waits between rounds of a counter that other openers keep winning. Sleep func(time.Duration) + // Records is the record the import reads; nil is the records module over the bus. + Records Records } // The events this module says, one per kind of change. @@ -119,16 +121,9 @@ func (t *Tracker) Open(a OpenAsk) (*Opened, error) { if record, err = checkSlug(a.Record); err != nil { return nil, err } - if len(located) > 0 { - return nil, errors.New("an issue with a folder in hq keeps located-in in that folder's report until " + - "the import: open it without located-in, or without record") - } } at := t.now() - is := Issue{Title: title, Report: report, Opened: at, OpenedBy: t.by(), LocatedIn: located, Opening: newID()} - if record == "" { - is.Status = StatusOpen - } + is := Issue{Title: title, Status: StatusOpen, Report: report, Opened: at, OpenedBy: t.by(), LocatedIn: located, Opening: newID()} n, err := t.next() if err != nil { @@ -144,7 +139,7 @@ func (t *Tracker) Open(a OpenAsk) (*Opened, error) { out := &Opened{Number: n, Title: title, Status: is.Status, LocatedIn: located, Record: is.Record, By: is.OpenedBy} out.Said = fmt.Sprintf("Opened issue %d: %s.", n, title) if is.Record != "" { - out.Said += fmt.Sprintf(" Its narrative goes in hq's %s/00-report.md, which keeps its status.", is.Record) + out.Said += fmt.Sprintf(" Its narrative goes in hq's %s/00-report.md; its status stays here.", is.Record) } out.Announced, out.Warning = t.emit(EventOpened, map[string]any{ "number": n, "title": title, "status": is.Status, "located-in": located, "record": is.Record, @@ -377,10 +372,6 @@ func (t *Tracker) Move(a MoveAsk) (*Moved, error) { var changed []string is, err := t.change(a.Number, func(is *Issue) error { changed = nil - if is.Record != "" { - return fmt.Errorf("issue %d keeps its status in its folder %s until the import of slice 3 has run: "+ - "change it there. The tracker holds its number, title, notes and links", is.Number, is.Record) - } from = is.Status if from == to { return fmt.Errorf("issue %d is already %s", is.Number, to) @@ -391,7 +382,7 @@ func (t *Tracker) Move(a MoveAsk) (*Moved, error) { changed = append(changed, name) } } - set("located-in", located != nil, func() { is.LocatedIn = located }) + set("located-in", located != nil, func() { is.LocatedIn, is.LocatedInUnverified = located, nil }) set("fixed-by", fixed != nil, func() { is.FixedBy = fixed }) set("amended-design", designs != nil, func() { is.AmendedDesign = designs }) set("replay", replay != "", func() { is.Replay, is.ReplayNone = replay, "" }) @@ -591,9 +582,6 @@ type Listed struct { Said string `json:"said"` } -// InFolder is the status a list shows for an issue whose folder in hq keeps its status. -const InFolder = "kept in its folder" - // List is the issues that match, newest first. func (t *Tracker) List(a ListAsk) (*Listed, error) { status := strings.TrimSpace(a.Status) @@ -649,11 +637,7 @@ func (t *Tracker) List(a ListAsk) (*Listed, error) { } out.Matched++ if len(out.Rows) < limit { - st := is.Status - if is.Record != "" { - st = InFolder - } - out.Rows = append(out.Rows, Row{Number: n, Status: st, Owner: strings.Join(is.LocatedIn, "; "), Title: is.Title}) + out.Rows = append(out.Rows, Row{Number: n, Status: is.Status, Owner: strings.Join(is.LocatedIn, "; "), Title: is.Title}) } } switch { diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker_test.go b/modules/mesh-issues/cmd/mesh-issues/tracker_test.go index eedbe248..9e0dd22e 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker_test.go @@ -72,7 +72,6 @@ func TestARefusedOpenSpendsNoNumber(t *testing.T) { {Title: "t\x1b[31m", Report: "r"}, {Title: "t‮", Report: "r"}, {Title: "t", Report: "r\x00"}, - {Title: "t", Report: "r", Record: "a-folder", LocatedIn: []string{"mesh-controller"}}, } for i, a := range refused { if _, err := h.Open(a); err == nil { @@ -244,18 +243,18 @@ func TestAnIssueWithAFolderIsNumberedTitledAndPointsAtItsFolder(t *testing.T) { if err != nil { t.Fatal(err) } - if o.Record != "04-ISSUES/365-the-counter-skips-a-number" || o.Status != "" { + if o.Record != "04-ISSUES/365-the-counter-skips-a-number" || o.Status != StatusOpen { t.Fatalf("answer %+v", o) } - _, err = h.Move(MoveAsk{Number: 365, Status: StatusDiagnosing}) - if err == nil || !strings.Contains(err.Error(), "keeps its status in its folder") { + // Since the import of slice 3, an issue with a folder is whole in the tracker: its status moves here. + if _, err := h.Move(MoveAsk{Number: 365, Status: StatusDiagnosing}); err != nil { t.Fatalf("a move of an issue with a folder: %v", err) } if _, err := h.Note(365, "notes are taken"); err != nil { t.Fatal(err) } l, _ := h.List(ListAsk{}) - if l.Rows[0].Status != InFolder { + if l.Rows[0].Status != StatusDiagnosing { t.Fatalf("listed %+v", l.Rows) } } diff --git a/modules/mesh-issues/module.json b/modules/mesh-issues/module.json index f1607758..f0a10775 100644 --- a/modules/mesh-issues/module.json +++ b/modules/mesh-issues/module.json @@ -12,7 +12,8 @@ "list", "move", "note", - "link" + "link", + "import" ] } ], @@ -26,7 +27,8 @@ "list", "move", "note", - "link" + "link", + "import" ] } ], @@ -34,7 +36,8 @@ "opened", "moved", "noted", - "linked" + "linked", + "imported" ], "state": [ {