diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f851db..7a993bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ Versioning, with pre-1.0 compatibility rules described in - Changed: passwords and other secrets that are typed into a prompt are redacted before a transcript is stored (REQ-526, PAIN-17). Until now only tokens with a fixed prefix (private keys, `sk-ant-`, `sk-`, `ghp_`, `AKIA`, `xox`, JWT) were replaced, so "Passwort: …" stayed in `session_chunks.text` and `.raw`, in both full-text indexes, in exports and in backups. The new rule replaces the value behind a label and keeps the label: `Passwort: Hx7!kQ2pz` becomes `Passwort: [REDACTED:labeled]`. Labels, German and English, any case, also at the end of a compound or after an underscore or camel case hump (`Mailpasswort`, `DB_PASSWORD`, `dbPassword`): password, passwd, pwd, pw, pass, passphrase, Passwort, Kennwort, secret (also `secret key`, `client secret`), api key / api_key / apikey, token, access token, bearer, PIN, Zugangsdaten, credentials. A separator may follow (`:`, `=`, `:=`, `=>`, `->`, a quote mark, `ist` / `is` / `lautet`, markdown bold) or nothing but a blank (`Mailkonto Passwort Hx7!kQ2pz`). A value counts only if it looks like a secret: one token without blanks of 6 to 200 characters that has at least two kinds of characters from lower case, upper case, digit and special character, one of them a digit or special character (a single kind, such as `correcthorsebatterystaple`, only from 12 characters and only behind `:` or in quotes or in `KEY=value`); after a bare label (`Passwort Sommer`) it needs a digit or two special characters; a PIN is 4 to 8 digits. It is not a secret if it is a placeholder (`${DB_PASSWORD}`, `$PW`, ``, `****`, `xxxx`, `changeme`, `your-token`), code (`r.FormValue("password")`, `os.Getenv(…)`, `process.env.X`, a type, a struct tag, a call), a path or a URL, or an identifier, version or number. Prose such as "Passwort zurücksetzen" or "das Passwort ist zu kurz" stays as it is. The password in `scheme://user:password@host` is replaced by `[REDACTED:url]`. The rule also runs on the written form of a JSON line: an escaped quote, line break or `\uXXXX` inside a value is part of it, and the line stays valid JSON. - Changed: the server redacts on arrival, in `AppendChunks` and `AppendChunkBatches`, next to the image placeholder, on `text` and on `raw`, so clients that still upload unredacted text are covered. `ctx watch` uses the JSON-aware form for `raw`. The question before sharing a session with guests uses the same detection (`FindSecretsIn(raw, JSON)`) and now finds a password behind a label; sessions stored after this change have nothing left to ask about. Document writes are refused for the same labeled secrets, not only for prefixed tokens. Not changed: knowledge entries, requests, coordination messages and notes are not redacted when written (human tasks, explanations and work notes are refused when they look like a secret, as before); a value an agent copied into one of them stays there. - Added: `ctx redact-sessions --db [--dry-run] [--batch 200] [--sleep 20ms] [--after-id N] [--skip-optimize]` applies the same redaction to the chunks already stored, and to every copy of their text: `session_chunks.text` and `.raw`, `chunks_fts` (the full-text index over `text`: the old entry is deleted with its old text and the new one inserted), `chunk_index` and `sess_fts` (the search index of the session view, rebuilt from the redacted `raw`), `sessions.title` where it was cut from the prompt, and the quotes in `knowledge_evidence` and `request_sightings`. Deleting an entry from an FTS5 index only writes a deletion mark, the words stay in the old segments, so both indexes are merged with `optimize` at the end, and the write-ahead log is truncated; the run uses `secure_delete`. It runs beside the server (own connection without migrations, short transactions, waits for locks, walks `session_chunks` by id), is idempotent, can be stopped and continued, and reports counts per table and per kind (`raw/labeled:password`), never a value, plus the knowledge entries and requests whose evidence comes from a changed chunk (their text is not touched). Pages freed before the run and older backups still hold the old text until `VACUUM` (with the server stopped) and until the backups are replaced; see `deploy/README.md`. Context snapshots are immutable and are not touched. +- Added: the gardener consolidates and asks about regression protection (REQ-457, jobs 3 and 5, part A). It still only proposes: what it files is marked as coming from the gardener (harness `gardener`, confidence `staged`), and every change to another entry is a button a person presses on the review page. + - Duplicates: a duplicate note on `/ui/review` now has **Merge**. The card first shows which entry stays (the newest) and which are replaced; pressing it supersedes the older entries by the newer one with the same store call and the same right as `context_supersede` (the right to change every entry involved), the reason is "Gärtner-Vorschlag #note, bestätigt von person", and the note is closed as confirmed. It is refused (409) when an entry is no longer current, the scopes differ or the entry that stays is not trusted yet. The duplicate note no longer has plain approval, and "Approve all" skips it. + - Contradictions: `ctx garden check-knowledge --propose` opens a thread per validated contradiction (instead of a staged note), linked to every entry it is about, titled as a question and marked "[Gärtner]", with the reason and the verbatim quotes as its first post, written by the gardener's own account. The gardener registers as the agent `gardener:` in the project room for that. For the same entries there is never a second open thread, a thread a person closed (or archived, or parked) is not reopened and not answered with a new one while the entries are unchanged (the thread's question records the wording it was about), and if the existing threads cannot be looked up nothing is opened. A contradiction whose thread cannot be opened is filed as a staged note. The gardener has no call that sets a thread's state. + - Regression protection: in the same sessions, without extra ones, the gardener asks about open pitfalls (nobody has judged them with `context_regression_cover`, and it has no proposal standing for them), at most eight per session. A session names a test that exists (file and name; `func TestX` has to be in a tracked `_test.go` file the session read, other languages: the name as a word), sketches one (file, name, 3 to 6 lines of steps) or says "not testable" with a reason; `ctx` drops every answer the checkout does not bear out. Each answer is a staged gardener note. On the review page **Record as covered** (existing test) and **Record as not testable** call the regression verdict (`covered` with the test, or `not_applicable`) with the right to verify; a sketch only has the plain approve and reject. Sketches also get a section of their own ("Testskizzen") in the morning report written with `--out`. + - The answer format grows by an optional `regression` field, so older answers and prompts still work; the question sits in the task of a package that has such pitfalls (about 1,000 characters once per session, about 1.4 % of a package of 30 pitfalls with 2,000 characters each) and not in the standing instruction. No schema change. + - Operations: set the gardener's account on the server host with `ctx account set-gardener --db ` (it works on the database like the other `ctx account` commands, looks the account up once and stores its number in `org_state`, no schema change; the running server uses it at once; `--clear` takes it away; `ctx account list` marks it `gardener=true`; `ctx serve` logs `gardener_account` at start). Only that account may write an entry with harness `gardener` or a session reference starting with `gardener:` (also through `POST /api/knowledge` with `supersedes` and `POST /api/migrated-knowledge`); everyone else gets 403 `gardener_reserved`, and so does everyone while no account is set, so run the command on every server that runs the gardener before the next run. The role is the account number and never a name: renaming the account changes nothing, and an account that takes its old name gets nothing. The review actions Merge, Record as covered and Record as not testable act only on a note that this account wrote (by the stored account number, not by the label), that nobody has edited since, and whose title names the entries its reference names; otherwise the card shows "Not vouched for as the gardener's: no action", the request is refused with 403 and nothing changes, and without a gardener account the gardener notes offer no action at all. The card shows the entries from the reference with title and link. The test of a regression note must have the shape `path/to/x_test.go:TestName` and is recorded as the note holds it (the request carries no test of its own); the server cannot check it against a checkout, `ctx` did before filing the note. + - New tests record every call that is not a read of a gardener run in agent mode with a stand-in for Claude Code against the real server: only staged gardener notes, the registration in the room, threads, their links and first post; nothing that patches, supersedes, retires, judges, sets a thread's state, sets a criterion or closes a request; threads and posts belong to the gardener's account; entries and requests are unchanged; a second run adds nothing; and the gardener's own account (a member) is refused for every one of those steps. The review actions are tested with a guest, a stranger, a member, a reviewer and a lead. - Changed: stored session transcripts no longer keep base64 images and files (REQ-517, PAIN-12). Screenshots in tool results made up two thirds of the 1.7 GB the database grew by per day, yet search and the session view only ever used text. The server now replaces every base64 payload of 1 KB or more in a chunk's `raw` line with a placeholder such as `[ghosttree: Bild entfernt, image/png, 1234567 Bytes, sha256:0123456789abcdef]` (type, size of the decoded file, first 16 hex digits of its SHA-256). It covers Claude's `{"type":"base64","media_type":…,"data":…}` (also inside `tool_result`, `message.content` and `toolUseResult`), Claude Code's `toolUseResult.file.base64`, MCP `{"type":"image","data":…,"mimeType":…}` and `blob`, and `data:;base64,…` strings (Codex `input_image`), for images, PDFs and audio alike. The line stays valid JSON with the same structure and every other byte untouched; a line without a payload is stored byte for byte. Old `ctx` versions are covered because the server does it, and `ctx watch` now strips before uploading, which also saves the transfer. `ctx export` therefore writes placeholders instead of images. Base64 inside a text string (JSON in JSON, the content of a `.b64` file) is not touched. - Added: `ctx strip-images --db [--dry-run] [--batch 200] [--sleep 50ms]` does the same for the chunks already stored. It runs beside the server (short transactions, waits for locks, walks `session_chunks` by id), is idempotent, can be stopped and resumed, and reports the chunks checked and changed and the bytes saved. The file shrinks only after `VACUUM` with the server stopped; see `deploy/README.md`. The image data is gone for good, so take a backup first. - Changed: knowledge that arrives with a prompt (`UserPromptSubmit`) now arrives once per session and only for words that are unusual in the reader's own language (REQ-521). The hook sends the harness session id (`session_id`, the same field in Claude Code and Codex) and the server remembers per account and session which entries it delivered, in the new table `context_deliveries` (created at open, no schema upgrade; rows older than seven days are removed whenever a delivery is recorded). The start-up briefing records what it actually carried, so a pitfall that was already in the bootstrap is not sent again when a prompt names it, while an entry the budget cut off can still arrive; without a session id nothing changes. A word is distinctive only if it also passes a second gate besides being rare among the entries: it must be unusual in the reader's own session text, so "dinge" or "gärtner" no longer fetch unrelated entries while a codename such as `hlpvpngw` does. The gate counts only chunks of sessions of the caller's own account (never anyone else's, so a prompt cannot probe whether a word is frequent in other people's sessions), in the caller's most recent sessions (at most 24 sessions or 40,000 chunks), counted in `chunks_fts` per session up to a ceiling of one chunk in two thousand of that sample (at least 8). An account with fewer than 5,000 chunks of its own has no gate, as before; a word that few chunks carry anywhere is settled with one short look at the index without counting. A word that at most two entries name now also matches the text of an entry, not only its title. Staged entries and the gardener's notes are never delivered this way. Every delivered entry ends with `(triggered by "word")`; the heading `## Possibly relevant to what you just said (ghosttree)` is unchanged. Measured on a synthetic archive of 1.14 million chunks (four accounts, sessions uploaded interleaved) and 3000 entries: 1.4 to 2.2 ms per prompt in the store for ordinary prompts, at most 55 ms for a prompt of 24 words that other accounts use all the time, whether the caller has many sessions or few. `ctx usage --relevance` shows the word behind each replayed hit and judges each prompt by the account that typed it. diff --git a/cmd/ctx/account.go b/cmd/ctx/account.go index faf761f..e6135b4 100644 --- a/cmd/ctx/account.go +++ b/cmd/ctx/account.go @@ -17,7 +17,17 @@ const accountUsage = `usage: ctx account add [--email E] [--admin] --db < ctx account token create [--label L] [--machine M] [--expires-in 720h] --db ctx account token revoke --db ctx account claim-code --db - ctx account login-link [--url https://host] --db ` + ctx account login-link [--url https://host] --db + ctx account set-gardener --db + ctx account set-gardener --clear --db + +set-gardener makes one account the gardener's, once and for good: the number of +the account is stored (name, or person:N, or the bare number is looked up once), +not its name, so renaming that account or giving its old name to another does not +change who the gardener is. Only that account may write entries with harness +"gardener" or a "gardener:" reference, and the review page acts only on notes it +wrote. The running server picks it up at once. Without one nobody may, and the +gardener's proposals are refused. Give it the role member in the project, not more.` // cmdAccount arbeitet wie person direkt auf der Datenbank: Konten und Tokens // auszustellen braucht Zugriff auf den Server-Host, nicht ein Token. @@ -37,6 +47,8 @@ func cmdAccount(args []string, stdout io.Writer) int { return cmdAccountClaimCode(args[1:], stdout) case "login-link": return cmdAccountLoginLink(args[1:], stdout) + case "set-gardener": + return cmdAccountSetGardener(args[1:], stdout) case "token": if len(args) > 1 && args[1] == "create" { return cmdAccountTokenCreate(args[2:], stdout) @@ -119,12 +131,86 @@ func cmdAccountList(args []string, stdout io.Writer) int { fmt.Fprintf(stdout, "list accounts: %v\n", err) return 1 } + gardener := st.GardenerAccountID() for _, a := range accounts { - fmt.Fprintf(stdout, "%s\t%s\tstate=%s admin=%t email=%s\n", a.ID, a.Name, a.State, a.Admin, a.Email) + fmt.Fprintf(stdout, "%s\t%s\tstate=%s admin=%t email=%s", a.ID, a.Name, a.State, a.Admin, a.Email) + if id, ok := store.AccountNumericID(a.ID); ok && id == gardener { + fmt.Fprint(stdout, " gardener=true") + } + fmt.Fprintln(stdout) } return 0 } +// cmdAccountSetGardener names the account the gardener runs as. Like the other +// account commands it works on the database itself: whoever has the host decides +// who the gardener is, and nothing a user can do over HTTP changes it. +func cmdAccountSetGardener(args []string, stdout io.Writer) int { + var unset *bool + _, who, db, ok := accountFlags("account set-gardener", args, stdout, func(fs *flag.FlagSet) { + unset = fs.Bool("clear", false, "take the role away: nobody is the gardener afterwards") + }) + if !ok || (who == "") == !*unset { + fmt.Fprintln(stdout, "usage: ctx account set-gardener --db \n ctx account set-gardener --clear --db ") + return 2 + } + st, ok := openAccountStore(*db, stdout) + if !ok { + return 1 + } + defer st.Close() + before, wasSet := st.GardenerAccount() + if *unset { + if err := st.ClearGardenerAccount(); err != nil { + fmt.Fprintf(stdout, "clear gardener account: %v\n", err) + return 1 + } + fmt.Fprintln(stdout, "no gardener account is set: nobody may write gardener entries, and the gardener's proposals are refused") + return 0 + } + acct, err := resolveAccountRef(st, who) + if err != nil { + fmt.Fprintf(stdout, "%v\n", err) + return 1 + } + id, _ := store.AccountNumericID(acct.ID) + if err := st.SetGardenerAccount(id); err != nil { + fmt.Fprintf(stdout, "set gardener account: %v\n", err) + return 1 + } + fmt.Fprintf(stdout, "the gardener's account is %s (%s); the number is what counts, a rename changes nothing. The server picks it up at once.\n", acct.ID, acct.Name) + if acct.Admin { + fmt.Fprintln(stdout, "note: this account is an administrator; the gardener needs the role member in the project and no more") + } + if wasSet && before.ID != acct.ID { + fmt.Fprintf(stdout, "it was %s (%s); that account may not write gardener entries any more\n", before.ID, before.Name) + } + return 0 +} + +// resolveAccountRef finds the account a person named on the command line: a +// number ("7" or "person:7") by its number, anything else by its name. The +// number is what the caller stores. +func resolveAccountRef(st *store.Store, ref string) (store.Account, error) { + ref = strings.TrimSpace(ref) + byNumber := ref + if _, err := strconv.ParseInt(ref, 10, 64); err == nil { + byNumber = "person:" + ref + } + if strings.HasPrefix(byNumber, "person:") { + a, err := st.AccountByPrincipalID(byNumber) + if err != nil { + return store.Account{}, fmt.Errorf("no account %s", byNumber) + } + return a, nil + } + a, err := st.AccountByName(store.NormalizeAccountName(ref)) + if err != nil { + return store.Account{}, fmt.Errorf("no account named %q (ctx account list shows them)", ref) + } + return a, nil +} + func cmdAccountTokens(args []string, stdout io.Writer) int { _, name, db, ok := accountFlags("account tokens", args, stdout, nil) if !ok || name == "" { diff --git a/cmd/ctx/account_gardener_test.go b/cmd/ctx/account_gardener_test.go new file mode 100644 index 0000000..cc2de0a --- /dev/null +++ b/cmd/ctx/account_gardener_test.go @@ -0,0 +1,99 @@ +package main + +import ( + "bytes" + "path/filepath" + "strings" + "testing" + + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +// `ctx account set-gardener` is how the host's operator says who the gardener is: +// it looks the account up once and stores its NUMBER. +func TestAccountSetGardenerStoresTheNumberAndNothingElse(t *testing.T) { + db := filepath.Join(t.TempDir(), "ghosttree.db") + do := func(want int, args ...string) string { + t.Helper() + var out bytes.Buffer + if code := run(append(args, "--db", db), &out); code != want { + t.Fatalf("%v: exit=%d, want %d\n%s", args, code, want, out.String()) + } + return out.String() + } + do(0, "account", "add", "alice", "--admin") + do(0, "account", "add", "gardener") + do(0, "account", "add", "robot") + gardenerID := func() int64 { + t.Helper() + st, err := store.Open(db) + if err != nil { + t.Fatal(err) + } + defer st.Close() + return st.GardenerAccountID() + } + if got := do(0, "account", "list"); strings.Contains(got, "gardener=true") { + t.Fatalf("an account is marked as the gardener's before one is set:\n%s", got) + } + + // By name: looked up once, the number is stored. + out := do(0, "account", "set-gardener", "gardener") + if !strings.Contains(out, "person:2") || !strings.Contains(out, "(gardener)") || gardenerID() != 2 { + t.Fatalf("by name: %q, id %d", out, gardenerID()) + } + listed := do(0, "account", "list") + for _, line := range strings.Split(strings.TrimSpace(listed), "\n") { + if strings.Contains(line, "gardener=true") != strings.HasPrefix(line, "person:2\t") { + t.Errorf("the mark is on the wrong line: %q", line) + } + } + // By number, in both spellings; the earlier account is named as dropped. + if out := do(0, "account", "set-gardener", "person:3"); !strings.Contains(out, "it was person:2 (gardener)") || gardenerID() != 3 { + t.Errorf("by principal id: %q, id %d", out, gardenerID()) + } + if do(0, "account", "set-gardener", "2"); gardenerID() != 2 { + t.Errorf("by number: id %d", gardenerID()) + } + // An administrator is accepted with a warning. + if out := do(0, "account", "set-gardener", "alice"); !strings.Contains(out, "administrator") || gardenerID() != 1 { + t.Errorf("an administrator: %q, id %d", out, gardenerID()) + } + do(0, "account", "set-gardener", "gardener") + + // What cannot be an account is refused and leaves the setting as it was. + for _, bad := range []string{"nobody", "person:99", "99", "person:", "person:0", "0"} { + if out := do(1, "account", "set-gardener", bad); gardenerID() != 2 || out == "" { + t.Errorf("%q: %q, id %d", bad, out, gardenerID()) + } + } + do(2, "account", "set-gardener") + do(2, "account", "set-gardener", "--clear", "gardener") + + // The number is what is stored: renaming the account changes nothing. + st, err := store.Open(db) + if err != nil { + t.Fatal(err) + } + if _, err := st.SetOwnName("person:2", "garden-keeper"); err != nil { + t.Fatal(err) + } + st.Close() + if gardenerID() != 2 { + t.Error("renaming the account moved the role") + } + if got := do(0, "account", "list"); !strings.Contains(got, "garden-keeper") || !strings.Contains(got, "gardener=true") { + t.Errorf("list after the rename:\n%s", got) + } + // A name that is not a number is not looked up after the fact, either: the name + // "gardener" is free now, and setting it by name finds nobody. + do(1, "account", "set-gardener", "gardener") + if gardenerID() != 2 { + t.Error("a name that no account holds changed the setting") + } + + // And it can be taken away again. + if out := do(0, "account", "set-gardener", "--clear"); !strings.Contains(out, "no gardener account is set") || gardenerID() != 0 { + t.Errorf("clear: %q, id %d", out, gardenerID()) + } +} diff --git a/cmd/ctx/garden.go b/cmd/ctx/garden.go index f76ec67..dc73927 100644 --- a/cmd/ctx/garden.go +++ b/cmd/ctx/garden.go @@ -42,9 +42,13 @@ Two ways to run it: through the entries together: the entries are split into topics (code area of the files they name), one session per package checks them against the code, finds duplicates and contradictions - between entries, and answers with JSON. ctx checks that JSON - (every verdict needs a citation that exists in the checkout, every - relation a verbatim quote of each entry) before anything is filed. + between entries, and for pitfalls nobody has judged says which + test would catch the defect's return (one that exists, a sketch of + one, or "not testable", at most 8 per session), and answers with + JSON. ctx checks that JSON (every verdict needs a citation that + exists in the checkout, every relation a verbatim quote of each + entry, every named test has to be defined in a file the session + read) before anything is filed. The session can read the checkout (Read, Grep, Glob, only below it; git_log and git_show as fixed tools, no Bash) and, unless --no-mcp, search ghosttree read-only; it cannot write, has no network but the model proxy and ghosttree, never bypasses @@ -76,9 +80,19 @@ and calls nothing). --dry-run count tokens and cost, call nothing --max-usd stop cleanly before a call that could exceed this budget (default 2) --limit send at most this many entries to the model - --propose file stale and correction verdicts as staged notes marked as - coming from the gardener. Never verifies, deprecates or deletes - anything, and the entry checked is left untouched. + --propose file what the run found, and only that: + - stale and correction verdicts, duplicates between entries and + answers to the regression question as staged notes marked as + coming from the gardener (nothing is verified, deprecated, + superseded, retired or judged: a person confirms a note on the + review page, where a duplicate note offers "merge" and a + regression note offers to record the test or "not testable"); + - each contradiction between entries as a thread, linked to + all of them, marked as the gardener's, with the reason and the + verbatim quotes as its first post. For the same entries there + is never a second open thread, and a thread a person closed is + not opened again while the entries are unchanged. + The entries checked are left untouched. --out write the Markdown report to a file instead of stdout --db the ghosttree database. With it the gardener remembers, per entry, when and against which commit it looked (the next run takes entries @@ -95,10 +109,13 @@ and calls nothing). cleanly, names itself, and exits with status 3.` // gardenStore is what the command needs from the server: read the entries, -// and, with --propose, add new ones. There is no patch or delete in it. +// and, with --propose, add new ones and open threads for contradictions. There +// is no patch, supersede, retire, regression verdict or thread state in it: +// those are what a person does on the review page. type gardenStore interface { ProjectKnowledge(project string, includeArchived bool) ([]store.Knowledge, error) Remember(k store.Knowledge, autoCtx scope.Axes) (store.Knowledge, error) + garden.ThreadSink } // gardenState is what the gardener remembers between runs. It is the database @@ -400,12 +417,21 @@ func gardenDue(st gardenStore, project string, opts gardenOpts, stdout io.Writer fmt.Fprintf(stdout, "cannot read knowledge: %v\n", err) return nil, nil, 1, true } + // A rejected proposal of the gardener is an archived entry, and a pitfall it + // was already asked about is not asked again. + withArchived, err := st.ProjectKnowledge(project, true) + if err != nil { + fmt.Fprintf(stdout, "cannot read knowledge: %v\n", err) + return nil, nil, 1, true + } known = map[int64]garden.Entry{} for _, k := range all { if !opts.Types[k.Type] || k.Status != "active" || k.Scope.Project != project || k.Harness == garden.Harness { continue } - e := garden.Entry{ID: k.ID, Type: k.Type, Title: k.Title, Body: k.Body, ObservedAt: k.ObservedAt, CreatedAt: k.CreatedAt, UpdatedAt: k.UpdatedAt} + e := garden.Entry{ID: k.ID, Type: k.Type, Title: k.Title, Body: k.Body, ObservedAt: k.ObservedAt, CreatedAt: k.CreatedAt, UpdatedAt: k.UpdatedAt, + Regression: k.RegressionState} + e.AskCover = e.Type == "pitfall" && e.Regression == "" && !garden.CoverProposed(withArchived, e) entries = append(entries, e) known[e.ID] = e } @@ -469,11 +495,7 @@ func runGardenAgent(st gardenStore, repo *garden.Repo, project string, opts gard if meter.err != nil { fmt.Fprintf(stdout, "could not record usage: %v\n", meter.err) } - titles := map[int64]string{} - for id, e := range known { - titles[id] = e.Title - } - return finishGarden(st, project, opts, res, outPath, stdout, titles) + return finishGarden(st, project, opts, res, outPath, stdout, known) } func runGardenCheck(st gardenStore, repo *garden.Repo, project string, opts gardenOpts, model llm.UsageClient, pricing garden.Pricing, outPath string, stdout io.Writer) int { @@ -524,7 +546,7 @@ func runGardenCheck(st gardenStore, repo *garden.Repo, project string, opts gard // finishGarden writes the report, files the proposals and turns the way the // run ended into an exit status. It is the same for both modes. -func finishGarden(st gardenStore, project string, opts gardenOpts, res garden.Result, outPath string, stdout io.Writer, titles map[int64]string) int { +func finishGarden(st gardenStore, project string, opts gardenOpts, res garden.Result, outPath string, stdout io.Writer, known map[int64]garden.Entry) int { var report bytes.Buffer garden.WriteReport(&report, project, res, time.Now()) @@ -551,26 +573,9 @@ func finishGarden(st gardenStore, project string, opts gardenOpts, res garden.Re } if opts.Propose && !opts.DryRun { - existing, err := st.ProjectKnowledge(project, true) - if err != nil { - fmt.Fprintf(stdout, "cannot read existing proposals: %v\n", err) - return 1 - } - n, err := garden.Propose(st, project, existing, res.Verdicts) - if err != nil { - fmt.Fprintf(stdout, "%v (%d proposals filed before the failure; the report above is complete)\n", err, n) - return 1 - } - rn, err := garden.ProposeRelations(st, project, existing, res.Relations, titles) - if err != nil { - fmt.Fprintf(stdout, "%v (%d proposals filed before the failure; the report above is complete)\n", err, n+rn) - return 1 - } - fmt.Fprintf(stdout, "%s %d proposals filed as staged notes from the gardener (see 'ctx review')", garden.Headline(res.Verdicts), n+rn) - if rn > 0 { - fmt.Fprintf(stdout, ", %d of them about duplicates or contradictions between entries", rn) + if c := proposeGarden(st, project, res, stdout, known); c != 0 { + return c } - fmt.Fprintln(stdout) } if code != 0 { return code @@ -591,6 +596,83 @@ func finishGarden(st gardenStore, project string, opts gardenOpts, res garden.Re return 0 } +// proposeGarden files what the run found: staged notes for verdicts, duplicates +// and regression answers, a thread for each contradiction. It writes nothing else. +// A contradiction whose thread cannot be opened is filed as a staged note, so the +// finding is not lost. +func proposeGarden(st gardenStore, project string, res garden.Result, stdout io.Writer, known map[int64]garden.Entry) int { + existing, err := st.ProjectKnowledge(project, true) + if err != nil { + fmt.Fprintf(stdout, "cannot read existing proposals: %v\n", err) + return 1 + } + titles := map[int64]string{} + for id, e := range known { + titles[id] = e.Title + } + n, err := garden.Propose(st, project, existing, res.Verdicts) + if err != nil { + fmt.Fprintf(stdout, "%v (%d proposals filed before the failure; the report above is complete)\n", err, n) + return 1 + } + var dups, contradictions []garden.Relation + for _, r := range res.Relations { + if r.Kind == garden.Contradiction { + contradictions = append(contradictions, r) + } else { + dups = append(dups, r) + } + } + rn, err := garden.ProposeRelations(st, project, existing, dups, titles) + if err != nil { + fmt.Fprintf(stdout, "%v (%d proposals filed before the failure; the report above is complete)\n", err, n+rn) + return 1 + } + cn, err := garden.ProposeCovers(st, project, existing, res.Covers) + if err != nil { + fmt.Fprintf(stdout, "%v (%d proposals filed before the failure; the report above is complete)\n", err, n+rn+cn) + return 1 + } + outcomes := garden.OpenContradictionThreads(st, project, existing, contradictions, known) + var failed []garden.Relation + opened, kept := 0, 0 + for _, o := range outcomes { + switch o.State { + case garden.ThreadOpened: + opened++ + fmt.Fprintf(stdout, "contradiction %v: thread %d opened\n", o.Relation.IDs, o.ThreadID) + if o.Warn != "" { + fmt.Fprintf(stdout, " (%s)\n", o.Warn) + } + case garden.ThreadExists: + kept++ + fmt.Fprintf(stdout, "contradiction %v: thread %d is open already, no second one\n", o.Relation.IDs, o.ThreadID) + case garden.ThreadDecided: + kept++ + default: + failed = append(failed, o.Relation) + fmt.Fprintf(stdout, "contradiction %v: no thread (%v); filing a staged note instead\n", o.Relation.IDs, o.Err) + } + } + if len(failed) > 0 { + fn, err := garden.ProposeRelations(st, project, existing, failed, titles) + rn += fn + if err != nil { + fmt.Fprintf(stdout, "%v (%d proposals filed before the failure; the report above is complete)\n", err, n+rn+cn) + return 1 + } + } + fmt.Fprintf(stdout, "%s %d proposals filed as staged notes from the gardener (see 'ctx review')", garden.Headline(res.Verdicts), n+rn+cn) + if rn+cn > 0 { + fmt.Fprintf(stdout, ", %d of them about duplicates or contradictions between entries, %d about regression protection", rn, cn) + } + fmt.Fprintln(stdout) + if opened+kept > 0 { + fmt.Fprintf(stdout, "%d contradictions opened as threads, %d already dealt with\n", opened, kept) + } + return 0 +} + // writePrivate writes the report readable by its owner only: it quotes the // project's knowledge and code, and an existing file keeps its old mode unless // it is set again. diff --git a/cmd/ctx/garden_consolidate_test.go b/cmd/ctx/garden_consolidate_test.go new file mode 100644 index 0000000..6055dac --- /dev/null +++ b/cmd/ctx/garden_consolidate_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "bytes" + "errors" + "strings" + "testing" + + "github.com/Deadweight-Labs/ghosttree/internal/garden" + "github.com/Deadweight-Labs/ghosttree/internal/scope" + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +func pitfall(id int64, regression string) store.Knowledge { + return store.Knowledge{ID: id, Type: "pitfall", Title: "t", Body: "siehe cmd/x.go", Status: "active", Confidence: "trusted", + Scope: scope.Axes{Project: gardenProject}, ObservedAt: "2000-01-01T00:00:00Z", RegressionState: regression, UpdatedAt: "2026-10-01T00:00:00Z"} +} + +// The regression question is put to a pitfall nobody has judged and that the +// gardener has not already made a proposal about; a decision, a judged pitfall +// and one with a proposal standing (in any state) are not asked. +func TestGardenAsksTheRegressionQuestionOnlyOfOpenPitfalls(t *testing.T) { + judged := pitfall(2, "covered") + decision := pitfall(3, "") + decision.Type = "decision" + asked := pitfall(4, "") + rejected := pitfall(5, "") + changed := pitfall(6, "") + changed.UpdatedAt = "2026-10-10T00:00:00Z" + note := func(id int64, status string) store.Knowledge { + return store.Knowledge{ID: 100 + id, Type: "note", Harness: "gardener", SessionRef: garden.CoverRef(id, garden.CoverSketch), Status: status, + Confidence: "staged", Scope: scope.Axes{Project: gardenProject}, CreatedAt: "2026-10-05T00:00:00Z"} + } + st := &gardenFakeStore{entries: []store.Knowledge{pitfall(1, ""), judged, decision, asked, rejected, changed, + note(5, "deprecated"), note(6, "deprecated"), note(7, "active")}} + var out bytes.Buffer + opts := gardenOpts{Types: map[string]bool{"pitfall": true, "decision": true}} + entries, _, code, done := gardenDue(st, gardenProject, opts, &out) + if done || code != 0 { + t.Fatalf("done=%v code=%d %s", done, code, out.String()) + } + got := map[int64]bool{} + for _, e := range entries { + got[e.ID] = e.AskCover + } + // 5 has a rejected proposal for its current wording; 6 changed after the one it had. + want := map[int64]bool{1: true, 2: false, 3: false, 4: true, 5: false, 6: true} + for id, ask := range want { + if got[id] != ask { + t.Errorf("#%d: AskCover=%v, want %v", id, got[id], ask) + } + } + if len(got) != len(want) { + t.Errorf("entries = %v", got) + } +} + +// The thread is the way a contradiction is brought to a person. When it cannot be +// opened the finding is not lost: it is filed as a staged note, marked as the +// gardener's, like a duplicate. +func TestGardenFilesAContradictionAsANoteWhenItsThreadCannotBeOpened(t *testing.T) { + repo := gardenRepo(t) + st := &gardenFakeStore{threadErr: errors.New("refused"), entries: []store.Knowledge{ + {ID: 1, Type: "pitfall", Title: "eins", Body: "siehe cmd/x.go, Dienst läuft auf server-a", Status: "active", Confidence: "trusted", Scope: scope.Axes{Project: gardenProject}, ObservedAt: "2000-01-01T00:00:00Z"}, + {ID: 2, Type: "decision", Title: "zwei", Body: "siehe cmd/x.go, Dienst läuft auf server-b", Status: "active", Confidence: "trusted", Scope: scope.Axes{Project: gardenProject}, ObservedAt: "2000-01-02T00:00:00Z"}, + }} + fr := &gardenFakeRunner{text: func(string) string { + return `{"verdicts":[{"id":1,"verdict":"holds","reason":"r"},{"id":2,"verdict":"holds","reason":"r"}],` + + `"relations":[{"kind":"contradiction","ids":[1,2],"reason":"Ort","quotes":[{"id":1,"text":"Dienst läuft auf server-a"},{"id":2,"text":"Dienst läuft auf server-b"}]}]}` + }} + opts := gardenOpts{Types: map[string]bool{"pitfall": true, "decision": true}, MaxUSD: 1, Propose: true, Sessions: 4, PerSession: 30, Turns: 30, Runner: fr} + var out bytes.Buffer + if code := runGardenAgent(st, repo, gardenProject, opts, gardenPrice, "", &out); code != 0 { + t.Fatalf("exit %d: %s", code, out.String()) + } + if len(st.threads) != 0 || len(st.written) != 1 { + t.Fatalf("%d threads, %d notes written:\n%s", len(st.threads), len(st.written), out.String()) + } + k := st.written[0] + if k.Harness != "gardener" || k.Confidence != "staged" || !strings.Contains(k.Title, "Widerspruch?") || k.SessionRef != "gardener:relation:contradiction:1-2" { + t.Errorf("fallback note = %+v", k) + } + if !strings.Contains(out.String(), "filing a staged note instead") { + t.Errorf("the fallback is not said:\n%s", out.String()) + } + // And the next run, with the thread working, does not open one on top of a note somebody may have seen. + st.threadErr = nil + st.entries = append(st.entries, st.written...) + st.written = nil + runGardenAgent(st, repo, gardenProject, opts, gardenPrice, "", &out) + if len(st.threads) != 0 || len(st.written) != 0 { + t.Errorf("after the note: %d threads, %d notes", len(st.threads), len(st.written)) + } +} + +// Duplicates and answers to the regression question are staged notes; the +// regression answer is validated against the checkout first. +func TestGardenFilesRegressionAnswersAsStagedNotesAfterCheckingThem(t *testing.T) { + repo := gardenRepo(t) + st := &gardenFakeStore{entries: []store.Knowledge{pitfall(1, ""), pitfall(2, "")}} + fr := &gardenFakeRunner{text: func(prompt string) string { + if !strings.Contains(prompt, "[Regressionsfrage]") { + t.Errorf("the open pitfalls were not asked:\n%s", prompt) + } + return `{"verdicts":[{"id":1,"verdict":"holds"},{"id":2,"verdict":"holds"}],"regression":[` + + `{"id":1,"kind":"sketch","file":"cmd/x_test.go","test":"TestNewX","steps":"eins\nzwei\ndrei"},` + + `{"id":2,"kind":"existing","file":"cmd/x_test.go","test":"TestInvented"}]}` + }} + opts := gardenOpts{Types: map[string]bool{"pitfall": true}, MaxUSD: 1, Propose: true, Sessions: 4, PerSession: 30, Turns: 30, Runner: fr} + var out bytes.Buffer + if code := runGardenAgent(st, repo, gardenProject, opts, gardenPrice, "", &out); code != 0 { + t.Fatalf("exit %d: %s", code, out.String()) + } + if len(st.written) != 1 || st.written[0].SessionRef != "gardener:regression:1:sketch" { + t.Fatalf("written = %+v", st.written) + } + k := st.written[0] + if k.Harness != "gardener" || k.Confidence != "staged" || k.RegressionState != "" { + t.Errorf("note = %+v", k) + } + if !strings.Contains(out.String(), "1 about regression protection") { + t.Errorf("summary:\n%s", out.String()) + } +} diff --git a/cmd/ctx/garden_provenance_test.go b/cmd/ctx/garden_provenance_test.go new file mode 100644 index 0000000..37e7c44 --- /dev/null +++ b/cmd/ctx/garden_provenance_test.go @@ -0,0 +1,527 @@ +package main + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "sync" + "testing" + + "github.com/Deadweight-Labs/ghosttree/internal/client" + "github.com/Deadweight-Labs/ghosttree/internal/config" + "github.com/Deadweight-Labs/ghosttree/internal/garden" + requestdomain "github.com/Deadweight-Labs/ghosttree/internal/request" + "github.com/Deadweight-Labs/ghosttree/internal/scope" + "github.com/Deadweight-Labs/ghosttree/internal/server" + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +// REQ-457, criterion 3: everything the gardener writes is marked as the +// gardener's and is not trusted until a person confirms it; nothing is silently +// deleted, closed or overwritten. The tests below run the real command in agent +// mode, with a stand-in for Claude Code that answers with every kind of finding, +// against the real server and a real store, and record EVERY call that is not a +// read. + +type recordedWrite struct { + Method, Path string + Body []byte + Status int +} + +type writeRecorder struct { + inner http.Handler + mu sync.Mutex + writes []recordedWrite +} + +type statusWriter struct { + http.ResponseWriter + code int +} + +func (s *statusWriter) WriteHeader(code int) { + s.code = code + s.ResponseWriter.WriteHeader(code) +} + +func (r *writeRecorder) ServeHTTP(w http.ResponseWriter, req *http.Request) { + if req.Method == http.MethodGet || req.Method == http.MethodHead { + r.inner.ServeHTTP(w, req) + return + } + body, _ := io.ReadAll(req.Body) + req.Body = io.NopCloser(bytes.NewReader(body)) + sw := &statusWriter{ResponseWriter: w, code: http.StatusOK} + r.inner.ServeHTTP(sw, req) + r.mu.Lock() + r.writes = append(r.writes, recordedWrite{Method: req.Method, Path: req.URL.Path, Body: body, Status: sw.code}) + r.mu.Unlock() +} + +func (r *writeRecorder) count() int { + r.mu.Lock() + defer r.mu.Unlock() + return len(r.writes) +} + +func (r *writeRecorder) since(n int) []recordedWrite { + r.mu.Lock() + defer r.mu.Unlock() + return append([]recordedWrite(nil), r.writes[n:]...) +} + +const provenanceProject = "github.com/acme/ghosttree" + +var digits = regexp.MustCompile(`[0-9]+`) + +// The only calls the gardener may make that are not reads, and what each is for: +// a proposal (a staged note), the agent that speaks for it in the project room, +// a thread for a contradiction, its links and its first post. +var allowedGardenWrites = regexp.MustCompile(`^POST /api/(knowledge|coord/agents|threads|threads/[0-9]+/links|coord/messages)$`) + +type provenanceFixture struct { + t *testing.T + srv *httptest.Server + st *store.Store + rec *writeRecorder + repo *garden.Repo + dir string + claude string + ids map[string]int64 + request int64 + gardener *client.Client +} + +func newProvenanceFixture(t *testing.T) *provenanceFixture { + t.Helper() + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(dir, "config")) + t.Setenv("GHOSTTREE_LLM_CONFIG", filepath.Join(dir, "absent.json")) + t.Setenv("GHOSTTREE_GARDEN_KEY", "") + + st, err := store.Open(":memory:") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { st.Close() }) + // alice is the person who owns the project; the gardener is an account of its + // own with the plain role of a member, and not a reviewer. + if _, err := st.AddAccount("alice", "", true); err != nil { + t.Fatal(err) + } + if _, err := st.AddAccount("gardener", "", false); err != nil { + t.Fatal(err) + } + org, err := st.CreateOrg("person:1", "Acme", "acme") + if err != nil { + t.Fatal(err) + } + if _, err := st.EnsureProject("person:1", provenanceProject); err != nil { + t.Fatal(err) + } + code, _, err := st.CreateInvitation("person:1", org.ID, "", store.OrgMember, 0) + if err != nil { + t.Fatal(err) + } + if _, err := st.AcceptInvitation("person:2", code); err != nil { + t.Fatal(err) + } + if err := st.SetProjectRole("person:1", provenanceProject, "person:2", store.RoleMember, false, store.RoleViaCLI); err != nil { + t.Fatal(err) + } + st.SetAccessMode(store.AccessMode{Enforce: true}) + // The gardener's account is set by its number, as `ctx account set-gardener` + // does; without it the server refuses everything of the gardener's. + if err := st.SetGardenerAccount(2); err != nil { + t.Fatal(err) + } + token, _, err := st.CreateToken("gardener", store.TokenSpec{Label: "garden"}) + if err != nil { + t.Fatal(err) + } + + rec := &writeRecorder{inner: server.New(st)} + srv := httptest.NewServer(rec) + t.Cleanup(srv.Close) + cfg := config.Config{ServerURL: srv.URL, Token: token, Machine: "server-a"} + if err := config.Save(cfg); err != nil { + t.Fatal(err) + } + + // The checkout: a file the entries name, and a test the gardener can point to. + repoDir := filepath.Join(dir, "repo") + if err := os.MkdirAll(filepath.Join(repoDir, "cmd"), 0o755); err != nil { + t.Fatal(err) + } + for name, body := range map[string]string{ + "cmd/x.go": "package cmd\n", + "cmd/x_test.go": "package cmd\n\nimport \"testing\"\n\nfunc TestHoldsX(t *testing.T) {}\n", + } { + if err := os.WriteFile(filepath.Join(repoDir, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + for _, args := range [][]string{{"init", "-q"}, {"add", "."}, {"-c", "user.name=t", "-c", "user.email=t@example.invalid", "commit", "-q", "-m", "init"}, + {"remote", "add", "origin", "https://" + provenanceProject + ".git"}} { + if out, err := exec.Command("git", append([]string{"-C", repoDir}, args...)...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + repo, err := garden.OpenRepo(repoDir) + if err != nil { + t.Fatal(err) + } + + fx := &provenanceFixture{t: t, srv: srv, st: st, rec: rec, repo: repo, dir: dir, ids: map[string]int64{}, gardener: client.New(cfg)} + put := func(name, typ, title, body string) { + id, err := st.InsertKnowledge(store.Knowledge{Type: typ, Title: title, Body: body, Scope: scope.Axes{Project: provenanceProject}, + Person: "alice", Confidence: "trusted", ObservedAt: "2026-01-01T00:00:00Z"}) + if err != nil { + t.Fatal(err) + } + fx.ids[name] = id + } + put("where_a", "decision", "Der Dienst läuft auf server-a", "Siehe cmd/x.go. Der Dienst läuft auf server-a und nirgends sonst.") + put("where_b", "pitfall", "Der Dienst läuft auf server-b", "Siehe cmd/x.go. Der Dienst läuft auf server-b, nicht auf server-a.") + put("dup_a", "pitfall", "Die Datenbank liegt in einer Datei", "Siehe cmd/x.go. Die Datenbank liegt in einer einzigen SQLite-Datei.") + put("dup_b", "pitfall", "Eine Datei hält die Datenbank", "Siehe cmd/x.go. Die Datenbank liegt in einer einzigen SQLite-Datei, mehr gibt es nicht.") + put("tested", "pitfall", "Ein Pitfall mit Test", "Siehe cmd/x.go. Der Fall ist durch einen Test belegt.") + put("untested", "pitfall", "Ein Pitfall ohne Test", "Siehe cmd/x.go. Dafür gibt es noch keinen Test.") + put("tool", "pitfall", "Ein Pitfall über ein Werkzeug", "Siehe cmd/x.go. Das Werkzeug verhält sich so.") + if d, err := st.CreateRequest(requestdomain.CreateInput{Request: requestdomain.Request{Type: "feature", Title: "Offener Auftrag", Description: "d", + Scope: scope.Axes{Project: provenanceProject}, Person: "alice"}, Criteria: []string{"Das Kriterium ist offen"}}); err != nil { + t.Fatal(err) + } else { + fx.request = d.Request.ID + } + + // The stand-in for Claude Code: it reads the files, then answers with a stale + // verdict, a contradiction, a duplicate and all three kinds of regression answer. + ids := fx.ids + quote := func(id int64, text string) map[string]any { return map[string]any{"id": id, "text": text} } + verdicts := []map[string]any{} + for _, id := range ids { + v := map[string]any{"id": id, "verdict": "holds", "reason": "gilt weiter"} + if id == ids["where_b"] { + v = map[string]any{"id": id, "verdict": "stale", "reason": "überholt", "evidence": []string{"cmd/x.go:1"}} + } + verdicts = append(verdicts, v) + } + answer, _ := json.Marshal(map[string]any{ + "verdicts": verdicts, + "relations": []map[string]any{ + {"kind": "contradiction", "ids": []int64{ids["where_a"], ids["where_b"]}, "reason": "Zwei Orte für denselben Dienst.", + "quotes": []map[string]any{quote(ids["where_a"], "Der Dienst läuft auf server-a und nirgends sonst"), quote(ids["where_b"], "Der Dienst läuft auf server-b, nicht auf server-a")}}, + {"kind": "duplicate", "ids": []int64{ids["dup_a"], ids["dup_b"]}, "reason": "Beide sagen, dass die Datenbank in einer Datei liegt.", + "quotes": []map[string]any{quote(ids["dup_a"], "Die Datenbank liegt in einer einzigen SQLite-Datei"), quote(ids["dup_b"], "Die Datenbank liegt in einer einzigen SQLite-Datei, mehr gibt es nicht")}}, + }, + "regression": []map[string]any{ + {"id": ids["tested"], "kind": "existing", "file": "cmd/x_test.go", "test": "TestHoldsX", "reason": "Er prüft genau das."}, + {"id": ids["untested"], "kind": "sketch", "file": "cmd/y_test.go", "test": "TestNewY", "steps": "lege den Zustand an\nrufe die Funktion auf\nprüfe das Ergebnis"}, + {"id": ids["tool"], "kind": "untestable", "reason": "Eine Eigenschaft des Werkzeugs, nichts im Code."}, + }, + }) + read := func(n int, name string) string { + return fmt.Sprintf(`{"type":"assistant","message":{"id":"m%d","content":[{"type":"tool_use","id":"t%d","name":"Read","input":{"file_path":%q}}],"usage":{"input_tokens":10,"output_tokens":5}}}`, + n, n, filepath.Join(repoDir, name)) + } + result, _ := json.Marshal(map[string]any{"type": "result", "subtype": "success", "is_error": false, "num_turns": 3, "result": string(answer), + "usage": map[string]int{"input_tokens": 30, "output_tokens": 40}}) + fx.claude = filepath.Join(dir, "claude") + script := "#!/bin/sh\ncat > /dev/null\ncat <<'EOF'\n" + read(1, "cmd/x.go") + "\n" + read(2, "cmd/x_test.go") + "\n" + string(result) + "\nEOF\n" + if err := os.WriteFile(fx.claude, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + return fx +} + +// run starts the gardener the way the nightly unit does, with --propose. +func (fx *provenanceFixture) run() string { + fx.t.Helper() + var out bytes.Buffer + args := []string{"check-knowledge", "--project", provenanceProject, "--repo", fx.repo.Root, "--claude", fx.claude, + "--claude-dir", filepath.Join(fx.dir, "sessions"), "--sessions", "1", "--propose", "--no-mcp"} + if code := cmdGarden(args, &out); code != 0 { + fx.t.Fatalf("exit %d:\n%s", code, out.String()) + } + return out.String() +} + +func (fx *provenanceFixture) knowledge() map[int64]store.Knowledge { + fx.t.Helper() + all, err := fx.st.KnowledgeForProject(provenanceProject) + if err != nil { + fx.t.Fatal(err) + } + out := map[int64]store.Knowledge{} + for _, k := range all { + out[k.ID] = k + } + return out +} + +func (fx *provenanceFixture) count(query string, args ...any) int { + fx.t.Helper() + var n int + if err := fx.st.DB().QueryRow(query, args...).Scan(&n); err != nil { + fx.t.Fatal(err) + } + return n +} + +func TestGardenerWritesOnlyMarkedStagedProposalsAndThreadsAndNothingThatDecides(t *testing.T) { + fx := newProvenanceFixture(t) + before := fx.knowledge() + out := fx.run() + writes := fx.rec.since(0) + + // (c) Every call that is not a read is one of a short list; nothing patches, + // supersedes, retires, judges, sets a thread's state, sets a criterion or + // closes a request. A new kind of call fails here until somebody decides it belongs. + counts := map[string]int{} + for _, w := range writes { + call := w.Method + " " + w.Path + if !allowedGardenWrites.MatchString(call) { + t.Errorf("the gardener made a call that is not on the list: %s %s", call, w.Body) + } + if w.Status >= 300 { + t.Errorf("%s answered %d", call, w.Status) + } + counts[digits.ReplaceAllString(call, "N")]++ + } + want := map[string]int{"POST /api/knowledge": 5, "POST /api/coord/agents": 1, "POST /api/threads": 1, "POST /api/threads/N/links": 1, "POST /api/coord/messages": 1} + for call, n := range want { + if counts[call] != n { + t.Errorf("%s was called %d times, want %d (%v)", call, counts[call], n, counts) + } + } + + // (a) Every entry it wrote is a note of the gardener, staged, never verified, + // and does not replace anything. + notes := 0 + for _, w := range writes { + if w.Path != "/api/knowledge" { + continue + } + var body map[string]any + if err := json.Unmarshal(w.Body, &body); err != nil { + t.Fatal(err) + } + notes++ + if body["harness"] != "gardener" || body["confidence"] != "staged" || body["type"] != "note" || body["status"] != "active" || body["origin"] == "human" { + t.Errorf("a proposal is not marked as the gardener's and staged: %s", w.Body) + } + if _, ok := body["supersedes"]; ok { + t.Errorf("a proposal supersedes something: %s", w.Body) + } + if ref, _ := body["session_ref"].(string); !strings.HasPrefix(ref, "gardener:") { + t.Errorf("a proposal has no gardener reference: %s", w.Body) + } + } + after := fx.knowledge() + if notes != 5 || len(after) != len(before)+5 { + t.Fatalf("%d proposals written, %d entries new", notes, len(after)-len(before)) + } + for id, k := range after { + if _, old := before[id]; old { + continue + } + if k.Harness != "gardener" || k.Confidence != "staged" || k.Status != "active" || k.Person != "gardener" || k.ConfirmedBy != "" || k.RegressionState != "" { + t.Errorf("new entry #%d: %+v", id, k) + } + } + // What it checked is exactly as it was: no status, wording, verdict or successor changed. + for id, k := range before { + a := after[id] + if a.Confidence != k.Confidence || a.Status != k.Status || a.Title != k.Title || a.Body != k.Body || a.RegressionState != k.RegressionState || a.RegressionTest != k.RegressionTest || a.SupersededBy != 0 || a.UpdatedAt != k.UpdatedAt { + t.Errorf("entry #%d was changed:\nbefore %+v\nafter %+v", id, k, a) + } + } + for q, what := range map[string]string{ + `SELECT COUNT(*) FROM knowledge_relations`: "relations (supersede, merge)", + `SELECT COUNT(*) FROM knowledge_retirements`: "retirements", + `SELECT COUNT(*) FROM knowledge_versions`: "archived versions (an entry was rewritten)", + } { + if n := fx.count(q); n != 0 { + t.Errorf("%d %s were written", n, what) + } + } + + // (b) The thread and its post belong to the gardener's account and say so. + var title, state, person, author string + var archived int + if err := fx.st.DB().QueryRow(`SELECT title,state,archived,person,author_principal_id FROM threads`).Scan(&title, &state, &archived, &person, &author); err != nil { + t.Fatalf("no thread: %v", err) + } + if !strings.HasPrefix(title, "[Gärtner] ") || state != store.ThreadOpen || archived != 0 || person != "gardener" || author != "person:2" { + t.Errorf("thread = %q state %s archived %d by %s (%s)", title, state, archived, person, author) + } + if n := fx.count(`SELECT COUNT(*) FROM threads`); n != 1 { + t.Errorf("%d threads", n) + } + if n := fx.count(`SELECT COUNT(*) FROM thread_links WHERE object_kind='knowledge' AND object_id IN (?,?)`, fx.ids["where_a"], fx.ids["where_b"]); n != 2 { + t.Errorf("the thread is linked to %d of the two entries", n) + } + var sender, post string + var postAuthor string + if err := fx.st.DB().QueryRow(`SELECT sender_external_id,author_principal_id,body FROM coord_messages WHERE destination_kind=?`, store.DestinationDiscussion).Scan(&sender, &postAuthor, &post); err != nil { + t.Fatalf("no first post: %v", err) + } + if sender != garden.ThreadAgentID(provenanceProject) || postAuthor != "person:2" || !strings.Contains(post, "Herkunft gardener, nicht bestätigt") || + !strings.Contains(post, `Zitat: "Der Dienst läuft auf server-a und nirgends sonst"`) { + t.Errorf("first post by %s (%s):\n%s", sender, postAuthor, post) + } + // Criteria and requests: untouched. + d, err := fx.st.RequestByID(fx.request) + if err != nil || d.Request.State != "open" || len(d.Criteria) != 1 || d.Criteria[0].State != "open" { + t.Errorf("the request was touched: %+v %v", d, err) + } + // The contradiction is a thread, not a note; the report names what became of it. + if !strings.Contains(out, "thread 1 opened") || !strings.Contains(out, "5 proposals filed") { + t.Errorf("output:\n%s", out) + } + for _, k := range after { + if k.Harness == "gardener" && strings.HasPrefix(k.SessionRef, "gardener:relation:contradiction:") { + t.Errorf("a contradiction was filed as a note although its thread opened: %s", k.Title) + } + } +} + +// The same run again, a thread closed by a person, an entry changed: nothing is +// duplicated, nothing reopened, and only a change in an entry asks again. +func TestGardenerRunsAreIdempotentAndRespectWhatPeopleDecided(t *testing.T) { + fx := newProvenanceFixture(t) + fx.run() + first := fx.rec.count() + + fx.run() + for _, w := range fx.rec.since(first) { + // Only the gardener's own registration in the room is repeated; it is how it reads the threads. + if w.Path != "/api/coord/agents" { + t.Errorf("the second run wrote again: %s %s %s", w.Method, w.Path, w.Body) + } + } + if n := fx.count(`SELECT COUNT(*) FROM threads`); n != 1 { + t.Fatalf("%d threads after the second run", n) + } + if n := len(fx.knowledge()); n != 7+5 { + t.Fatalf("%d entries after the second run", n) + } + + // A person closes the thread. The gardener neither reopens it nor opens another. + if _, err := fx.st.DB().Exec(`UPDATE threads SET state='resolved', resolved_at='2026-10-20T08:00:00Z' WHERE id=1`); err != nil { + t.Fatal(err) + } + before := fx.rec.count() + fx.run() + for _, w := range fx.rec.since(before) { + if w.Path != "/api/coord/agents" { + t.Errorf("the run after the decision wrote: %s %s", w.Method, w.Path) + } + } + var state string + if err := fx.st.DB().QueryRow(`SELECT state FROM threads WHERE id=1`).Scan(&state); err != nil || state != store.ThreadResolved { + t.Errorf("thread 1 is %q (%v), a person closed it", state, err) + } + if n := fx.count(`SELECT COUNT(*) FROM threads`); n != 1 { + t.Errorf("%d threads: a closed one was answered with a new one while the entries are unchanged", n) + } + + // One of the entries changes: the question may be asked again, as a new thread. + b := fx.knowledge()[fx.ids["where_b"]] + if err := fx.st.UpdateKnowledgeBy(b.ID, map[string]string{"body": b.Body + " Seit dem Umzug."}, "alice"); err != nil { + t.Fatal(err) + } + fx.run() + if n := fx.count(`SELECT COUNT(*) FROM threads`); n != 2 { + t.Errorf("%d threads after a changed entry, want the old one and a new one", n) + } + if err := fx.st.DB().QueryRow(`SELECT state FROM threads WHERE id=1`).Scan(&state); err != nil || state != store.ThreadResolved { + t.Errorf("the old thread is %q (%v), it must stay as the person left it", state, err) + } +} + +// (d) What changes anything is a person's step on the review page. The account the +// gardener runs as, a member without the review right, cannot take any of them +// itself, whatever the program does: the server refuses. +func TestGardenerAccountCannotTakeTheStepsThatArePeoplesOnTheServer(t *testing.T) { + fx := newProvenanceFixture(t) + fx.run() + c := fx.gardener + ids := fx.ids + if err := c.SetRegressionCover(ids["tested"], "covered", "cmd/x_test.go:TestHoldsX"); err == nil { + t.Error("the gardener's account recorded a regression verdict") + } + if _, err := c.SupersedeKnowledge(ids["dup_b"], []int64{ids["dup_a"]}, "Gärtner-Vorschlag", "cli"); err == nil { + t.Error("the gardener's account superseded an entry") + } + if _, err := c.RetireKnowledge(ids["dup_a"], "weg"); err == nil { + t.Error("the gardener's account retired an entry") + } + if err := c.PatchKnowledge(ids["where_b"], map[string]string{"status": "deprecated"}); err == nil { + t.Error("the gardener's account deprecated an entry") + } + if err := c.PatchKnowledge(ids["where_b"], map[string]string{"confidence": "verified"}); err == nil { + t.Error("the gardener's account verified an entry") + } + for _, id := range ids { + k, _ := fx.st.KnowledgeByID(id) + if k.Status != "active" || k.Confidence != "trusted" || k.RegressionState != "" || k.SupersededBy != 0 { + t.Errorf("entry #%d changed through the gardener's account: %+v", id, k) + } + } +} + +// (a), from the server's side: the marks of the gardener are the gardener's alone. +// The account of a person, an owner included, cannot file a note that looks like +// the gardener's, with the real client and the real server; and the gardener run +// of an installation that has not named the account is refused at its first note. +func TestOnlyTheGardenersAccountCanFileNotesThatLookLikeTheGardeners(t *testing.T) { + fx := newProvenanceFixture(t) + aliceToken, _, err := fx.st.CreateToken("alice", store.TokenSpec{Label: "t"}) + if err != nil { + t.Fatal(err) + } + alice := client.New(config.Config{ServerURL: fx.serverURL(), Token: aliceToken, Machine: "laptop"}) + forged := store.Knowledge{Type: "note", Title: "[Gärtner] Test vorhanden? #5: x", Body: "Test: cmd/x_test.go:TestHoldsX", Scope: scope.Axes{Project: provenanceProject}, + Confidence: "staged", Origin: "distilled", Harness: "gardener", SessionRef: "gardener:regression:5:existing"} + if _, err := alice.Remember(forged, scope.Axes{Project: provenanceProject}); err == nil || !strings.Contains(err.Error(), "gardener_reserved") { + t.Errorf("the owner filed a note that looks like the gardener's: %v", err) + } + if _, err := fx.gardener.Remember(forged, scope.Axes{Project: provenanceProject}); err != nil { + t.Errorf("the gardener's own account is refused: %v", err) + } + + // The same run, on a server with no gardener account: the first proposal is refused, + // and nothing is filed as the gardener's. + fx2 := newProvenanceFixture(t) + if err := fx2.st.ClearGardenerAccount(); err != nil { + t.Fatal(err) + } + var out bytes.Buffer + args := []string{"check-knowledge", "--project", provenanceProject, "--repo", fx2.repo.Root, "--claude", fx2.claude, + "--claude-dir", filepath.Join(fx2.dir, "sessions"), "--sessions", "1", "--propose", "--no-mcp"} + if code := cmdGarden(args, &out); code == 0 { + t.Errorf("a run without a gardener account went through:\n%s", out.String()) + } + if !strings.Contains(out.String(), "gardener_reserved") { + t.Errorf("the refusal is not shown:\n%s", out.String()) + } + for _, k := range fx2.knowledge() { + if k.Harness == "gardener" { + t.Errorf("an entry of the gardener's was filed: %+v", k) + } + } +} + +func (fx *provenanceFixture) serverURL() string { return fx.srv.URL } diff --git a/cmd/ctx/garden_test.go b/cmd/ctx/garden_test.go index d08868a..323e6f0 100644 --- a/cmd/ctx/garden_test.go +++ b/cmd/ctx/garden_test.go @@ -40,6 +40,13 @@ func TestGardenWithoutKeyStopsWithOneLine(t *testing.T) { type gardenFakeStore struct { entries []store.Knowledge written []store.Knowledge + // What a thread needs: the threads opened, what they are linked to, their + // posts and the agents registered. threadErr makes opening a thread fail. + threads []store.Thread + links map[int64][]store.ThreadLink + posts []store.CoordMessage + agents []store.CoordAgent + threadErr error } func (f *gardenFakeStore) ProjectKnowledge(string, bool) ([]store.Knowledge, error) { @@ -51,6 +58,56 @@ func (f *gardenFakeStore) Remember(k store.Knowledge, _ scope.Axes) (store.Knowl return k, nil } +func (f *gardenFakeStore) RegisterCoordAgent(a store.CoordAgent) (int64, error) { + f.agents = append(f.agents, a) + return int64(len(f.agents)), nil +} + +func (f *gardenFakeStore) ThreadsForObject(kind, id, _ string) ([]store.Thread, error) { + var out []store.Thread + for _, t := range f.threads { + for _, l := range f.links[t.ID] { + if l.Kind == kind && l.ID == id { + out = append(out, t) + break + } + } + } + return out, nil +} + +func (f *gardenFakeStore) ThreadLinks(id int64, _ string) ([]store.ThreadLink, error) { + return f.links[id], nil +} + +func (f *gardenFakeStore) CreateThread(t store.Thread, _ string) (int64, error) { + if f.threadErr != nil { + return 0, f.threadErr + } + t.ID = int64(len(f.threads) + 1) + t.State = store.ThreadOpen + if f.links == nil { + f.links = map[int64][]store.ThreadLink{} + } + if t.Link != nil { + l := *t.Link + l.ThreadID = t.ID + f.links[t.ID] = append(f.links[t.ID], l) + } + f.threads = append(f.threads, t) + return t.ID, nil +} + +func (f *gardenFakeStore) LinkThread(l store.ThreadLink, _ string) error { + f.links[l.ThreadID] = append(f.links[l.ThreadID], l) + return nil +} + +func (f *gardenFakeStore) SendCoordMessage(m store.CoordMessage) (int64, error) { + f.posts = append(f.posts, m) + return int64(len(f.posts)), nil +} + type gardenFakeModel struct{ calls int } func (m *gardenFakeModel) CompleteWithUsage(context.Context, string, []llm.Message, int) (string, llm.Usage, error) { @@ -196,21 +253,23 @@ func TestGardenAgentFilesProposalsAndRelationsOnceAndOnlyStaged(t *testing.T) { if fr.calls != 1 { t.Fatalf("sessions = %d, want one for the two entries", fr.calls) } - if len(st.written) != 2 { - t.Fatalf("wrote %d, want the stale proposal and the contradiction: %s", len(st.written), out.String()) + // The contradiction is a thread now, not a note. + if len(st.written) != 1 || len(st.threads) != 1 || len(st.posts) != 1 { + t.Fatalf("wrote %d notes, %d threads, %d posts, want the stale proposal and one thread with its first post: %s", + len(st.written), len(st.threads), len(st.posts), out.String()) } for _, k := range st.written { if k.Harness != "gardener" || k.Confidence != "staged" || k.Status != "active" || k.Origin == "human" { t.Errorf("proposal must be staged and gardener-marked: %+v", k) } } - if !strings.Contains(out.String(), "1 of them about duplicates or contradictions") { + if !strings.Contains(out.String(), "1 contradictions opened as threads") { t.Errorf("summary line: %s", out.String()) } - before := len(st.written) + before, threads := len(st.written), len(st.threads) runGardenAgent(st, repo, gardenProject, opts, gardenPrice, "", &out) - if len(st.written) != before { - t.Errorf("second run wrote %d more", len(st.written)-before) + if len(st.written) != before || len(st.threads) != threads { + t.Errorf("second run wrote %d notes and opened %d threads more", len(st.written)-before, len(st.threads)-threads) } for _, k := range st.entries { if k.Confidence != "trusted" || k.Status != "active" { diff --git a/cmd/ctx/serve.go b/cmd/ctx/serve.go index d1be291..09419b4 100644 --- a/cmd/ctx/serve.go +++ b/cmd/ctx/serve.go @@ -303,6 +303,11 @@ func runServer(ctx context.Context, st *store.Store, cfg serveConfig, stdout, st } } slog.New(slog.NewJSONHandler(stderr, nil)).Info("writer_config", "max_operations", cfg.Writer.MaxOperations, "max_bytes", cfg.Writer.MaxBytes, "max_batch", cfg.Writer.MaxBatch, "read_connections", cfg.Writer.ReadConnections) + if a, ok := st.GardenerAccount(); ok { + slog.New(slog.NewJSONHandler(stderr, nil)).Info("gardener_account", "set", true, "account", a.ID, "name", a.Name) + } else { + slog.New(slog.NewJSONHandler(stderr, nil)).Info("gardener_account", "set", false, "note", "nobody may write gardener entries; set the account with: ctx account set-gardener --db ") + } // The session search index of chunks written before it existed is caught up // in the background, in small resumable steps; serving does not wait for it. indexCtx, stopIndex := context.WithCancel(ctx) diff --git a/deploy/README.md b/deploy/README.md index 6076724..8057a83 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -451,6 +451,73 @@ allowlist, so a gardener that must not read other projects' entries runs with config), is not allowed those two tools and is not told about them. The fixed git tools `git_log` and `git_show` stay either way. +### What the gardener writes, and what only people do + +The gardener proposes, a person decides. With `--propose` it makes exactly these +calls to the server besides reading, and the account of its client config +(`garden/claude/.config/ghosttree/config.json`) needs nothing more than the role +**member** in the project, not lead and not reviewer: + +- **Staged notes** with harness `gardener`, never trusted, never replacing + anything: stale and correction verdicts, duplicates between entries, and + answers to the regression question for pitfalls nobody has judged (a test that + exists, a sketch of a missing test, or "not testable"). The answers are checked + against the checkout first: a named test has to be defined in the file the + session read. +- **One thread per contradiction**, linked to every entry it is about, titled as a + question, opened by the gardener's account with the reason and the verbatim + quotes as its first post. For the same entries there is never a second open + thread, and a thread a person closed stays closed while the entries are + unchanged (the thread's question records which wording it was about); once an + entry changes, a new thread may ask again, and the old one is left as it is. + To read and open threads the gardener registers as the agent `gardener:` + in the project's room. If a thread cannot be opened the finding becomes a staged + note. + +Everything that changes an entry is a button on the review page (`/ui/review`) +that a person presses, and the server asks that person for the right the same +step needs elsewhere: **Merge** on a duplicate note supersedes the older entry by +the newer one (right to change every entry involved; the reason is "Gärtner-Vorschlag" +with the number of the note and the name of the person), **Record as covered** and +**Record as not testable** on a regression note set the regression verdict of the +pitfall (right to verify). The gardener has no call for any of this, and its own +account, a plain member, is refused by the server if it tried. + +**Set the gardener's account on the server.** Only one account may write entries +with harness `gardener` or a session reference starting with `gardener:`, and the +review page acts only on notes it wrote: the account whose token is in the +gardener's client config. Say which one on the server host, once: + +```bash +sudo systemd-run --pipe --wait --collect --quiet \ + -p DynamicUser=yes -p User=ghosttree -p StateDirectory=ghosttree \ + /usr/local/bin/ctx account set-gardener --db /var/lib/ghosttree/ghosttree.db +``` + +(The identity is the one of "Running maintenance commands" above; run it as the +user that owns the database if yours differs.) The command looks the account up +once and stores its **number** in the database (`org_state`, no schema change); the +running server uses it at once, without a restart. `ctx account list` marks it with +`gardener=true`, `ctx account set-gardener --clear` takes the role away, and +`ctx serve` logs `gardener_account` at start with the account and whether one is +set. The number, not the name, is what counts: renaming the account changes +nothing, and an account that takes its old name gets nothing. Nothing that is +reachable over HTTP sets it. Until one is set nobody may write gardener entries: +the API answers 403 `gardener_reserved`, which is also what the gardener's +proposals get on a server that has not been set up, and the review page offers no +action on a gardener note. + +The review page acts on a note only if that account wrote it (the account number +stored with the note, not a label), nobody has edited it since, and the entries its +title names are the ones its session reference names; the card shows the entries of +the reference, with title and link. The test a regression note names is checked for +its shape (a relative path, a colon, a function name), shown whole on the card and +recorded as the note holds it. The server has no checkout, so it cannot look the +test up; `ctx` did that before it filed the note, and the account and the unedited +note are what vouch for it. The server does not force `staged` on the gardener's +own account; the command does, and a test records every call a gardener run makes +against a real server. + Layout under `/var/lib/ghosttree/garden`: `repo/` (checkout, owned by root, updated by `ghosttree-garden-update.service`), `bin/` (the `claude` binary, pass `--claude`), `run.sh`, `state/`, `reports/`, `claude/` (writable by diff --git a/internal/garden/agent.go b/internal/garden/agent.go index 73855fa..2f8bfe1 100644 --- a/internal/garden/agent.go +++ b/internal/garden/agent.go @@ -248,6 +248,13 @@ func PackagePrompt(p Package, project string, maxTurns int) string { var b strings.Builder fmt.Fprintf(&b, "Projekt: %s\nPaket: %s (%d Einträge)\n\n", project, p.Name, len(p.Entries)) fmt.Fprintf(&b, "Du hast höchstens %d Turns. Beginne spätestens ab Turn %d mit der Antwort; ein Lauf ohne JSON am Ende ist verloren. Viele Einträge lassen sich mit einer Suche entscheiden, gehe nicht jede Datei einzeln durch.\n\n", maxTurns, maxTurns-8) + asked := map[int64]bool{} + for _, e := range coverAsked(p) { + asked[e.ID] = true + } + if len(asked) > 0 { + b.WriteString(regressionQuestion) + } for _, e := range p.Entries { body := strings.TrimSpace(e.Body) if len(body) > maxBodyLen { @@ -257,12 +264,28 @@ func PackagePrompt(p Package, project string, maxTurns int) string { } body = cut + "\n[gekürzt]" } - fmt.Fprintf(&b, "=== EINTRAG #%d (%s), beobachtet %s ===\nTitel: %s\n\n%s\n\n", e.ID, e.Type, e.Since(), e.Title, body) + tag := "" + if asked[e.ID] { + tag = " [Regressionsfrage]" + } + fmt.Fprintf(&b, "=== EINTRAG #%d (%s)%s, beobachtet %s ===\nTitel: %s\n\n%s\n\n", e.ID, e.Type, tag, e.Since(), e.Title, body) } b.WriteString("Prüfe diese Einträge jetzt und antworte mit dem JSON-Objekt.") return b.String() } +// regressionQuestion is the second question of a package that has open pitfalls +// (job 5). It sits in the task and not in the standing instruction, so a package +// without such a pitfall does not pay for it, and the schema of the answer only +// grows by an optional field. +const regressionQuestion = `Regressionsfrage: Bei den Einträgen mit [Regressionsfrage] hat noch niemand beurteilt, welcher Test die Wiederkehr des Fehlers bemerken würde. Beantworte sie zusätzlich im Feld "regression", ein Objekt je Eintrag, oder lass den Eintrag aus, wenn du keine klare Antwort hast: +{"id":123,"kind":"existing","file":"pfad/x_test.go","test":"TestName","reason":"warum er den Fehler fängt"} (ein Test aus dem Checkout, dessen Datei du gelesen oder durchsucht hast) +{"id":123,"kind":"sketch","file":"pfad/x_test.go","test":"TestName","steps":"3 bis 6 Zeilen: was der Test aufsetzt und prüft"} (es gibt keinen, und es ließe sich einer schreiben) +{"id":123,"kind":"untestable","reason":"Grund"} (nichts zu testen, etwa eine Eigenschaft eines Werkzeugs) +Eine Suche je Pitfall genügt. Der Testname ist der Name der Funktion (Go: func TestName); ctx prüft, dass sie in der Datei steht. + +` + // The two spots of agentSystemTemplate that depend on whether the session has // the ghosttree tools. Without them the prompt must not promise a tool the // session cannot call: it would try, be refused, and spend turns on it. @@ -307,6 +330,8 @@ type agentRelation struct { type agentAnswer struct { Verdicts []agentVerdict `json:"verdicts"` Relations []agentRelation `json:"relations"` + // Regression answers the regression question; absent when none was asked. + Regression []agentCover `json:"regression"` } // extractAnswer finds the JSON object a session ended with. The model may wrap @@ -379,6 +404,7 @@ func (r *Repo) lineExists(file, num string) bool { type Validated struct { Verdicts []Verdict Relations []Relation + Covers []Cover Rejected []string } @@ -462,6 +488,29 @@ func validateAnswer(repo *Repo, text string, pkg Package, known map[int64]Entry, } out.Relations = append(out.Relations, rel) } + asked := map[int64]Entry{} + for _, e := range coverAsked(pkg) { + asked[e.ID] = e + } + covered := map[int64]bool{} + for _, a := range ans.Regression { + e, ok := asked[a.ID] + switch { + case !ok: + reject("Absicherung #%d: nicht erfragt", a.ID) + continue + case covered[a.ID]: + reject("Absicherung #%d: doppelt", a.ID) + continue + } + c, why := validateCover(repo, a, e, looked) + if why != "" { + reject("Absicherung #%d (%s): %s", a.ID, a.Kind, why) + continue + } + covered[a.ID] = true + out.Covers = append(out.Covers, c) + } return out, nil } @@ -709,6 +758,7 @@ func CheckAgent(ctx context.Context, repo *Repo, entries []Entry, opt AgentOptio } res.Verdicts = append(res.Verdicts, val.Verdicts...) res.Relations = append(res.Relations, val.Relations...) + res.Covers = append(res.Covers, val.Covers...) } return res } diff --git a/internal/garden/check.go b/internal/garden/check.go index 9504ed5..1e8c391 100644 --- a/internal/garden/check.go +++ b/internal/garden/check.go @@ -112,7 +112,9 @@ type Result struct { Sessions []SessionStat Packages []Package Relations []Relation - Deferred int + // Covers are the answers to the regression question (job 5). + Covers []Cover + Deferred int } const systemPrompt = `Du prüfst einen Wissens-Eintrag (Pitfall oder Decision) eines Softwareprojekts gegen den aktuellen Code. Du entscheidest nichts, du machst einen Vorschlag, den ein Mensch prüft. diff --git a/internal/garden/consolidate_test.go b/internal/garden/consolidate_test.go new file mode 100644 index 0000000..9635ecd --- /dev/null +++ b/internal/garden/consolidate_test.go @@ -0,0 +1,567 @@ +package garden + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "reflect" + "sort" + "strings" + "testing" + "time" + + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +// coverRepo is a checkout with a Go test, a Go file that is no test, and a test +// of another language. +func coverRepo(t *testing.T) *Repo { + t.Helper() + dir := t.TempDir() + run := func(args ...string) { + cmd := exec.Command("git", append([]string{"-C", dir, "-c", "user.name=t", "-c", "user.email=t@example.invalid"}, args...)...) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + run("init", "-q") + for name, body := range map[string]string{ + "internal/store/x_test.go": "package store\n\nimport \"testing\"\n\nfunc TestHoldsX(t *testing.T) {}\n\nfunc helperOnly() {}\n", + "internal/store/x.go": "package store\n\nfunc TestNotATest() {}\n", + "web/app.test.js": "test('x', () => {})\nfunction checksLogin() {}\n", + } { + if err := os.MkdirAll(filepath.Dir(filepath.Join(dir, name)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + run("add", ".") + run("commit", "-q", "-m", "init") + repo, err := OpenRepo(dir) + if err != nil { + t.Fatal(err) + } + return repo +} + +func coverEntry(id int64) Entry { + return Entry{ID: id, Type: "pitfall", Title: fmt.Sprintf("pitfall %d", id), Body: "b", AskCover: true} +} + +func covers(t *testing.T, repo *Repo, answer string, looked *Looked, entries ...Entry) Validated { + t.Helper() + text := `{"verdicts":[],"regression":[` + answer + `]}` + v, err := validateAnswer(repo, text, Package{Name: "p", Entries: entries}, nil, looked) + if err != nil { + t.Fatal(err) + } + return v +} + +func TestRegressionAnswerNamesATestThatExistsOrIsDropped(t *testing.T) { + repo := coverRepo(t) + seen := &Looked{Read: map[string]bool{"/work/internal/store/x_test.go": true}} + cases := []struct { + name, answer string + ok bool + }{ + {"defined test", `{"id":1,"kind":"existing","file":"internal/store/x_test.go","test":"TestHoldsX","reason":"prüft es"}`, true}, + {"leading ./", `{"id":1,"kind":"existing","file":"./internal/store/x_test.go","test":"TestHoldsX"}`, true}, + {"not in the file", `{"id":1,"kind":"existing","file":"internal/store/x_test.go","test":"TestInvented"}`, false}, + {"a helper is no test of that name", `{"id":1,"kind":"existing","file":"internal/store/x_test.go","test":"TestHold"}`, false}, + {"file does not exist", `{"id":1,"kind":"existing","file":"internal/store/nope_test.go","test":"TestHoldsX"}`, false}, + {"go file that is no test file", `{"id":1,"kind":"existing","file":"internal/store/x.go","test":"TestNotATest"}`, false}, + {"file the session never looked at", `{"id":1,"kind":"existing","file":"web/app.test.js","test":"checksLogin"}`, false}, + {"outside the checkout", `{"id":1,"kind":"existing","file":"../x_test.go","test":"TestHoldsX"}`, false}, + {"absolute path", `{"id":1,"kind":"existing","file":"/etc/passwd","test":"TestHoldsX"}`, false}, + {"name that is no identifier", `{"id":1,"kind":"existing","file":"internal/store/x_test.go","test":"Test Holds X"}`, false}, + {"missing name", `{"id":1,"kind":"existing","file":"internal/store/x_test.go"}`, false}, + } + for _, c := range cases { + v := covers(t, repo, c.answer, seen, coverEntry(1)) + if got := len(v.Covers) == 1; got != c.ok { + t.Errorf("%s: accepted=%v, want %v (rejected: %v)", c.name, got, c.ok, v.Rejected) + } + if !c.ok && len(v.Rejected) != 1 { + t.Errorf("%s: a dropped answer must be named in the report: %v", c.name, v.Rejected) + } + } + // Another language: the name as a word in a file the session read. + other := &Looked{Read: map[string]bool{"/work/web/app.test.js": true}} + if v := covers(t, repo, `{"id":1,"kind":"existing","file":"web/app.test.js","test":"checksLogin"}`, other, coverEntry(1)); len(v.Covers) != 1 { + t.Errorf("a test of another language: %v", v.Rejected) + } + v := covers(t, repo, `{"id":1,"kind":"existing","file":"internal/store/x_test.go","test":"TestHoldsX","reason":"r"}`, seen, coverEntry(1)) + if c := v.Covers[0]; c.TestID() != "internal/store/x_test.go:TestHoldsX" || c.Kind != CoverExisting || c.Entry.ID != 1 { + t.Errorf("cover = %+v", c) + } +} + +func TestRegressionSketchNeedsAPlaceAStepListAndAFreeName(t *testing.T) { + repo := coverRepo(t) + steps := "lege einen Eintrag an\nrufe die Funktion auf\nprüfe das Ergebnis" + ok := func(file, name, steps string) string { + return fmt.Sprintf(`{"id":1,"kind":"sketch","file":%q,"test":%q,"steps":%q}`, file, name, steps) + } + for _, c := range []struct { + name, answer string + ok bool + }{ + {"a sketch", ok("internal/store/y_test.go", "TestNewY", steps), true}, + {"in a file that exists, under a new name", ok("internal/store/x_test.go", "TestNewY", steps), true}, + {"the test exists already: say so as an existing test", ok("internal/store/x_test.go", "TestHoldsX", steps), false}, + {"a directory the checkout does not have", ok("nowhere/y_test.go", "TestNewY", steps), false}, + {"no steps", ok("internal/store/y_test.go", "TestNewY", ""), false}, + {"one line", ok("internal/store/y_test.go", "TestNewY", "alles prüfen"), false}, + {"far too long", ok("internal/store/y_test.go", "TestNewY", strings.Repeat("schritt\n", 12)), false}, + {"inside .git", ok(".git/hooks/y_test.go", "TestNewY", steps), false}, + {"no name", ok("internal/store/y_test.go", "", steps), false}, + } { + v := covers(t, repo, c.answer, nil, coverEntry(1)) + if got := len(v.Covers) == 1; got != c.ok { + t.Errorf("%s: accepted=%v, want %v (%v)", c.name, got, c.ok, v.Rejected) + } + } +} + +func TestRegressionUntestableNeedsAReason(t *testing.T) { + repo := coverRepo(t) + if v := covers(t, repo, `{"id":1,"kind":"untestable","reason":"Eine Eigenschaft des Werkzeugs, nichts im Code."}`, nil, coverEntry(1)); len(v.Covers) != 1 { + t.Errorf("untestable with a reason: %v", v.Rejected) + } + for _, a := range []string{`{"id":1,"kind":"untestable"}`, `{"id":1,"kind":"untestable","reason":"nein"}`, `{"id":1,"kind":"whatever","reason":"ein langer Grund hier"}`} { + if v := covers(t, repo, a, nil, coverEntry(1)); len(v.Covers) != 0 || len(v.Rejected) != 1 { + t.Errorf("%s accepted: %+v", a, v) + } + } +} + +// Only a question that was asked is answered: the entry of the package that did not +// ask, another entry, a doubled answer and an eleventh pitfall are dropped. +func TestRegressionAnswersAreTakenOnlyForQuestionsThatWereAsked(t *testing.T) { + repo := coverRepo(t) + one := func(id int64) string { + return fmt.Sprintf(`{"id":%d,"kind":"untestable","reason":"nichts zu testen, ein Werkzeug"}`, id) + } + var es []Entry + for i := int64(1); i <= 10; i++ { + es = append(es, coverEntry(i)) + } + es[1].AskCover = false // judged already + es = append(es, Entry{ID: 20, Type: "decision", Title: "d", AskCover: true}) // a decision has no regression question + var answers []string + for _, id := range []int64{1, 2, 20, 99, 1, 10} { + answers = append(answers, one(id)) + } + v := covers(t, repo, strings.Join(answers, ","), nil, es...) + got := map[int64]bool{} + for _, c := range v.Covers { + got[c.Entry.ID] = true + } + // 1 is asked and answered once (its double is dropped); 10 is the ninth pitfall + // that asks, past the limit of eight a session; 2, 20 and 99 were not asked. + if len(got) != 1 || !got[1] || len(v.Rejected) != 5 { + t.Errorf("covers %v, rejected %v", got, v.Rejected) + } +} + +func TestPromptAsksTheRegressionQuestionOnlyWhereItIsAsked(t *testing.T) { + plain := Package{Name: "p", Entries: []Entry{{ID: 1, Type: "pitfall", Title: "a", Body: "b"}, {ID: 2, Type: "decision", Title: "c", Body: "d"}}} + if p := PackagePrompt(plain, "o/r", 30); strings.Contains(p, "Regressionsfrage") || strings.Contains(p, "regression") { + t.Errorf("a package that asks nothing is charged for the question:\n%s", p) + } + var es []Entry + for i := int64(1); i <= 12; i++ { + es = append(es, coverEntry(i)) + } + asked := Package{Name: "p", Entries: es} + p := PackagePrompt(asked, "o/r", 30) + if n := strings.Count(p, "[Regressionsfrage], beobachtet"); n != maxCoverAsks { + t.Errorf("%d entries tagged, want %d", n, maxCoverAsks) + } + if !strings.Contains(p, `"regression"`) || strings.Count(p, "Regressionsfrage: ") != 1 { + t.Errorf("the question is not put exactly once:\n%s", p) + } + // The standing instruction and the answer of a package that never asks are the + // old ones: the schema grew by an optional field only. + if strings.Contains(AgentSystemPrompt(false), "regression") { + t.Error("the system prompt carries the regression question for every package") + } + if _, err := extractAnswer(`{"verdicts":[{"id":1,"verdict":"holds"}]}`); err != nil { + t.Errorf("an old-style answer is not read: %v", err) + } +} + +func TestPromptGrowthOfTheRegressionQuestionIsSmall(t *testing.T) { + var base, asked []Entry + for i := int64(1); i <= 30; i++ { + e := Entry{ID: i, Type: "pitfall", Title: "Titel eines Pitfalls mit einiger Länge", Body: strings.Repeat("Ein Satz über den Code, der etwas Konkretes behauptet. ", 40), ObservedAt: "2026-10-01T00:00:00Z"} + base = append(base, e) + e.AskCover = true + asked = append(asked, e) + } + plain := len(PackagePrompt(Package{Name: "p", Entries: base}, "o/r", 60)) + len(AgentSystemPrompt(false)) + with := len(PackagePrompt(Package{Name: "p", Entries: asked}, "o/r", 60)) + len(AgentSystemPrompt(false)) + t.Logf("package of 30 pitfalls with 2000 characters each: %d -> %d characters (+%d, %.1f%%)", plain, with, with-plain, 100*float64(with-plain)/float64(plain)) + if growth := float64(with-plain) / float64(plain); growth > 0.03 { + t.Errorf("the question makes the prompt %.1f%% longer", growth*100) + } +} + +func TestCoverProposalIsStagedGardenerMarkedAndAsksOnce(t *testing.T) { + c := Cover{Entry: Entry{ID: 12, Type: "pitfall", Title: "Pitfall"}, Kind: CoverSketch, File: "a/b_test.go", Test: "TestB", Steps: "eins\nzwei\ndrei", Reason: "weil"} + k := CoverProposal("github.com/o/r", c) + if k.Harness != Harness || k.Confidence != "staged" || k.Status != "active" || k.Type != "note" || k.Origin == "human" || k.RegressionState != "" { + t.Errorf("proposal = %+v", k) + } + if k.SessionRef != "gardener:regression:12:sketch" || !strings.HasPrefix(k.Title, "[Gärtner] Test fehlt? #12: ") { + t.Errorf("ref %q title %q", k.SessionRef, k.Title) + } + for _, want := range []string{"Herkunft gardener, nicht bestätigt", "Test: a/b_test.go:TestB", "Schritte:", " zwei", "Begründung: weil"} { + if !strings.Contains(k.Body, want) { + t.Errorf("body lacks %q:\n%s", want, k.Body) + } + } + sink := &recordingSink{} + if n, err := ProposeCovers(sink, "github.com/o/r", nil, []Cover{c}); err != nil || n != 1 { + t.Fatalf("n=%d err=%v", n, err) + } + existing := []store.Knowledge{sink.written[0]} + existing[0].CreatedAt = "2026-10-01T00:00:00Z" + // The same pitfall, unchanged: not again, in whatever state the note is in. + if n, _ := ProposeCovers(sink, "github.com/o/r", existing, []Cover{c}); n != 0 { + t.Errorf("proposed again: %d", n) + } + existing[0].Status = "deprecated" + c.Entry.UpdatedAt = "2026-09-01T00:00:00Z" + if !CoverProposed(existing, c.Entry) { + t.Error("a rejected proposal is forgotten") + } + // Changed after the proposal: the question may be asked again. + c.Entry.UpdatedAt = "2026-10-02T00:00:00Z" + if CoverProposed(existing, c.Entry) { + t.Error("a changed pitfall stays unasked") + } + if n, _ := ProposeCovers(sink, "github.com/o/r", existing, []Cover{c}); n != 1 { + t.Errorf("a changed pitfall was not proposed again: %d", n) + } +} + +func TestReportGivesTestSketchesASectionOfTheirOwn(t *testing.T) { + var b strings.Builder + WriteReport(&b, "o/r", Result{Agent: true, Covers: []Cover{ + {Entry: Entry{ID: 12, Title: "Pitfall A"}, Kind: CoverSketch, File: "a/b_test.go", Test: "TestB", Steps: "lege an\nrufe auf\nprüfe", Reason: "weil"}, + {Entry: Entry{ID: 13, Title: "Pitfall B"}, Kind: CoverExisting, File: "c/d_test.go", Test: "TestD"}, + {Entry: Entry{ID: 14, Title: "Pitfall C"}, Kind: CoverUntestable, Reason: "ein Werkzeug"}, + }}, mustTime("2026-10-19T03:00:00Z")) + out := b.String() + sketch := out[strings.Index(out, "## Testskizzen"):] + sketch = sketch[:strings.Index(sketch, "\n\n")] + for _, want := range []string{"#12 Pitfall A", "a/b_test.go:TestB", "rufe auf"} { + if !strings.Contains(sketch, want) { + t.Errorf("the sketch section lacks %q:\n%s", want, sketch) + } + } + if strings.Contains(sketch, "Pitfall B") || strings.Contains(sketch, "Pitfall C") { + t.Errorf("the sketch section holds more than sketches:\n%s", sketch) + } + rest := out[strings.Index(out, "## Regressionsschutz"):] + if !strings.Contains(rest, "c/d_test.go:TestD deckt ihn ab") || !strings.Contains(rest, "nicht testbar (ein Werkzeug)") { + t.Errorf("the other answers are missing:\n%s", rest) + } +} + +// ---------------------------------------------------------------- threads + +// fakeThreads is a thread store with the behaviour the gardener depends on: a +// thread is found through the object it is linked to, and a person can close it. +type fakeThreads struct { + threads []store.Thread + links map[int64][]store.ThreadLink + posts []store.CoordMessage + agents []store.CoordAgent + calls []string + failFor string +} + +func (f *fakeThreads) log(s string) { f.calls = append(f.calls, s) } + +func (f *fakeThreads) RegisterCoordAgent(a store.CoordAgent) (int64, error) { + f.log("register") + f.agents = append(f.agents, a) + return 1, nil +} + +func (f *fakeThreads) ThreadsForObject(kind, id, _ string) ([]store.Thread, error) { + f.log("find") + if f.failFor == "find" { + return nil, errors.New("server down") + } + var out []store.Thread + for _, t := range f.threads { + for _, l := range f.links[t.ID] { + if l.Kind == kind && l.ID == id { + out = append(out, t) + break + } + } + } + return out, nil +} + +func (f *fakeThreads) ThreadLinks(id int64, _ string) ([]store.ThreadLink, error) { + f.log("links") + return f.links[id], nil +} + +func (f *fakeThreads) CreateThread(t store.Thread, _ string) (int64, error) { + f.log("create") + if f.failFor == "create" { + return 0, errors.New("refused") + } + t.ID = int64(len(f.threads) + 1) + t.State = store.ThreadOpen + if f.links == nil { + f.links = map[int64][]store.ThreadLink{} + } + if t.Link != nil { + l := *t.Link + l.ThreadID = t.ID + f.links[t.ID] = append(f.links[t.ID], l) + } + f.threads = append(f.threads, t) + return t.ID, nil +} + +func (f *fakeThreads) LinkThread(l store.ThreadLink, _ string) error { + f.log("link") + f.links[l.ThreadID] = append(f.links[l.ThreadID], l) + return nil +} + +func (f *fakeThreads) SendCoordMessage(m store.CoordMessage) (int64, error) { + f.log("post") + f.posts = append(f.posts, m) + return int64(len(f.posts)), nil +} + +func (f *fakeThreads) setState(id int64, state string, archived bool) { + for i := range f.threads { + if f.threads[i].ID == id { + f.threads[i].State, f.threads[i].Archived = state, archived + } + } +} + +func contradiction() (Relation, map[int64]Entry) { + known := map[int64]Entry{ + 12: {ID: 12, Type: "decision", Title: "Dienst läuft auf server-a", Body: "Der Dienst läuft auf server-a und nirgends sonst."}, + 15: {ID: 15, Type: "pitfall", Title: "Dienst läuft auf server-b", Body: "Der Dienst läuft auf server-b."}, + } + return Relation{Kind: Contradiction, IDs: []int64{12, 15}, Reason: "Zwei Orte.", + Quotes: map[int64]string{12: "läuft auf server-a und nirgends sonst", 15: "läuft auf server-b"}}, known +} + +const testProject = "github.com/o/r" + +func TestContradictionOpensOneThreadLinkedToBothWithTheQuotes(t *testing.T) { + rel, known := contradiction() + f := &fakeThreads{} + out := OpenContradictionThreads(f, testProject, nil, []Relation{rel}, known) + if len(out) != 1 || out[0].State != ThreadOpened || out[0].ThreadID != 1 || out[0].Err != nil || out[0].Warn != "" { + t.Fatalf("outcome = %+v", out) + } + th := f.threads[0] + if th.Project != testProject || !strings.HasPrefix(th.Title, "[Gärtner] Widersprechen sich #12 „Dienst läuft auf server-a“ und #15") || !strings.HasSuffix(th.Title, "?") { + t.Errorf("title = %q", th.Title) + } + if th.Link == nil || th.Link.Kind != "knowledge" || th.Link.ID != "12" { + t.Errorf("link on creation = %+v", th.Link) + } + got := map[string]bool{} + for _, l := range f.links[1] { + got[l.Kind+":"+l.ID] = true + } + if !got["knowledge:12"] || !got["knowledge:15"] { + t.Errorf("linked to %v, want both entries", got) + } + if len(f.posts) != 1 { + t.Fatalf("%d posts, want the first post", len(f.posts)) + } + p := f.posts[0] + if p.DestinationKind != store.DestinationDiscussion || p.DestinationID != store.ThreadDestinationID(1) || p.SenderExternalID != ThreadAgentID(testProject) { + t.Errorf("post = %+v", p) + } + for _, want := range []string{"Herkunft gardener, nicht bestätigt", `Zitat: "läuft auf server-a und nirgends sonst"`, `Zitat: "läuft auf server-b"`, "Begründung: Zwei Orte."} { + if !strings.Contains(p.Body, want) { + t.Errorf("first post lacks %q:\n%s", want, p.Body) + } + } + if len(f.agents) != 1 || f.agents[0].Provider != Harness || f.agents[0].RoomKey != store.RoomKeyForProject(testProject) || f.agents[0].ExternalID != ThreadAgentID(testProject) { + t.Errorf("registered as %+v", f.agents) + } + for _, c := range f.calls { + if c != "register" && c != "find" && c != "links" && c != "create" && c != "link" && c != "post" { + t.Errorf("unexpected call %q", c) + } + } +} + +func TestContradictionThreadIsIdempotentAndNeverReopened(t *testing.T) { + rel, known := contradiction() + f := &fakeThreads{} + open := func() ThreadOutcome { return OpenContradictionThreads(f, testProject, nil, []Relation{rel}, known)[0] } + open() + + // Open: no second one. + if o := open(); o.State != ThreadExists || o.ThreadID != 1 || len(f.threads) != 1 { + t.Fatalf("second run: %+v, %d threads", o, len(f.threads)) + } + // Parked or archived is not answered either, and the same entries do not ask again. + for name, set := range map[string]func(){ + "resolved": func() { f.setState(1, store.ThreadResolved, false) }, + "deferred": func() { f.setState(1, store.ThreadDeferred, false) }, + "archived": func() { f.setState(1, store.ThreadOpen, true) }, + } { + set() + callsBefore := len(f.calls) + if o := open(); len(f.threads) != 1 || (o.State != ThreadDecided && o.State != ThreadExists) { + t.Errorf("%s: %+v, %d threads", name, o, len(f.threads)) + } + for _, c := range f.calls[callsBefore:] { + if c == "create" || c == "post" { + t.Errorf("%s: %s after a person's decision", name, c) + } + } + } + f.setState(1, store.ThreadResolved, false) + // The gardener has no call that reopens or closes: nothing but these ever ran. + for _, c := range f.calls { + if strings.Contains(c, "state") || strings.Contains(c, "archive") { + t.Errorf("a thread was touched: %s", c) + } + } + // One of the entries changed: the old thread stays closed as it is, a new one asks again. + changed := map[int64]Entry{12: known[12], 15: known[15]} + e := changed[15] + e.Body += " Seit dem Umzug." + changed[15] = e + if o := OpenContradictionThreads(f, testProject, nil, []Relation{rel}, changed)[0]; o.State != ThreadOpened || o.ThreadID != 2 { + t.Fatalf("a changed entry: %+v", o) + } + if f.threads[0].State != store.ThreadResolved { + t.Error("the closed thread was reopened") + } + // And that one is open now, so again no second one. + if o := OpenContradictionThreads(f, testProject, nil, []Relation{rel}, changed)[0]; o.State != ThreadExists || o.ThreadID != 2 || len(f.threads) != 2 { + t.Errorf("after the new thread: %+v", o) + } +} + +func TestContradictionIsLeftToPeopleWhereTheyAlreadyActed(t *testing.T) { + rel, known := contradiction() + // A thread people opened on both entries counts in every state, with or without our stand line. + f := &fakeThreads{ + threads: []store.Thread{{ID: 1, Project: testProject, Title: "Welcher Ort gilt?", State: store.ThreadResolved}}, + links: map[int64][]store.ThreadLink{1: {{ThreadID: 1, Kind: "knowledge", ID: "12"}, {ThreadID: 1, Kind: "knowledge", ID: "15"}}}, + } + if o := OpenContradictionThreads(f, testProject, nil, []Relation{rel}, known)[0]; o.State != ThreadDecided || len(f.threads) != 1 { + t.Errorf("a closed thread of people: %+v", o) + } + // A thread on only one of the entries is another question. + f = &fakeThreads{ + threads: []store.Thread{{ID: 1, Project: testProject, Title: "Nur zu zwölf", State: store.ThreadOpen}}, + links: map[int64][]store.ThreadLink{1: {{ThreadID: 1, Kind: "knowledge", ID: "12"}}}, + } + if o := OpenContradictionThreads(f, testProject, nil, []Relation{rel}, known)[0]; o.State != ThreadOpened { + t.Errorf("a thread on one entry blocked the pair: %+v", o) + } + // A note of the gardener on the same relation, in any state, means somebody has seen it. + note := RelationProposal(testProject, rel, map[int64]string{12: "a", 15: "b"}) + note.Status = "deprecated" + f = &fakeThreads{} + if o := OpenContradictionThreads(f, testProject, []store.Knowledge{note}, []Relation{rel}, known)[0]; o.State != ThreadDecided || len(f.calls) != 0 { + t.Errorf("after a rejected note: %+v, calls %v", o, f.calls) + } +} + +func TestContradictionThreadIsNotOpenedWhenThreadsCannotBeLookedUp(t *testing.T) { + rel, known := contradiction() + f := &fakeThreads{failFor: "find"} + o := OpenContradictionThreads(f, testProject, nil, []Relation{rel}, known)[0] + if o.State != ThreadFailed || o.Err == nil || len(f.threads) != 0 { + t.Errorf("without a look-up: %+v, %d threads", o, len(f.threads)) + } + f = &fakeThreads{failFor: "create"} + if o := OpenContradictionThreads(f, testProject, nil, []Relation{rel}, known)[0]; o.State != ThreadFailed || o.Err == nil { + t.Errorf("refused: %+v", o) + } + // Duplicates are notes, not threads, and a relation reported twice is one thread. + dup := Relation{Kind: Duplicate, IDs: []int64{12, 15}, Reason: "r", Quotes: rel.Quotes} + f = &fakeThreads{} + if out := OpenContradictionThreads(f, testProject, nil, []Relation{dup, rel, rel}, known); len(out) != 1 || len(f.threads) != 1 { + t.Errorf("outcomes %+v, threads %d", out, len(f.threads)) + } +} + +// What the gardener may do on the server is a short list, and it holds no +// call that changes, closes, supersedes, retires, judges, or answers somebody +// else's thread. A new method on these interfaces has to be added here, which is +// the moment somebody asks whether it belongs there. +func TestWhatTheGardenerMayCallIsAShortListWithoutAnythingThatDecides(t *testing.T) { + names := func(i any) []string { + ty := reflect.TypeOf(i).Elem() + var out []string + for n := 0; n < ty.NumMethod(); n++ { + out = append(out, ty.Method(n).Name) + } + sort.Strings(out) + return out + } + if got, want := names((*Sink)(nil)), []string{"Remember"}; !reflect.DeepEqual(got, want) { + t.Errorf("Sink = %v, want %v", got, want) + } + want := []string{"CreateThread", "LinkThread", "RegisterCoordAgent", "SendCoordMessage", "ThreadLinks", "ThreadsForObject"} + if got := names((*ThreadSink)(nil)); !reflect.DeepEqual(got, want) { + t.Errorf("ThreadSink = %v, want %v", got, want) + } + for _, n := range append(names((*Sink)(nil)), names((*ThreadSink)(nil))...) { + for _, banned := range []string{"Patch", "Update", "Delete", "Supersede", "Retire", "Regression", "State", "Archive", "Criterion", "Complete", "Drop", "Verify", "Outcome"} { + if strings.Contains(n, banned) { + t.Errorf("%s can decide something (%s)", n, banned) + } + } + } +} + +func mustTime(s string) time.Time { + t, err := time.Parse(time.RFC3339, s) + if err != nil { + panic(err) + } + return t +} + +// The server reserves harness "gardener" and "gardener:" references to the +// gardener's account (store.ReservedForGardener). Every note the gardener files has +// to be one of those, or it would be refused, and the constants have to agree. +func TestEveryNoteTheGardenerFilesIsReservedToItsAccount(t *testing.T) { + if Harness != store.GardenerHarness { + t.Fatalf("the gardener writes harness %q, the server reserves %q", Harness, store.GardenerHarness) + } + rel := Relation{Kind: Duplicate, IDs: []int64{1, 2}, Reason: "r", Quotes: map[int64]string{1: "a", 2: "b"}} + stale, _ := Proposal(testProject, Verdict{Entry: Entry{ID: 1, Title: "t"}, Kind: Stale, Reason: "r"}) + for name, k := range map[string]store.Knowledge{ + "stale": stale, + "duplicate": RelationProposal(testProject, rel, map[int64]string{1: "a", 2: "b"}), + "regression": CoverProposal(testProject, Cover{Entry: Entry{ID: 1, Title: "t"}, Kind: CoverUntestable, Reason: "ein Grund hier"}), + } { + if !store.ReservedForGardener(k) || !strings.HasPrefix(k.SessionRef, store.GardenerRefPrefix) { + t.Errorf("%s: harness %q reference %q are not reserved to the gardener", name, k.Harness, k.SessionRef) + } + } +} diff --git a/internal/garden/context.go b/internal/garden/context.go index 25907d0..217e128 100644 --- a/internal/garden/context.go +++ b/internal/garden/context.go @@ -22,6 +22,11 @@ type Entry struct { ObservedAt string CreatedAt string UpdatedAt string + // Regression is the regression judgement recorded for a pitfall ("" = nobody + // has judged it). AskCover says the gardener should put the regression question + // for it in this run: an open pitfall with no proposal of the gardener standing. + Regression string + AskCover bool } func (e Entry) Since() string { diff --git a/internal/garden/cover.go b/internal/garden/cover.go new file mode 100644 index 0000000..5f5c841 --- /dev/null +++ b/internal/garden/cover.go @@ -0,0 +1,272 @@ +package garden + +import ( + "fmt" + "io" + "os" + "path" + "path/filepath" + "regexp" + "strings" + + "github.com/Deadweight-Labs/ghosttree/internal/scope" + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +// Kinds of answer to the regression question (job 5): a pitfall nobody has +// judged yet gets one of these, or none. +const ( + // CoverExisting names a test in the checkout that would catch the defect's return. + CoverExisting = "existing" + // CoverSketch says no such test exists and sketches one. + CoverSketch = "sketch" + // CoverUntestable says there is nothing to test here, and why. + CoverUntestable = "untestable" +) + +const ( + // maxCoverAsks is how many open pitfalls of one session get the question. The + // rest wait for the next run: every answer costs a lookup, and the nightly + // budget stays what it is. + maxCoverAsks = 8 + + coverRefPrefix = "gardener:regression:" + minUntestableReason = 12 + minSketchLines = 2 + maxSketchLines = 10 + maxSketchChars = 1500 + maxTestFile = 4 << 20 +) + +var testNameRe = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]{1,119}$`) + +// Cover is one validated answer to the regression question. It only exists once +// the checkout bears it out: a named test is defined in the file it is named in, +// and the session looked at that file. +type Cover struct { + Entry Entry + Kind string + // File and Test name the test: the one that exists, or where the sketched one + // belongs. Empty for an untestable pitfall. + File, Test string + // Steps is the sketch: what the test sets up and checks. + Steps string + Reason string + Session string +} + +// CoverLabel is the question a regression proposal asks, as the title and the card say it. +func CoverLabel(kind string) string { + switch kind { + case CoverExisting: + return "Test vorhanden?" + case CoverSketch: + return "Test fehlt?" + } + return "Nicht testbar?" +} + +// TestID is how the test is recorded as the cover of a pitfall: file and name, +// the form the regression tool uses. +func (c Cover) TestID() string { + if c.File == "" || c.Test == "" { + return "" + } + return c.File + ":" + c.Test +} + +// coverAsked is the pitfalls of a package the regression question is put to: the +// first maxCoverAsks of those that asked for it. The prompt and the validation +// both go by it, so an answer is only taken for a question that was asked. +func coverAsked(p Package) []Entry { + var out []Entry + for _, e := range p.Entries { + if e.AskCover && e.Type == "pitfall" && len(out) < maxCoverAsks { + out = append(out, e) + } + } + return out +} + +type agentCover struct { + ID int64 `json:"id"` + Kind string `json:"kind"` + File string `json:"file"` + Test string `json:"test"` + Steps string `json:"steps"` + Reason string `json:"reason"` +} + +// testDefined says whether the file, a tracked regular file of the checkout, +// defines the named test: for Go a `func Name(` in a _test.go file, for any other +// language the name as a word. +func (r *Repo) testDefined(file, name string) bool { + if !r.tracked(file) { + return false + } + p := filepath.Join(r.Root, file) + info, err := os.Lstat(p) + if err != nil || !info.Mode().IsRegular() { + return false + } + f, err := os.Open(p) + if err != nil { + return false + } + defer f.Close() + raw, err := io.ReadAll(io.LimitReader(f, maxTestFile)) + if err != nil { + return false + } + if strings.HasSuffix(file, ".go") { + if !strings.HasSuffix(file, "_test.go") { + return false + } + return regexp.MustCompile(`(?m)^func\s+` + regexp.QuoteMeta(name) + `\s*\(`).Match(raw) + } + return regexp.MustCompile(`\b` + regexp.QuoteMeta(name) + `\b`).Match(raw) +} + +// cleanTestFile is a relative path inside the checkout, or "". +func cleanTestFile(f string) string { + f = strings.TrimSpace(f) + if f == "" || strings.ContainsAny(f, ": \t\r\n\x00\\") || strings.HasPrefix(f, "/") { + return "" + } + f = path.Clean(strings.TrimPrefix(f, "./")) + if f == "." || f == ".." || strings.HasPrefix(f, "../") { + return "" + } + for _, seg := range strings.Split(f, "/") { + if seg == ".git" { + return "" + } + } + return f +} + +// stepsOf cleans a sketch to its non-empty lines. +func stepsOf(s string) []string { + var out []string + for _, l := range strings.Split(strings.ReplaceAll(s, "\r\n", "\n"), "\n") { + if l = strings.TrimSpace(l); l != "" { + out = append(out, l) + } + } + return out +} + +// validateCover holds one answer to the regression question to the checkout. It +// returns the cover, or why it was dropped. +func validateCover(repo *Repo, a agentCover, e Entry, looked *Looked) (Cover, string) { + c := Cover{Entry: e, Kind: a.Kind, Reason: strings.TrimSpace(a.Reason)} + switch a.Kind { + case CoverExisting: + file := cleanTestFile(a.File) + if file == "" || !testNameRe.MatchString(strings.TrimSpace(a.Test)) { + return Cover{}, "Datei oder Testname unbrauchbar" + } + name := strings.TrimSpace(a.Test) + if !repo.testDefined(file, name) { + return Cover{}, fmt.Sprintf("%s definiert keinen Test %s im Checkout", file, name) + } + if !looked.Has(file + ":1") { + return Cover{}, fmt.Sprintf("die Session hat %s nicht angesehen", file) + } + c.File, c.Test = file, name + case CoverSketch: + file := cleanTestFile(a.File) + name := strings.TrimSpace(a.Test) + if file == "" || !testNameRe.MatchString(name) { + return Cover{}, "Datei oder Testname der Skizze unbrauchbar" + } + if info, err := os.Stat(filepath.Join(repo.Root, filepath.Dir(file))); err != nil || !info.IsDir() { + return Cover{}, fmt.Sprintf("das Verzeichnis von %s gibt es im Checkout nicht", file) + } + if repo.testDefined(file, name) { + return Cover{}, fmt.Sprintf("%s gibt es schon in %s, als vorhandenen Test melden", name, file) + } + lines := stepsOf(a.Steps) + if n := len(lines); n < minSketchLines || n > maxSketchLines || len(strings.Join(lines, "\n")) > maxSketchChars { + return Cover{}, fmt.Sprintf("Skizze hat %d Zeilen (verlangt sind 3 bis 6, angenommen werden %d bis %d)", len(lines), minSketchLines, maxSketchLines) + } + c.File, c.Test, c.Steps = file, name, strings.Join(lines, "\n") + case CoverUntestable: + if len([]rune(c.Reason)) < minUntestableReason { + return Cover{}, "nicht testbar ohne Grund" + } + default: + return Cover{}, fmt.Sprintf("unbekannte Art %q", a.Kind) + } + return c, "" +} + +// CoverRef is the session reference of a regression proposal: the pitfall and +// what is proposed, so the review page knows the action without reading prose. +func CoverRef(id int64, kind string) string { return fmt.Sprintf("%s%d:%s", coverRefPrefix, id, kind) } + +// CoverProposed says whether the gardener has made a regression proposal for this +// pitfall that still stands for its current wording, in whatever state it is +// now. A person who said no is not asked again every night; only a change to the +// pitfall after the proposal reopens the question. +func CoverProposed(existing []store.Knowledge, e Entry) bool { + changed := parseTime(e.UpdatedAt) + prefix := fmt.Sprintf("%s%d:", coverRefPrefix, e.ID) + for _, k := range existing { + if k.Harness != Harness || !strings.HasPrefix(k.SessionRef, prefix) { + continue + } + if changed.IsZero() || !changed.After(parseTime(k.CreatedAt)) { + return true + } + } + return false +} + +// CoverProposal builds the staged note for a regression answer. Like every note +// of the gardener it changes nothing by itself: the pitfall stays as it is until +// a person confirms the note on the review page. +func CoverProposal(project string, c Cover) store.Knowledge { + var b strings.Builder + fmt.Fprintf(&b, "Vorschlag des Gärtners (Herkunft gardener, nicht bestätigt) zu Eintrag #%d: %s\n\n", c.Entry.ID, c.Entry.Title) + fmt.Fprintf(&b, "Urteil: %s\n", CoverLabel(c.Kind)) + switch c.Kind { + case CoverExisting: + fmt.Fprintf(&b, "\nTest: %s\n", c.TestID()) + case CoverSketch: + fmt.Fprintf(&b, "\nTest: %s (Skizze, noch nicht geschrieben)\n\nSchritte:\n", c.TestID()) + for _, l := range strings.Split(c.Steps, "\n") { + fmt.Fprintf(&b, " %s\n", l) + } + } + if c.Reason != "" { + fmt.Fprintf(&b, "\nBegründung: %s\n", c.Reason) + } + return store.Knowledge{ + Type: "note", + Title: fmt.Sprintf("%s%s #%d: %s", titlePrefix, CoverLabel(c.Kind), c.Entry.ID, c.Entry.Title), + Body: b.String(), + Scope: scope.Axes{Project: project}, + Confidence: proposalState, + Status: "active", + Origin: "distilled", + Harness: Harness, + SessionRef: CoverRef(c.Entry.ID, c.Kind), + } +} + +// ProposeCovers files one staged note per answer to the regression question that +// has no proposal yet, and returns how many. +func ProposeCovers(sink Sink, project string, existing []store.Knowledge, covers []Cover) (int, error) { + written := 0 + for _, c := range covers { + if CoverProposed(existing, c.Entry) { + continue + } + if _, err := sink.Remember(CoverProposal(project, c), scope.Axes{Project: project}); err != nil { + return written, fmt.Errorf("propose regression cover for #%d: %w", c.Entry.ID, err) + } + written++ + } + return written, nil +} diff --git a/internal/garden/propose.go b/internal/garden/propose.go index 033e6f0..faf392c 100644 --- a/internal/garden/propose.go +++ b/internal/garden/propose.go @@ -99,28 +99,41 @@ func RelationRef(r Relation) string { return fmt.Sprintf("gardener:relation:%s:%s", r.Kind, strings.Join(ids, "-")) } -// RelationProposal builds the staged note for a duplicate or a contradiction. -// titles maps entry id to title. -func RelationProposal(project string, r Relation, titles map[int64]string) store.Knowledge { - label := "Duplikat?" - if r.Kind == Contradiction { - label = "Widerspruch?" +// RelationLabel is the question a relation asks, as the title and the card say it. +func RelationLabel(kind string) string { + if kind == Contradiction { + return "Widerspruch?" } + return "Duplikat?" +} + +// relationText is what a relation says: the entries with the verbatim quote of +// each, the reason and the evidence. It is the body of the staged note and the +// first post of a contradiction thread. +func relationText(r Relation, titles map[int64]string) string { var b strings.Builder - var ids []string - fmt.Fprintf(&b, "Vorschlag des Gärtners (Herkunft gardener, nicht bestätigt): %s\n\n", label) + fmt.Fprintf(&b, "Vorschlag des Gärtners (Herkunft gardener, nicht bestätigt): %s\n\n", RelationLabel(r.Kind)) for _, id := range r.IDs { - ids = append(ids, fmt.Sprintf("#%d", id)) fmt.Fprintf(&b, "- #%d: %s\n Zitat: \"%s\"\n", id, titles[id], r.Quotes[id]) } fmt.Fprintf(&b, "\nBegründung: %s\n", r.Reason) if len(r.Evidence) > 0 { fmt.Fprintf(&b, "\nBelege: %s\n", strings.Join(r.Evidence, ", ")) } + return b.String() +} + +// RelationProposal builds the staged note for a duplicate or a contradiction. +// titles maps entry id to title. +func RelationProposal(project string, r Relation, titles map[int64]string) store.Knowledge { + var ids []string + for _, id := range r.IDs { + ids = append(ids, fmt.Sprintf("#%d", id)) + } return store.Knowledge{ Type: "note", - Title: fmt.Sprintf("%s%s %s: %s", titlePrefix, label, strings.Join(ids, " und "), titles[r.IDs[0]]), - Body: b.String(), + Title: fmt.Sprintf("%s%s %s: %s", titlePrefix, RelationLabel(r.Kind), strings.Join(ids, " und "), titles[r.IDs[0]]), + Body: relationText(r, titles), Scope: scope.Axes{Project: project}, Confidence: proposalState, Status: "active", @@ -130,6 +143,19 @@ func RelationProposal(project string, r Relation, titles map[int64]string) store } } +// RelationProposed says whether a note about this very relation exists, in any +// state: a person has seen it, and asking again would make their answer count for +// nothing. +func RelationProposed(existing []store.Knowledge, r Relation) bool { + ref := RelationRef(r) + for _, k := range existing { + if k.Harness == Harness && k.SessionRef == ref { + return true + } + } + return false +} + // ProposeRelations files one staged note per relation not already proposed // (in any state, so a rejection stays a rejection) and returns how many. func ProposeRelations(sink Sink, project string, existing []store.Knowledge, rels []Relation, titles map[int64]string) (int, error) { diff --git a/internal/garden/report.go b/internal/garden/report.go index 3b87280..fd9a7aa 100644 --- a/internal/garden/report.go +++ b/internal/garden/report.go @@ -153,6 +153,8 @@ func WriteReport(w io.Writer, project string, res Result, at time.Time) { fmt.Fprintln(w) } + writeCovers(w, res.Covers) + order := map[string]int{Stale: 0, Correction: 1, Unclear: 2, Holds: 3, Failed: 4, Skipped: 5, NotRun: 6} vs := append([]Verdict(nil), res.Verdicts...) sort.SliceStable(vs, func(i, j int) bool { return order[vs[i].Kind] < order[vs[j].Kind] }) @@ -188,6 +190,48 @@ func WriteReport(w io.Writer, project string, res Result, at time.Time) { } } +// writeCovers lists the answers to the regression question (job 5). The test +// sketches have a section of their own, whole: they are the part of the answer +// somebody has to write. The two other kinds are one line each. +func writeCovers(w io.Writer, covers []Cover) { + var sketches, others []Cover + for _, c := range covers { + if c.Kind == CoverSketch { + sketches = append(sketches, c) + } else { + others = append(others, c) + } + } + if len(sketches) > 0 { + fmt.Fprintf(w, "## Testskizzen (Regressionsschutz)\nZu diesen Pitfalls gibt es keinen Test, der ihre Wiederkehr bemerken würde. Der Gärtner skizziert einen; geschrieben wird er von einem Menschen.\n") + for _, c := range sketches { + fmt.Fprintf(w, "- #%d %s\n - Test: %s\n", c.Entry.ID, c.Entry.Title, c.TestID()) + for _, l := range strings.Split(c.Steps, "\n") { + fmt.Fprintf(w, " %s\n", l) + } + if c.Reason != "" { + fmt.Fprintf(w, " - Begründung: %s\n", c.Reason) + } + } + fmt.Fprintln(w) + } + if len(others) > 0 { + fmt.Fprintf(w, "## Regressionsschutz: vorhandene Tests und nicht Testbares\n") + for _, c := range others { + if c.Kind == CoverExisting { + fmt.Fprintf(w, "- #%d %s: %s deckt ihn ab", c.Entry.ID, c.Entry.Title, c.TestID()) + } else { + fmt.Fprintf(w, "- #%d %s: nicht testbar", c.Entry.ID, c.Entry.Title) + } + if c.Reason != "" { + fmt.Fprintf(w, " (%s)", c.Reason) + } + fmt.Fprintln(w) + } + fmt.Fprintln(w) + } +} + // Headline is the morning sentence: how many entries the model judged and how // many of those led to a proposal. func Headline(vs []Verdict) string { diff --git a/internal/garden/threads.go b/internal/garden/threads.go new file mode 100644 index 0000000..2192656 --- /dev/null +++ b/internal/garden/threads.go @@ -0,0 +1,307 @@ +package garden + +import ( + "fmt" + "regexp" + "sort" + "strconv" + "strings" + + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +// ThreadSink is the slice of the server client that opening a thread for a +// contradiction needs: look for threads on the entries, register as the agent +// that speaks for the gardener in the project room, open a thread, link it to +// the other entries, and write its first post. There is deliberately no way to +// set a thread's state, archive it, answer in a thread somebody else opened or +// record an outcome: a thread the gardener opened is closed by a person. +type ThreadSink interface { + RegisterCoordAgent(a store.CoordAgent) (int64, error) + ThreadsForObject(kind, id, agentExternalID string) ([]store.Thread, error) + ThreadLinks(id int64, agentExternalID string) ([]store.ThreadLink, error) + CreateThread(t store.Thread, agentExternalID string) (int64, error) + LinkThread(l store.ThreadLink, agentExternalID string) error + SendCoordMessage(m store.CoordMessage) (int64, error) +} + +// What became of one contradiction. +const ( + // ThreadOpened: a new thread was opened. + ThreadOpened = "opened" + // ThreadExists: an open thread for these entries exists already; nothing was opened. + ThreadExists = "open" + // ThreadDecided: somebody dealt with this before (a note of the gardener on + // it, or a thread they closed while the entries stayed as they were); nothing + // was opened, and nothing was reopened. + ThreadDecided = "decided" + // ThreadFailed: the thread could not be opened or looked for; the caller falls + // back to a staged note. + ThreadFailed = "failed" +) + +// ThreadOutcome is the result for one contradiction. +type ThreadOutcome struct { + Relation Relation + State string + // ThreadID is the thread opened or the one that stood in the way. + ThreadID int64 + // Warn names a part that did not work although the thread was opened. + Warn string + Err error +} + +const ( + // standLabel starts the line of a thread's question that records which wording + // of the entries the thread is about, so that a closed thread stays closed + // while they are unchanged and a changed entry may ask again. + standLabel = "Stand der Einträge beim Fund:" + // maxThreadTitle keeps the question title far below the store's limit. + maxThreadTitle = 220 + maxQuoteRunes = 800 +) + +var standRe = regexp.MustCompile(`#(\d+) \(([0-9a-f]{8,64})\)`) + +// ThreadAgentID is the coordination id under which the gardener speaks in the +// room of a project. +func ThreadAgentID(project string) string { return "gardener:" + project } + +// shorten cuts s to at most n runes. +func shorten(s string, n int) string { + s = strings.Join(strings.Fields(s), " ") + r := []rune(s) + if len(r) <= n { + return s + } + return string(r[:n-1]) + "…" +} + +// ThreadTitle is the contradiction as a question, marked as the gardener's. +func ThreadTitle(r Relation, titles map[int64]string) string { + var parts []string + for _, id := range r.IDs { + parts = append(parts, fmt.Sprintf("#%d „%s“", id, shorten(titles[id], 60))) + } + return shorten(fmt.Sprintf("%sWidersprechen sich %s?", titlePrefix, strings.Join(parts, " und ")), maxThreadTitle) +} + +// threadQuestion is the question of the thread: what is asked, who asks and +// the stand of the entries it was asked about. +func threadQuestion(r Relation, known map[int64]Entry) string { + var stand []string + for _, id := range r.IDs { + stand = append(stand, fmt.Sprintf("#%d (%s)", id, HashOf(known[id]))) + } + return fmt.Sprintf("Behaupten diese Einträge Unvereinbares, und welcher gilt? Der Gärtner (Herkunft gardener, nicht bestätigt) hat den Widerspruch gefunden; die Begründung und die wörtlichen Zitate stehen im ersten Beitrag. Der Gärtner entscheidet nichts.\n\n%s %s", + standLabel, strings.Join(stand, ", ")) +} + +// parseStand reads the stand line of a thread's question: entry id to hash. +func parseStand(question string) map[int64]string { + for _, line := range strings.Split(question, "\n") { + if !strings.HasPrefix(line, standLabel) { + continue + } + out := map[int64]string{} + for _, m := range standRe.FindAllStringSubmatch(line, -1) { + if id, err := strconv.ParseInt(m[1], 10, 64); err == nil { + out[id] = m[2] + } + } + return out + } + return nil +} + +// threadPost is the first post: the reason and the verbatim quotes. +func threadPost(r Relation, titles map[int64]string) string { + t := r + t.Quotes = map[int64]string{} + for id, q := range r.Quotes { + t.Quotes[id] = shorten(q, maxQuoteRunes) + } + return relationText(t, titles) + + "\nDieser Thread bleibt offen, bis ein Mensch ihn schließt. Er wird nicht wieder geöffnet, solange die Einträge unverändert sind." +} + +type threadOpener struct { + sink ThreadSink + project string + agent string + known map[int64]Entry + titles map[int64]string + joined bool +} + +func (o *threadOpener) join() error { + if o.joined { + return nil + } + _, err := o.sink.RegisterCoordAgent(store.CoordAgent{ + ExternalID: o.agent, Provider: Harness, RoomKey: store.RoomKeyForProject(o.project), + DisplayName: "Gärtner", Role: "member", + }) + o.joined = err == nil + return err +} + +// belongs says whether a thread is about exactly these entries: its stand line +// names them, or it is linked to all of them (a thread people opened has no stand +// line). +func (o *threadOpener) belongs(t store.Thread, r Relation) bool { + if stand := parseStand(t.Question); stand != nil { + same := len(stand) == len(r.IDs) + for _, id := range r.IDs { + if _, ok := stand[id]; !ok { + same = false + } + } + if same { + return true + } + } + links, err := o.sink.ThreadLinks(t.ID, o.agent) + if err != nil { + // Could not tell: counting it as one of ours is the safe error, it opens nothing. + return true + } + have := map[string]bool{} + for _, l := range links { + if l.Kind == "knowledge" { + have[l.ID] = true + } + } + for _, id := range r.IDs { + if !have[strconv.FormatInt(id, 10)] { + return false + } + } + return true +} + +// blocking finds the thread that makes opening another one wrong: an open thread +// for these entries, or a closed one that is not out of date. A closed thread of +// the gardener is out of date once one of its entries changed; a thread people +// opened counts in every state. +func (o *threadOpener) blocking(r Relation, found []store.Thread) (store.Thread, bool) { + for _, t := range found { + if !o.belongs(t, r) { + continue + } + if t.State == store.ThreadOpen && !t.Archived { + return t, true + } + stand := parseStand(t.Question) + if stand == nil { + return t, true + } + unchanged := true + for _, id := range r.IDs { + if stand[id] != HashOf(o.known[id]) { + unchanged = false + } + } + if unchanged { + return t, true + } + } + return store.Thread{}, false +} + +func (o *threadOpener) open(r Relation) ThreadOutcome { + out := ThreadOutcome{Relation: r} + if err := o.join(); err != nil { + out.State, out.Err = ThreadFailed, fmt.Errorf("register in the project room: %w", err) + return out + } + var found []store.Thread + seen := map[int64]bool{} + for _, id := range r.IDs { + ts, err := o.sink.ThreadsForObject("knowledge", strconv.FormatInt(id, 10), o.agent) + if err != nil { + // Not knowing whether there is a thread is not a reason to open one. + out.State, out.Err = ThreadFailed, fmt.Errorf("look for threads on #%d: %w", id, err) + return out + } + for _, t := range ts { + if !seen[t.ID] { + seen[t.ID] = true + found = append(found, t) + } + } + } + sort.SliceStable(found, func(i, j int) bool { return found[i].ID > found[j].ID }) + if t, ok := o.blocking(r, found); ok { + out.ThreadID = t.ID + out.State = ThreadDecided + if t.State == store.ThreadOpen && !t.Archived { + out.State = ThreadExists + } + return out + } + id, err := o.sink.CreateThread(store.Thread{ + Project: o.project, Title: ThreadTitle(r, o.titles), Question: threadQuestion(r, o.known), + Link: &store.ThreadLink{Kind: "knowledge", ID: strconv.FormatInt(r.IDs[0], 10)}, + }, o.agent) + if err != nil { + out.State, out.Err = ThreadFailed, fmt.Errorf("open thread: %w", err) + return out + } + out.State, out.ThreadID = ThreadOpened, id + var warn []string + for _, other := range r.IDs[1:] { + if err := o.sink.LinkThread(store.ThreadLink{ThreadID: id, Kind: "knowledge", ID: strconv.FormatInt(other, 10)}, o.agent); err != nil { + warn = append(warn, fmt.Sprintf("link #%d: %v", other, err)) + } + } + if _, err := o.sink.SendCoordMessage(store.CoordMessage{ + DestinationKind: store.DestinationDiscussion, DestinationID: store.ThreadDestinationID(id), + SenderExternalID: o.agent, ClientID: fmt.Sprintf("gardener-open-%d", id), Body: threadPost(r, o.titles), + }); err != nil { + warn = append(warn, fmt.Sprintf("first post: %v", err)) + } + out.Warn = strings.Join(warn, "; ") + return out +} + +// OpenContradictionThreads opens one thread per validated contradiction, linked +// to every entry it is about, with the gardener's reason and the verbatim quotes +// as the first post and the thread marked as the gardener's. It never reopens or +// closes anything, and it is idempotent: +// +// - a note of the gardener on the same relation, in any state, means somebody +// has seen it: no thread; +// - an open thread on the same entries means the question is being dealt with: +// no second one; +// - a thread closed by a person stays closed while the entries are unchanged +// (the thread's question records their wording); once one changed, a new +// thread may ask again, the old one is left as it is; +// - if the existing threads cannot be looked up, nothing is opened. +// +// known holds every entry a relation may name. +func OpenContradictionThreads(sink ThreadSink, project string, existing []store.Knowledge, rels []Relation, known map[int64]Entry) []ThreadOutcome { + titles := make(map[int64]string, len(known)) + for id, e := range known { + titles[id] = e.Title + } + o := &threadOpener{sink: sink, project: project, agent: ThreadAgentID(project), known: known, titles: titles} + var out []ThreadOutcome + done := map[string]bool{} + for _, r := range rels { + if r.Kind != Contradiction || len(r.IDs) < 2 { + continue + } + ref := RelationRef(r) + if done[ref] { + continue + } + done[ref] = true + if RelationProposed(existing, r) { + out = append(out, ThreadOutcome{Relation: r, State: ThreadDecided}) + continue + } + out = append(out, o.open(r)) + } + return out +} diff --git a/internal/server/gardener_gate_test.go b/internal/server/gardener_gate_test.go new file mode 100644 index 0000000..d7f6d13 --- /dev/null +++ b/internal/server/gardener_gate_test.go @@ -0,0 +1,208 @@ +package server + +import ( + "encoding/json" + "fmt" + "os" + "strings" + "testing" + + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +// REQ-457: harness "gardener" and "gardener:" references are what the review page +// acts on, so only the account whose number is set as the gardener's (`ctx account +// set-gardener`) may write them. The number, not a name: a name is what a person +// may change for themselves. + +// gardenerAPI is the access fixture with one more account, the gardener's, which +// is a member of the project, and set as the gardener's. +func gardenerAPI(t *testing.T) *accessAPIFixture { + t.Helper() + f := accessAPI(t, true) + if _, err := f.st.AddAccount("gardener", "", false); err != nil { + t.Fatal(err) + } + var err error + if f.tok["gardener"], _, err = f.st.CreateToken("gardener", store.TokenSpec{Label: "garden"}); err != nil { + t.Fatal(err) + } + code, _, err := f.st.CreateInvitation("person:1", 1, "", store.OrgMember, 0) + if err != nil { + t.Fatal(err) + } + if _, err := f.st.AcceptInvitation("person:7", code); err != nil { + t.Fatal(err) + } + if err := f.st.SetProjectRole("person:1", accProject, "person:7", store.RoleMember, false, store.RoleViaAPI); err != nil { + t.Fatal(err) + } + if err := f.st.SetGardenerAccount(7); err != nil { + t.Fatal(err) + } + return f +} + +func gardenerNote(harness, ref string) map[string]any { + return map[string]any{"type": "note", "title": "[Gärtner] Test vorhanden? #1: x", "body": "Test: a/x_test.go:TestX", "scope": map[string]string{"project": accProject}, + "confidence": "staged", "origin": "distilled", "harness": harness, "session_ref": ref} +} + +func countKnowledge(t *testing.T, st *store.Store) int { + t.Helper() + var n int + if err := st.DB().QueryRow(`SELECT COUNT(*) FROM knowledge`).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +func TestOnlyTheGardenersAccountMayWriteGardenerMarks(t *testing.T) { + f := gardenerAPI(t) + ref := "gardener:regression:1:existing" + + // The gardener's account: the note is written, as its own, and counts as its. + var saved store.Knowledge + body := f.expect(t, "gardener", 200, "POST", "/api/knowledge", gardenerNote("gardener", ref)) + if err := json.Unmarshal([]byte(body), &saved); err != nil { + t.Fatalf("%v: %s", err, body) + } + if saved.Harness != "gardener" || saved.SessionRef != ref || !f.st.WrittenByGardener(saved) { + t.Errorf("saved = %+v", saved) + } + + before := countKnowledge(t, f.st) + // Everyone else is refused, whatever their role: the instance owner, a lead, + // a reviewer, a member, a guest and an outsider. + for _, who := range []string{"robin", "lena", "rex", "mia", "gus", "nora"} { + for name, n := range map[string]map[string]any{ + "harness and reference": gardenerNote("gardener", ref), + "harness only": gardenerNote("gardener", ""), + "reference only": gardenerNote("", ref), + "harness in other case": gardenerNote("Gardener", ""), + "harness with space": gardenerNote(" gardener ", ""), + "reference in upper": gardenerNote("claude", "GARDENER:relation:duplicate:1-2"), + } { + code, out := f.call(t, who, "POST", "/api/knowledge", n) + if code != 403 && code != 404 { + t.Errorf("%s writing %s = %d (%s), want a refusal", who, name, code, strings.TrimSpace(out)) + } + if who != "nora" && who != "gus" && code == 403 && !strings.Contains(out, "gardener_reserved") { + t.Errorf("%s writing %s: refused without saying why: %s", who, name, out) + } + } + } + // The ones who may write at all (a lead, a reviewer, a member, the owner) are refused for this and only this: + for _, who := range []string{"robin", "lena", "rex", "mia"} { + f.expect(t, who, 403, "POST", "/api/knowledge", gardenerNote("gardener", ref)) + f.expect(t, who, 200, "POST", "/api/knowledge", gardenerNote("claude", "session-less")) + } + if got := countKnowledge(t, f.st) - before; got != 4 { + t.Errorf("%d entries written, want the 4 plain ones and no gardener entry", got) + } +} + +func TestGardenerMarksCannotRideOnASupersedeOrTheMigrationPath(t *testing.T) { + f := gardenerAPI(t) + old := f.id["k-mia"] + before := countKnowledge(t, f.st) + + n := gardenerNote("gardener", "gardener:relation:duplicate:1-2") + n["supersedes"] = []int64{old} + n["supersede_reason"] = "Gärtner-Vorschlag" + f.expect(t, "lena", 403, "POST", "/api/knowledge", n) + if k, _ := f.st.KnowledgeByID(old); k.Status != "active" || k.SupersededBy != 0 { + t.Errorf("the entry was superseded by a refused write: %+v", k) + } + + // The path of the migration takes the whole entry from the client too. + for _, who := range []string{"lena", "mia", "robin"} { + f.expect(t, who, 403, "POST", "/api/migrated-knowledge", map[string]any{"Knowledge": gardenerNote("gardener", "gardener:regression:1:existing")}) + f.expect(t, who, 403, "POST", "/api/migrated-knowledge", map[string]any{"Knowledge": gardenerNote("", "gardener:regression:1:existing")}) + } + if got := countKnowledge(t, f.st); got != before { + t.Errorf("%d entries written by refused calls", got-before) + } + // An entry cannot be turned into one afterwards either: harness and reference + // are not among the fields a patch may change. + f.expect(t, "lena", 400, "PATCH", fmt.Sprintf("/api/knowledge/%d", old), map[string]string{"harness": "gardener"}) + f.expect(t, "lena", 400, "PATCH", fmt.Sprintf("/api/knowledge/%d", old), map[string]string{"session_ref": "gardener:regression:1:existing"}) +} + +func TestNobodyMayWriteGardenerMarksWhenNoAccountIsSet(t *testing.T) { + f := gardenerAPI(t) + if err := f.st.ClearGardenerAccount(); err != nil { + t.Fatal(err) + } + out := f.expect(t, "gardener", 403, "POST", "/api/knowledge", gardenerNote("gardener", "gardener:regression:1:existing")) + if !strings.Contains(out, "gardener_reserved") || !strings.Contains(out, "ctx account set-gardener") { + t.Errorf("the refusal does not say how to set it up: %s", out) + } + f.expect(t, "robin", 403, "POST", "/api/knowledge", gardenerNote("gardener", "")) + // And what is not the gardener's still works for everyone, set or not. + f.expect(t, "gardener", 200, "POST", "/api/knowledge", gardenerNote("claude", "")) + f.expect(t, "mia", 200, "POST", "/api/knowledge", gardenerNote("gardenerbot", "gardener")) +} + +// The role is the account's number. Renaming the gardener's account does not +// take it away, and an account that gets the gardener's name does not get it. +func TestTheGardenersRoleDoesNotFollowTheName(t *testing.T) { + f := gardenerAPI(t) + ref := "gardener:regression:1:existing" + // Whoever may rename themselves renames the gardener's account (the person who + // owns it) and gives its old name to the account of a member. + if _, err := f.st.SetOwnName("person:7", "garden-keeper"); err != nil { + t.Fatal(err) + } + f.expect(t, "gardener", 200, "POST", "/api/knowledge", gardenerNote("gardener", ref)) + for _, q := range []string{`UPDATE persons SET name='garden-keeper-2' WHERE id=7`, `UPDATE persons SET name='gardener' WHERE id=3`} { + if _, err := f.st.DB().Exec(q); err != nil { + t.Fatal(err) + } + } + if a, _ := f.st.AccountByName("gardener"); a.ID != "person:3" { + t.Fatalf("the name did not move: %+v", a) + } + // mia (person:3) now carries the name "gardener". + out := f.expect(t, "mia", 403, "POST", "/api/knowledge", gardenerNote("gardener", ref)) + if !strings.Contains(out, "gardener_reserved") { + t.Errorf("refused without saying why: %s", out) + } + f.expect(t, "mia", 403, "POST", "/api/knowledge", gardenerNote("", ref)) + f.expect(t, "gardener", 200, "POST", "/api/knowledge", gardenerNote("gardener", ref)) +} + +// Nothing a person can reach over HTTP sets the gardener's account: the only +// callers of the two functions that write it are the account command and tests. +// The packages that answer requests (the API, the web pages, the MCP server) are +// read for them, so a new route that did would fail here and be looked at. +func TestNoRequestHandlerSetsTheGardenersAccount(t *testing.T) { + for _, dir := range []string{".", "../web", "../mcpserver", "../client"} { + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".go") || strings.HasSuffix(e.Name(), "_test.go") { + continue + } + raw, err := os.ReadFile(dir + "/" + e.Name()) + if err != nil { + t.Fatal(err) + } + for _, call := range []string{"SetGardenerAccount(", "ClearGardenerAccount(", "gardener_account"} { + if strings.Contains(string(raw), call) { + t.Errorf("%s/%s mentions %s: a request must not set the gardener's account", dir, e.Name(), call) + } + } + } + } + // And the account stays what it was after requests of people. + f := gardenerAPI(t) + for _, who := range []string{"robin", "lena", "mia"} { + f.call(t, who, "POST", "/api/knowledge", map[string]any{"type": "note", "title": "gardener_account", "body": "1", "scope": map[string]string{"project": accProject}}) + } + if id := f.st.GardenerAccountID(); id != 7 { + t.Errorf("the gardener's account is %d after requests of people", id) + } +} diff --git a/internal/server/handlers.go b/internal/server/handlers.go index ee101a6..d63e50c 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -247,6 +247,9 @@ func (a *api) createKnowledge(w http.ResponseWriter, r *http.Request) { if k.Scope.IsGlobal() && req.AutoScope != nil { k.Scope = scope.DefaultAxes(k.Type, req.AutoScope.Context) } + if !a.gateGardener(w, r, k) { + return + } if !a.gateMachineNote(w, r, k.Scope.Machine) || !a.gateProject(w, r, k.Scope.Project) { return } @@ -295,6 +298,25 @@ func (a *api) createKnowledge(w http.ResponseWriter, r *http.Request) { writeJSON(w, 200, a.access(r).KnowledgeView(saved)) } +// gateGardener keeps the gardener's marks the gardener's (REQ-457). An entry with +// harness "gardener" or a "gardener:" session reference is what the review page +// offers one-click actions on (merge duplicates, record a regression verdict), +// and it takes its target from that reference; so only the account the server +// names as the gardener's (an account number, set with `ctx account set-gardener`) +// may write one. Everyone else, and everyone when no account is set, is refused +// before anything is written, whatever else the entry says. +func (a *api) gateGardener(w http.ResponseWriter, r *http.Request, k store.Knowledge) bool { + if !store.ReservedForGardener(k) || a.st.IsGardenerPrincipal(principalOf(r)) { + return true + } + msg := `harness "gardener" and "gardener:" references are reserved for the account the gardener runs as` + if a.st.GardenerAccountID() == 0 { + msg += " (none is set: ctx account set-gardener --db )" + } + writeCoded(w, http.StatusForbidden, "gardener_reserved", msg) + return false +} + func (a *api) listKnowledge(w http.ResponseWriter, r *http.Request) { var ks []store.Knowledge var err error @@ -326,6 +348,9 @@ func (a *api) insertMigratedKnowledge(w http.ResponseWriter, r *http.Request) { writeStoreError(w, http.StatusBadRequest, err) return } + if !a.gateGardener(w, r, in.Knowledge) { + return + } in.Knowledge.Person = personOf(r) in.Knowledge.AuthorID = authorIDOf(r) in.Knowledge.Scope = scope.CanonicalAxes(in.Knowledge.Scope) diff --git a/internal/store/gardener.go b/internal/store/gardener.go new file mode 100644 index 0000000..ce7d7fb --- /dev/null +++ b/internal/store/gardener.go @@ -0,0 +1,130 @@ +package store + +import ( + "fmt" + "strconv" + "strings" +) + +// The gardener (REQ-457) writes notes that a person confirms with one click, and +// two of those clicks change other entries (merge duplicates, record a regression +// verdict). What such a note says about its target comes from its session +// reference and its text, so who may write a note that claims to be the +// gardener's is a security question: a note that only LOOKS like the gardener's +// would make the review page act on a target the card does not show. +// +// The gardener is an account NUMBER, set by whoever has the database of the +// server in their hands (`ctx account set-gardener`), kept in org_state, and read +// whenever it is asked. It is never an account name: a name is what a person may +// change for themselves (SetOwnName, the name the identity provider reports), so +// whoever could take the name would take the role. The number of an account never +// changes and is never given out again, so renaming the gardener's account does +// not take the role away, and renaming another account to the old name gives it +// nothing. Nothing reachable over HTTP writes this setting. +// +// Without a gardener account nobody may write harness "gardener" or a "gardener:" +// reference (the API refuses it), and the review page acts on no note of the +// gardener's. +const ( + // GardenerHarness is the harness of every entry the gardener writes. + GardenerHarness = "gardener" + // GardenerRefPrefix starts the session reference of a gardener note; what + // follows names the kind and the entries it is about. + GardenerRefPrefix = "gardener:" + + // gardenerAccountKey is the org_state key that holds the account number. + gardenerAccountKey = "gardener_account" +) + +// GardenerAccountID is the number of the account the gardener runs as (the n of +// "person:n"), 0 when none is set. +func (s *Store) GardenerAccountID() int64 { + if s.reader != nil { + return s.reader.GardenerAccountID() + } + var v string + if s.db.QueryRow(`SELECT value FROM org_state WHERE key=?`, gardenerAccountKey).Scan(&v) != nil { + return 0 + } + id, err := strconv.ParseInt(strings.TrimSpace(v), 10, 64) + if err != nil || id < 1 { + return 0 + } + return id +} + +// GardenerAccount is the account the gardener runs as, as it is named now. The +// name is for people to read; nothing decides by it. +func (s *Store) GardenerAccount() (Account, bool) { + if s.reader != nil { + return s.reader.GardenerAccount() + } + id := s.GardenerAccountID() + if id == 0 { + return Account{}, false + } + a, err := s.AccountByPrincipalID(fmt.Sprintf("person:%d", id)) + return a, err == nil +} + +// SetGardenerAccount makes the account with this number the gardener's. The +// account has to exist and be active; the number is what is stored. +func (s *Store) SetGardenerAccount(id int64) error { + if s.writer != nil { + return queueWrite(s, []any{id}, func(d *Store, p []any) error { return d.SetGardenerAccount(p[0].(int64)) }) + } + a, err := s.AccountByPrincipalID(fmt.Sprintf("person:%d", id)) + if err != nil { + return fmt.Errorf("%w: no account with the number %d", ErrInvalidInput, id) + } + if a.State != "active" { + return fmt.Errorf("%w: the account %s is %s, not active", ErrInvalidInput, a.Name, a.State) + } + _, err = s.db.Exec(`INSERT INTO org_state(key, value) VALUES(?, ?) + ON CONFLICT(key) DO UPDATE SET value=excluded.value`, gardenerAccountKey, strconv.FormatInt(id, 10)) + return err +} + +// ClearGardenerAccount takes the role away: nobody is the gardener afterwards. +func (s *Store) ClearGardenerAccount() error { + if s.writer != nil { + return queueWrite(s, nil, func(d *Store, _ []any) error { return d.ClearGardenerAccount() }) + } + _, err := s.db.Exec(`DELETE FROM org_state WHERE key=?`, gardenerAccountKey) + return err +} + +// IsGardenerPrincipal says whether the caller is the gardener's account with a +// token. A browser session of that account is not: nothing the gardener writes +// comes from one. +func (s *Store) IsGardenerPrincipal(p Principal) bool { + if s.reader != nil { + return s.reader.IsGardenerPrincipal(p) + } + want := s.GardenerAccountID() + if want == 0 || p.TokenKind == WebSessionKind { + return false + } + got, ok := accountNumericID(p.ID) + return ok && got == want +} + +// ReservedForGardener says whether a new entry claims to be the gardener's: +// harness "gardener" or a "gardener:" session reference, in any spelling of case +// or surrounding space. +func ReservedForGardener(k Knowledge) bool { + return strings.EqualFold(strings.TrimSpace(k.Harness), GardenerHarness) || + strings.HasPrefix(strings.ToLower(strings.TrimSpace(k.SessionRef)), GardenerRefPrefix) +} + +// WrittenByGardener says whether the account that wrote the entry is the +// gardener's. The author is the account number stored when the entry was +// written (knowledge_authors), never a name or a label the writer sent along; an +// entry without an author (legacy, or one that fitted two accounts) is nobody's. +func (s *Store) WrittenByGardener(k Knowledge) bool { + if s.reader != nil { + return s.reader.WrittenByGardener(k) + } + want := s.GardenerAccountID() + return want != 0 && k.ID != 0 && s.knowledgeAuthorID(k.ID) == want +} diff --git a/internal/store/gardener_test.go b/internal/store/gardener_test.go new file mode 100644 index 0000000..7040569 --- /dev/null +++ b/internal/store/gardener_test.go @@ -0,0 +1,260 @@ +package store + +import ( + "errors" + "testing" + + "github.com/Deadweight-Labs/ghosttree/internal/scope" +) + +func TestReservedForGardenerCatchesEveryClaimToBeTheGardeners(t *testing.T) { + for name, k := range map[string]Knowledge{ + "harness": {Harness: "gardener"}, + "harness, other case": {Harness: "Gardener"}, + "harness with space": {Harness: " gardener\t"}, + "reference": {SessionRef: "gardener:regression:12:existing"}, + "reference, upper": {SessionRef: "GARDENER:relation:duplicate:1-2"}, + "reference with space": {SessionRef: " gardener:check-knowledge:4"}, + "both": {Harness: "gardener", SessionRef: "gardener:x"}, + } { + if !ReservedForGardener(k) { + t.Errorf("%s is not reserved", name) + } + } + for name, k := range map[string]Knowledge{ + "nothing": {}, + "another harness": {Harness: "claude"}, + "a harness that continues": {Harness: "gardener-bot"}, + "a reference without colon": {SessionRef: "gardener"}, + "a session reference": {SessionRef: "session:12#3"}, + "a word in the title": {Title: "gardener", Body: "gardener:"}, + } { + if ReservedForGardener(k) { + t.Errorf("%s is reserved", name) + } + } +} + +func gardenerStore(t *testing.T, names ...string) *Store { + t.Helper() + st, err := Open(":memory:") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { st.Close() }) + for _, n := range names { + if _, err := st.AddPerson(n); err != nil { + t.Fatal(err) + } + } + return st +} + +func tokenOf(id string) Principal { return Principal{ID: id, TokenKind: "token"} } + +// Nothing is the gardener until an account number is set, and the setting +// takes an account that exists and is active. +func TestNobodyIsTheGardenerUntilAnAccountNumberIsSet(t *testing.T) { + st := gardenerStore(t, "alice", "gardener", "mallory") + if st.GardenerAccountID() != 0 { + t.Fatal("an account is the gardener's before one was set") + } + if _, ok := st.GardenerAccount(); ok { + t.Error("an account is shown as the gardener's before one was set") + } + for _, p := range []Principal{tokenOf("person:1"), tokenOf("person:2"), tokenOf("person:3")} { + if st.IsGardenerPrincipal(p) { + t.Errorf("%s is the gardener with none set", p.ID) + } + } + // The account called "gardener" is not the gardener for its name. + if err := st.SetGardenerAccount(99); !errors.Is(err, ErrInvalidInput) { + t.Errorf("an account that does not exist: %v", err) + } + if err := st.SetGardenerAccount(0); !errors.Is(err, ErrInvalidInput) { + t.Errorf("account 0: %v", err) + } + if _, err := st.DB().Exec(`UPDATE persons SET state='disabled' WHERE id=3`); err != nil { + t.Fatal(err) + } + if err := st.SetGardenerAccount(3); !errors.Is(err, ErrInvalidInput) { + t.Errorf("a disabled account: %v", err) + } + if st.GardenerAccountID() != 0 { + t.Fatal("a refused setting was stored") + } + + if err := st.SetGardenerAccount(2); err != nil { + t.Fatal(err) + } + if a, ok := st.GardenerAccount(); !ok || st.GardenerAccountID() != 2 || a.Name != "gardener" { + t.Fatalf("account %+v %v id %d", a, ok, st.GardenerAccountID()) + } + if !st.IsGardenerPrincipal(tokenOf("person:2")) { + t.Error("the account that was set is not the gardener") + } + for name, p := range map[string]Principal{ + "another account": tokenOf("person:1"), + "the gardener in a browser": {ID: "person:2", TokenKind: WebSessionKind}, + "no principal": {}, + "an id that is not an account": tokenOf("gardener"), + "an agent id with the same number": tokenOf("agent:2"), + "a number written another way": tokenOf("person:02"), + } { + if st.IsGardenerPrincipal(p) { + t.Errorf("%s is the gardener", name) + } + } + // Taking the role away leaves nobody. + if err := st.ClearGardenerAccount(); err != nil { + t.Fatal(err) + } + if st.GardenerAccountID() != 0 || st.IsGardenerPrincipal(tokenOf("person:2")) { + t.Error("the gardener stayed after the role was taken away") + } +} + +// A name is what a person changes for themselves, so it must carry no right: the +// gardener's account keeps the role when it is renamed, and another account that +// takes the name gets nothing. Both are done with the functions a person has, +// SetOwnName (what the profile page calls). +func TestRenamingAccountsNeitherGivesNorTakesTheGardenersRole(t *testing.T) { + st := gardenerStore(t, "alice", "gardener", "mallory") + if err := st.SetGardenerAccount(2); err != nil { + t.Fatal(err) + } + gardener, mallory := tokenOf("person:2"), tokenOf("person:3") + + // The gardener's account is renamed (by itself, the way any account may rename itself): + // it is still the gardener. + if _, err := st.SetOwnName("person:2", "garden-keeper"); err != nil { + t.Fatal(err) + } + if !st.IsGardenerPrincipal(gardener) || st.GardenerAccountID() != 2 { + t.Error("renaming the gardener's account took the role away") + } + if a, ok := st.GardenerAccount(); !ok || a.Name != "garden-keeper" { + t.Errorf("the account is shown as %+v", a) + } + // The store keeps an old name for the account that held it, so another account + // cannot take it with SetOwnName at all. + if _, err := st.SetOwnName("person:3", "gardener"); err == nil { + t.Error("another account took the name the gardener's account had") + } + // The role does not rest on that: should a name get to another account by any + // other way (the name the identity provider reports, an administrator's tool, a + // future mistake), it gives nothing, and the gardener keeps the role. + for _, q := range []string{ + `UPDATE persons SET name='garden-keeper-2' WHERE id=2`, + `UPDATE persons SET name='gardener' WHERE id=3`, + } { + if _, err := st.DB().Exec(q); err != nil { + t.Fatal(err) + } + } + if a, _ := st.AccountByName("gardener"); a.ID != "person:3" { + t.Fatalf("the test did not move the name: %+v", a) + } + if st.IsGardenerPrincipal(mallory) { + t.Fatal("an account that holds the gardener's name became the gardener") + } + if !st.IsGardenerPrincipal(gardener) || st.GardenerAccountID() != 2 { + t.Error("the gardener lost the role to the account that holds its old name") + } +} + +func TestAnEntryIsTheGardenersWhenTheGardenersAccountWroteIt(t *testing.T) { + st := gardenerStore(t, "alice", "gardener") + write := func(author int64, person string) Knowledge { + id, err := st.InsertKnowledge(Knowledge{Type: "note", Title: "t", Body: "b", Scope: scope.Axes{Project: "github.com/o/r"}, Person: person, AuthorID: author, Harness: "gardener", SessionRef: "gardener:regression:1:existing"}) + if err != nil { + t.Fatal(err) + } + k, err := st.KnowledgeByID(id) + if err != nil { + t.Fatal(err) + } + return k + } + if st.WrittenByGardener(write(2, "gardener")) { + t.Error("an entry counts while no account is set") + } + if err := st.SetGardenerAccount(2); err != nil { + t.Fatal(err) + } + mine, labelled := write(2, "gardener"), write(1, "gardener") + if !st.WrittenByGardener(mine) { + t.Error("an entry of the gardener's account is not the gardener's") + } + // The label a writer sends is no proof: the author is the account number. + if st.WrittenByGardener(labelled) { + t.Error("the label gardener on an entry of another account counts") + } + if st.WrittenByGardener(Knowledge{}) { + t.Error("an entry that was never written counts") + } + // Another account made the gardener's: the earlier entries are not its own any more. + if err := st.SetGardenerAccount(1); err != nil { + t.Fatal(err) + } + if st.WrittenByGardener(mine) || !st.WrittenByGardener(labelled) { + t.Error("the role did not move with the number") + } + if err := st.ClearGardenerAccount(); err != nil { + t.Fatal(err) + } + if st.WrittenByGardener(mine) || st.WrittenByGardener(labelled) { + t.Error("an entry counts after the role was taken away") + } +} + +// The server runs the store with a writer and a read-only pool: the setting goes +// through the writer, and every read, from either side, sees what was set. A +// second process (ctx account set-gardener on the host) writes the same file. +func TestGardenerAccountWorksWithTheRuntimeWriterAndFromAnotherProcess(t *testing.T) { + path := t.TempDir() + "/g.db" + st, err := OpenRuntime(path, DefaultWriterConfig()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { st.Close() }) + for _, n := range []string{"alice", "gardener"} { + if _, err := st.AddPerson(n); err != nil { + t.Fatal(err) + } + } + if st.GardenerAccountID() != 0 { + t.Fatal("set before anything was") + } + if err := st.SetGardenerAccount(2); err != nil { + t.Fatal(err) + } + if st.GardenerAccountID() != 2 || !st.IsGardenerPrincipal(tokenOf("person:2")) { + t.Error("the setting through the writer is not seen") + } + if err := st.SetGardenerAccount(77); err == nil { + t.Error("an account that does not exist was set through the writer") + } + if st.GardenerAccountID() != 2 { + t.Error("a refused setting changed the stored one") + } + + // The command line opens the file itself, beside the running server. + other, err := Open(path) + if err != nil { + t.Fatal(err) + } + if err := other.SetGardenerAccount(1); err != nil { + t.Fatal(err) + } + other.Close() + if st.GardenerAccountID() != 1 || st.IsGardenerPrincipal(tokenOf("person:2")) || !st.IsGardenerPrincipal(tokenOf("person:1")) { + t.Error("the running server does not see the change of another process") + } + if err := st.ClearGardenerAccount(); err != nil { + t.Fatal(err) + } + if st.GardenerAccountID() != 0 { + t.Error("clearing through the writer is not seen") + } +} diff --git a/internal/store/writer_inventory_test.go b/internal/store/writer_inventory_test.go index 6fc4777..e8a75e1 100644 --- a/internal/store/writer_inventory_test.go +++ b/internal/store/writer_inventory_test.go @@ -317,6 +317,12 @@ var writerMethodClasses = map[string]string{ "SetCoordWorkNote": "write", "SetCriterionState": "write", "SetRegressionCover": "write", + "GardenerAccountID": "read", + "GardenerAccount": "read", + "SetGardenerAccount": "write", + "ClearGardenerAccount": "write", + "IsGardenerPrincipal": "read", + "WrittenByGardener": "read", "Explanation": "read", "SaveExplanation": "write", "SaveAgentExplanation": "administrative", diff --git a/internal/web/knowledge.go b/internal/web/knowledge.go index 7f67265..4a14997 100644 --- a/internal/web/knowledge.go +++ b/internal/web/knowledge.go @@ -256,6 +256,7 @@ var levelKeys = map[string]string{ var doneKeys = map[string]string{ "approved": "knowledge.done.approved", "rejected": "knowledge.done.rejected", "released": "knowledge.done.released", "retired": "knowledge.done.retired", "restored": "knowledge.done.restored", "saved": "knowledge.done.saved", + "merged": "knowledge.done.merged", "covered": "knowledge.done.covered", "untestable": "knowledge.done.untestable", } func typeLabel(l lang, typ string) string { diff --git a/internal/web/messages.go b/internal/web/messages.go index 6dcd40d..e76eb6a 100644 --- a/internal/web/messages.go +++ b/internal/web/messages.go @@ -177,6 +177,18 @@ var messages = map[string]string{ "review.garden.correction": "Correction?", "review.garden.reject_all": "Reject all", "review.garden.approve_all": "Approve all %d", + "review.garden.duplicate": "Duplicate?", + "review.garden.contradiction": "Contradiction?", + "review.garden.cover_existing": "Test exists?", + "review.garden.cover_sketch": "Test missing?", + "review.garden.cover_untestable": "Not testable?", + "review.garden.merge": "Merge", + "review.garden.stays": "Stays:", + "review.garden.replaced": "Replaced:", + "review.garden.record_cover": "Record as covered", + "review.garden.record_none": "Record as not testable", + "review.garden.about": "Concerns:", + "review.garden.unverified": "Not vouched for as the gardener's: no action.", "review.select": "Select", "review.bulk.selected": "%d selected", "review.bulk.some": "Selection", @@ -212,6 +224,9 @@ var messages = map[string]string{ "knowledge.done.released": "Released to the reviewers: %s", "knowledge.done.retired": "Retired: %s", "knowledge.done.restored": "Restored: %s", + "knowledge.done.merged": "Merged: %s now replaces the older entry", + "knowledge.done.covered": "Recorded as covered: %s", + "knowledge.done.untestable": "Recorded as not testable: %s", "knowledge.replaced_by": "replaced by %s", "knowledge.closure.retired": "Retired", "knowledge.closure.superseded": "Replaced by", diff --git a/internal/web/messages_de.go b/internal/web/messages_de.go index 20b0bf5..b2f21c1 100644 --- a/internal/web/messages_de.go +++ b/internal/web/messages_de.go @@ -178,6 +178,18 @@ var messagesDE = map[string]string{ "review.garden.correction": "Korrektur?", "review.garden.reject_all": "Alle ablehnen", "review.garden.approve_all": "Alle %d freigeben", + "review.garden.duplicate": "Duplikat?", + "review.garden.contradiction": "Widerspruch?", + "review.garden.cover_existing": "Test vorhanden?", + "review.garden.cover_sketch": "Test fehlt?", + "review.garden.cover_untestable": "Nicht testbar?", + "review.garden.merge": "Zusammenführen", + "review.garden.stays": "Bleibt:", + "review.garden.replaced": "Wird ersetzt:", + "review.garden.record_cover": "Als abgesichert vermerken", + "review.garden.record_none": "Als nicht testbar vermerken", + "review.garden.about": "Betrifft:", + "review.garden.unverified": "Herkunft nicht bestätigt: keine Aktion.", "review.select": "Auswählen", "review.bulk.selected": "%d ausgewählt", "review.bulk.some": "Auswahl", @@ -213,6 +225,9 @@ var messagesDE = map[string]string{ "knowledge.done.released": "An die Reviewer zurückgegeben: %s", "knowledge.done.retired": "Zurückgezogen: %s", "knowledge.done.restored": "Wiederhergestellt: %s", + "knowledge.done.merged": "Zusammengeführt: %s ersetzt jetzt den älteren Eintrag", + "knowledge.done.covered": "Als abgesichert vermerkt: %s", + "knowledge.done.untestable": "Als nicht testbar vermerkt: %s", "knowledge.replaced_by": "ersetzt durch %s", "knowledge.closure.retired": "Zurückgezogen", "knowledge.closure.superseded": "Ersetzt durch", diff --git a/internal/web/review_garden.go b/internal/web/review_garden.go index c3e74ab..aa10873 100644 --- a/internal/web/review_garden.go +++ b/internal/web/review_garden.go @@ -30,6 +30,15 @@ type gardenRow struct { Explain *explainView CanApprove, CanReject bool Token, Next, Filter string + // Lines are what the note holds beyond its reason (the test, the sketch); + // Refs the entries a merge keeps and replaces; Act the one action besides + // rejecting, for a note that has one (see review_garden_actions.go). + Lines []string + Refs []gardenRef + Act *gardenAct + // Warn says a note claims to be the gardener's and is not vouched for: no + // action is offered on it. + Warn string } // Selectable: the row has a tick box, because at least one bulk action is open. @@ -76,6 +85,7 @@ func (a *app) gardenBlock(r *http.Request, pa *store.ProjectAccess, project stri Explain: card.Explain, CanApprove: card.CanApprove, CanReject: card.CanReject, Token: v.Token, Next: "review", Filter: project, } + a.decorateGardenRow(l, pa, k, &row) if row.CanApprove { v.ApproveIDs = append(v.ApproveIDs, row.ID) } @@ -151,6 +161,16 @@ func parseGardenTitle(l lang, title string) (verdict, subject string) { return l.msg("review.garden.stale"), subject case "Korrektur?": return l.msg("review.garden.correction"), subject + case "Duplikat?": + return l.msg("review.garden.duplicate"), subject + case "Widerspruch?": + return l.msg("review.garden.contradiction"), subject + case "Test vorhanden?": + return l.msg("review.garden.cover_existing"), subject + case "Test fehlt?": + return l.msg("review.garden.cover_sketch"), subject + case "Nicht testbar?": + return l.msg("review.garden.cover_untestable"), subject } return strings.TrimSpace(label), subject } diff --git a/internal/web/review_garden_actions.go b/internal/web/review_garden_actions.go new file mode 100644 index 0000000..5ac725f --- /dev/null +++ b/internal/web/review_garden_actions.go @@ -0,0 +1,510 @@ +package web + +import ( + "database/sql" + "errors" + "fmt" + "net/http" + "regexp" + "slices" + "strconv" + "strings" + + "github.com/Deadweight-Labs/ghosttree/internal/store" +) + +// The two things a person can do with a note of the gardener besides approving +// or rejecting it, and the only way the gardener's findings ever become a change +// to other entries: +// +// - "Merge" on a duplicate note: the older entry is superseded by the newer one, +// the same store call and the same right as context_supersede (the right to +// change every entry involved); +// - "Record" on a regression note about an existing test or an untestable +// pitfall: the pitfall gets its regression verdict (covered with the test, or +// not applicable), the same call and the same right as the regression tool. +// +// Both close the note as confirmed by the person. Nothing here is reachable +// without a person: the gardener has no call for it, and the page offers the +// button only to someone who may do what it does. +// +// A note takes its target from its session reference, and the regression note +// also the test from its text, so the page does not act on anything that merely +// looks like a note of the gardener. It acts only when the note was written by +// the account the server names as the gardener's (store.WrittenByGardener; the +// API refuses everyone else the harness), has not been edited since, and when the +// entries its title names are the ones its reference names. The card shows +// the entries from the reference, with title and link, never the ones the title +// claims. The test of a regression note is recorded as the note holds it: the +// server has no checkout to check it against, so it is held to a strict shape, +// shown whole before the button, and bound to a note nobody has changed; the +// request that presses the button carries no test of its own. + +const ( + // The session references the gardener writes (garden.RelationRef and + // garden.CoverRef; a test keeps both in step, so the web package need not import + // the gardener). + gardenRelationRef = "gardener:relation:" + gardenRegressionRef = "gardener:regression:" + + gardenKindDuplicate = "duplicate" + gardenKindExisting = "existing" + gardenKindSketch = "sketch" + gardenKindUntestable = "untestable" +) + +// errNothingToDo: the note no longer asks for anything (the entries changed, or +// somebody was quicker). +var errNothingToDo = errors.New("nothing to change") + +// errNotGardens: the note is not what it says it is, and no action is taken on it. +var errNotGardens = errors.New("the note does not come from the gardener") + +// gardenTestRe is the shape of the test a regression note may name: a relative +// file path and a function name, joined by a colon. No spaces, no "..", nothing +// that is not part of a path or an identifier. +var gardenTestRe = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9_.\-]*(?:/[A-Za-z0-9_][A-Za-z0-9_.\-]*)*:[A-Za-z_][A-Za-z0-9_]{1,119}$`) + +// validTestID: a test as the gardener names it, "path/to/x_test.go:TestName". +func validTestID(t string) bool { + if len(t) > 300 || !gardenTestRe.MatchString(t) { + return false + } + file, _, _ := strings.Cut(t, ":") + for _, seg := range strings.Split(file, "/") { + if seg == ".." || seg == ".git" || seg == "." { + return false + } + } + return true +} + +// titleIDs reads the entry numbers a note's title names: "[Gärtner] Duplikat? +// #12 und #15: Titel" gives 12 and 15. +func titleIDs(title string) ([]int64, bool) { + rest := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(title), "[Gärtner]")) + _, after, ok := strings.Cut(rest, " #") + if !ok { + return nil, false + } + list, _, ok := strings.Cut(after, ": ") + if !ok { + return nil, false + } + var ids []int64 + for _, part := range strings.Split(list, " und #") { + id, err := strconv.ParseInt(part, 10, 64) + if err != nil || id < 1 { + return nil, false + } + ids = append(ids, id) + } + return ids, true +} + +// sameIDs: the same set of entry numbers, in any order. +func sameIDs(a, b []int64) bool { + if len(a) != len(b) { + return false + } + x, y := slices.Clone(a), slices.Clone(b) + slices.Sort(x) + slices.Sort(y) + return slices.Equal(x, y) +} + +// gardenerVouches says whether a note is the gardener's: harness and reference +// are only a claim, so the account that wrote it has to be the one the server +// names as the gardener's, and nobody may have changed it since (a note is +// archived before every change, so a note with a history was edited). A failure +// to look is returned as it is. +func (a *app) gardenerVouches(note store.Knowledge, ids []int64) error { + if note.Harness != gardenerHarness || !a.store.WrittenByGardener(note) { + return errNotGardens + } + if got, ok := titleIDs(note.Title); !ok || !sameIDs(got, ids) { + return errNotGardens + } + history, err := a.store.KnowledgeHistory(note.ID) + if err != nil { + return err + } + if len(history) > 0 { + return errNotGardens + } + return nil +} + +// parseRelationRef reads "gardener:relation::-". +func parseRelationRef(ref string) (kind string, ids []int64, ok bool) { + rest, found := strings.CutPrefix(ref, gardenRelationRef) + if !found { + return "", nil, false + } + kind, list, found := strings.Cut(rest, ":") + if !found { + return "", nil, false + } + for _, part := range strings.Split(list, "-") { + id, err := strconv.ParseInt(part, 10, 64) + if err != nil || id < 1 { + return "", nil, false + } + ids = append(ids, id) + } + return kind, ids, len(ids) > 0 +} + +// parseRegressionRef reads "gardener:regression::". +func parseRegressionRef(ref string) (id int64, kind string, ok bool) { + rest, found := strings.CutPrefix(ref, gardenRegressionRef) + if !found { + return 0, "", false + } + raw, kind, found := strings.Cut(rest, ":") + if !found { + return 0, "", false + } + id, err := strconv.ParseInt(raw, 10, 64) + return id, kind, err == nil && id > 0 +} + +// gardenNoteLine is the value of the first line "