From b39fabfda487577e960be59cc3dd8fd09758db35 Mon Sep 17 00:00:00 2001 From: Tim Disney Date: Mon, 27 Jul 2026 11:10:55 -0700 Subject: [PATCH] improve message docs and add cli commands --- docs/design.md | 4 +- docs/operators.md | 6 +- docs/plan.md | 1 + docs/radial-json.md | 36 +++-- docs/running-an-agent.md | 83 +++++++++++- packages/core/src/index.ts | 1 + packages/core/src/thread.ts | 84 ++++++++++++ packages/sidecar/README.md | 23 ++++ packages/sidecar/src/cli.ts | 40 ++++++ packages/sidecar/src/index.ts | 1 + packages/sidecar/src/thread.ts | 96 ++++++++++++++ packages/sidecar/test/thread-list.test.mjs | 146 +++++++++++++++++++++ packages/ui/src/lib/replies.ts | 71 +--------- 13 files changed, 512 insertions(+), 80 deletions(-) create mode 100644 packages/core/src/thread.ts create mode 100644 packages/sidecar/src/thread.ts create mode 100644 packages/sidecar/test/thread-list.test.mjs diff --git a/docs/design.md b/docs/design.md index 94a7ebe..031b3f3 100644 --- a/docs/design.md +++ b/docs/design.md @@ -87,7 +87,7 @@ com.disnetdev.radial.message {goal | artifact, re?: strongref, — request this message responds to (§9) declines?: bool} — with re, from the assignee: declines it com.disnetdev.radial.agent {handleName, harness, models: [{name, costHint}], - artifactTypes: [string]} — registry names + "review" + artifactTypes: [string]} — registry names + "review"/"answer" ``` Notes: @@ -228,7 +228,7 @@ The invariant that keeps this from being alternative C (§14) is narrow and test ## 11. Agent operators -- An operator's daemon publishes one `com.disnetdev.radial.agent` record **per profile**, keyed by profile name in the agent's repo: harness, models, cost hints, and the artifact types it produces (registry names plus `review`). A single agent DID may publish many, so capability sets are added and retired by writing and deleting records rather than by provisioning accounts (§3). +- An operator's daemon publishes one `com.disnetdev.radial.agent` record **per profile**, keyed by profile name in the agent's repo: harness, models, cost hints, and the artifact types it produces (registry names plus the built-ins `review` and `answer`). A single agent DID may publish many, so capability sets are added and retired by writing and deleting records rather than by provisioning accounts (§3). - Spaces admit agents via membership like anyone else. Requests + profiles + claims determine who does what; there is no central assignment. - **Harness is per profile, and resolved fail-closed.** The daemon holds a registry of the harnesses it can run (Claude Code and pi today) and selects one per turn from the *acting* profile's `harness`, so a pi reviewer and a Claude implementer coexist in one daemon process. A profile naming a harness the daemon cannot run is refused — at `radiald init`, before the agent record is published, and again at `radiald run` startup — never silently downgraded to another one: the published record is what a space's assignee menu offers, and it has to be true. - **Routing is operator-side configuration.** The daemon config maps artifact types to model/provider choices — e.g. `plan: {provider: anthropic, model: opus}`, `review: {provider: openai, model: gpt-5}`, `implementation: {provider: anthropic, model: sonnet}`. An agent only accepts requests for types it has an assignment for. This keeps routing decisions (and their costs) with the party paying for tokens, and gets reviewer/implementer model diversity for free when an operator assigns different models to those types. diff --git a/docs/operators.md b/docs/operators.md index d105e7d..b72808c 100644 --- a/docs/operators.md +++ b/docs/operators.md @@ -74,7 +74,7 @@ by `radiald init`: "agents": { "planner": { "artifactTypes": ["plan", "adr", "architecture"] }, "implementer": { "artifactTypes": ["implementation"] }, - "reviewer": { "harness": "pi", "models": ["openai/gpt-5=high"], "artifactTypes": ["review"] } + "reviewer": { "harness": "pi", "models": ["openai/gpt-5=high"], "artifactTypes": ["review", "answer"] } } } ``` @@ -94,6 +94,10 @@ Three things follow from that shape: `radiald run` warns once per space about types no agent in the space accepts. Somebody else's daemon covering a type is a perfectly good answer; the warning is about types nobody covers. +- **`review` and `answer` are built-ins, and get no warning.** Neither is + registry data, so neither is counted by that coverage check — but a profile + still has to list the name to be offered the work. Drop `answer` and the UI's + "Ask an agent to reply" picker is simply empty in every space you serve. --- diff --git a/docs/plan.md b/docs/plan.md index c13f236..32cc895 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -130,6 +130,7 @@ The thread was one-directional. A human wrote into it, every turn read it, and t - **A reply is a message with `parent`** — the field both fold implementations have read since Phase 1 to decide a question has been answered, and which nothing in the product wrote. Until now the Awaiting-input list promised "answer in the thread and it can be picked up again" and answering in the thread did nothing at all: every request blocked on an agent's question was blocked forever. `message post` gains `--parent` (with `root` derived, never passed) and `--mention`; the question block and every thread row get a Reply action. - **An `answer` turn** (design §9, §10): a typed, human-authored `artifactRequest` naming one message version as its `subject`, whose terminal record is a reply in that thread rather than an artifact. Built into the daemon beside `review` — not registry data, never a unit, never a fraction of the goal's pie — with a read-only checkout, no forge token, and a deterministic rkey so a retried turn cannot double-post into a thread a human reads. +- **`radial message list`**, because both of those writes take a message's pinned `uri#cid` and the only place to read one was the browser. A read command like `type list`: it materializes the target's space and prints the thread — locator, author, time, flags, first line — one row per message, with `--unanswered` narrowing to the questions a request is parked on. The exchange itself is core's `threadRows`, the same fold the UI draws, so the terminal and the browser cannot disagree about what answers what. - **The invariant that keeps it out of §14's alternative C**: the daemon authors no answer request. A reply commissions nothing, so a conversation cannot sustain itself without a person writing a record for each turn of it. - Enabling it costs nothing on a live space and changes one thing that will surprise an operator: **every request parked in `awaiting_input` since the product shipped becomes dispatchable the moment somebody replies to it.** `radiald turn list --state awaiting_input` shows what is parked before you turn it on. - **Owed after it lands: an architecture (and glossary) request.** The system artifacts are on-protocol records, not files in this repo, so this phase cannot amend them — but it contradicts two of them. Architecture §10's boundary 7 still reads "The registry is data; only `review` is code" and names two documented exceptions to type-name branching; there are three now (`review`, `implementation`, `answer`). Its §8 table of what a container holds says nothing about the answer turn's no-token, read-only row. Glossary §6 (Turns) has no entry for the third turn type. Those artifacts ride in every bundle as the agents' long-term memory, so until the request lands the next turn reads a rule this phase broke. diff --git a/docs/radial-json.md b/docs/radial-json.md index 9ee44e5..d562ccb 100644 --- a/docs/radial-json.md +++ b/docs/radial-json.md @@ -45,7 +45,7 @@ credential. "implementer": { "artifactTypes": ["implementation"] }, "reviewer": { "models": ["claude-sonnet-4-5=high"], - "artifactTypes": ["review"] + "artifactTypes": ["review", "answer"] } }, "run": { @@ -75,7 +75,7 @@ which is what lets a single account offer distinct capability sets. | Field | Meaning | | --- | --- | -| `artifactTypes` | The type names this profile accepts. Required, and non-empty. See below — this is the field that has to agree with something outside this file. | +| `artifactTypes` | The type names this profile accepts: registry names, plus the built-ins `review` and `answer`. Required, and non-empty. See below — this is the field that has to agree with something outside this file. | | `identifier` | A second agent account for this profile. Its app password is read from `$RADIAL__PASSWORD` (e.g. `RADIAL_OTHER_EXAMPLE_PASSWORD`), not from `$RADIAL_PASSWORD`. | | `pds`, `harness`, `models` | Per-profile overrides of the top-level defaults. Overriding `harness` is what lets one identity run, say, a `pi` reviewer beside a `claude` implementer — see "Choosing a harness and a provider". | | `name` | The handle name published in the agent record. Defaults to the profile name. | @@ -154,7 +154,7 @@ identity: "reviewer": { "harness": "pi", "models": ["openai/gpt-5.2=high"], - "artifactTypes": ["review"] + "artifactTypes": ["review", "answer"] } } } @@ -366,10 +366,26 @@ names what moved: `--update` rewrites the record under the same key, so the profile keeps its identity and every request already assigned to it still resolves. -### `review` is not a registry type - -Auto-review (§7) writes requests with type `review`, and the daemon composes -that brief itself, so `review` does not need a registry record. A profile still -has to list `review` in its `artifactTypes` to answer one, which is why the -scaffold ships a `reviewer` profile. Because it is not in the registry, it never -appears in the coverage warning above. +### `review` and `answer` are not registry types + +Two type names are built-ins of the turn layer rather than registry data, and +`radial type create` refuses both: + +- **`review`** — auto-review (§7) writes the request and the daemon composes the + brief itself, so no registry record is needed. Its terminal record is a verdict + on the named subject. +- **`answer`** — a human-authored request naming one *message* version as its + `subject`, asking an agent to reply to it in that goal's thread (§9, §10). Its + terminal record is a `message`, not an artifact. The request must be + goal-scoped and must name an assignee; the daemon never authors one. + +A profile still has to list a name in its `artifactTypes` to be offered that +work, which is why the scaffold ships a `reviewer` profile carrying both. They +ride together because they are the same register of work — reading what is +already there and saying something about it — and because neither produces an +artifact or a row in a goal's unit list; splitting them across profiles is a +routing decision, not a structural one. + +Because neither is in the registry, neither ever appears in the coverage warning +above: a space with nobody publishing `answer` gets no startup warning and no +error, just an empty picker behind the UI's "Ask an agent to reply". diff --git a/docs/running-an-agent.md b/docs/running-an-agent.md index 6e21965..b7600ad 100644 --- a/docs/running-an-agent.md +++ b/docs/running-an-agent.md @@ -48,7 +48,9 @@ radiald config init # or: radiald config init agent.example It writes three profiles covering the built-in artifact types — `planner` (which also carries the system types `space create` seeds: `architecture`, `architecture-inventory`, `glossary`, `conventions`, and `adr`), `implementer`, -and `reviewer` — all on one identity. +and `reviewer` — all on one identity. The reviewer also carries `answer`, the +built-in that replies to a message in a goal's thread ("Answering in a thread" +below). Edit it, then register them (below); [`radial-json.md`](radial-json.md) documents every field. @@ -88,7 +90,7 @@ name, so one identity can offer several distinct capability sets: "reviewer": { "harness": "pi", "models": ["openai/gpt-5.2=high"], - "artifactTypes": ["review"] + "artifactTypes": ["review", "answer"] } }, "run": { @@ -117,6 +119,11 @@ profile publishes leaves its requests sitting open with nobody to assign. is the field-by-field reference, including how to add a type to a profile that is already published. +`review` and `answer` are the two exceptions: they are built-ins of the turn +layer rather than registry data — `radial type create` refuses both names, and +neither ever becomes a unit of a goal. A profile still has to list them to be +offered one, which is why the scaffold puts both on the reviewer. + The PDS is resolved from the identity itself — a handle through its `_atproto` DNS TXT record (falling back to `https:///.well-known/atproto-did`), then the DID document's `AtprotoPersonalDataServer` endpoint. Set `pds` @@ -131,7 +138,7 @@ printf '%s\n' "$AGENT_APP_PASSWORD" | radiald init --password-stdin ✓ planner did:plc:… plan, architecture, architecture-inventory, glossary, conventions, adr ✓ implementer did:plc:… implementation -✓ reviewer did:plc:… review +✓ reviewer did:plc:… review, answer radiald init planner # or just the named ones ``` @@ -225,6 +232,76 @@ awaiting_input` for the requests parked on a question); if a request exhausts its retry limit or needs to be retried immediately, run `radiald turn reset `. +## Answering in a thread + +A turn that needs input its bundle cannot answer posts its question as a message +and exits; the request parks in `awaiting_input`. What un-parks it is a **reply** +— a message carrying `parent`, from somebody other than the asker. Prose in the +thread with no `parent` does nothing, because the fold reads the field rather +than the paragraph: + +```sh +# the thread, oldest first: locator, author, time, flags, first line of the body +radial message list --profile human --goal "$GOAL" + +# or straight to the one blocking a request +QUESTION=$(radial message list --profile human --goal "$GOAL" --unanswered | cut -f1) + +radial message post --profile human --goal "$GOAL" \ + --parent "$QUESTION" --body 'Use the staging database; the fixtures are in test/.' +``` + +The request is dispatchable again on the next poll, with the richer thread as +context. `radiald turn list --state awaiting_input` is the operator's side of the +same list — the requests this daemon has parked — and `message list --unanswered` +is the space's, which is the one that holds the locator you reply to. Enabling +this on a space that has been running a while is worth doing deliberately: every +request parked since the space was created becomes dispatchable the moment +somebody replies to it. + +An agent can also be asked to write the reply. That is an **`answer` request**: a +typed `artifactRequest` naming one message version as its `subject`, whose +terminal record is a reply in that thread rather than an artifact. Configuring +for it is one name in a profile — + +```sh +# 1. in radial.json: "reviewer": { "artifactTypes": ["review", "answer"] } +# 2. republish, because the agent record is what the "ask an agent" picker reads +radiald init reviewer --update +✓ reviewer did:plc:… review, answer (republished) +``` + +— and without a published profile that picker is empty, the same way an +uncovered registry type leaves a request sitting open. `answer` is not registry +data, so it never appears in the startup coverage warning; nothing tells you it +is missing except an empty picker. + +The UI writes both records from "Ask an agent to reply". By hand it is a request +like any other, with three rules the write path enforces: it must be goal-scoped, +its `--subject` must be a message (not an artifact), and in v1 it must name an +assignee — the fold closes it on *that* agent's reply threaded under the subject, +so an unassigned one is a request nothing could close. + +```sh +radial request create --profile human --goal "$GOAL" \ + --type answer --subject "$QUESTION" --assignee agent.example \ + --brief 'Answer the question about the migration order.' +``` + +An answer turn runs in the same container shape as any other and differs only in +what it may write: a read-only checkout of the project's default branch, **no +forge token**, and a reply as its one terminal record. It is never a unit and +never a fraction of a goal's pie. Its record key is derived from the request, so +a retried turn adopts its own reply instead of posting a second one into a thread +a human is reading. Answer turns share `run.concurrency` with everything else and +are logged distinctly (`turn (answer)`). + +The daemon **never authors an `answer` request**. Auto-review is the one +daemon-authored hop in the system, so an agent's reply commissions nothing: a +mention dispatches nothing, no message body is parsed to decide what to run, and +a conversation cannot sustain itself without a person writing a record for each +turn of it (design §10, and §14's rejected alternative C). + ## One PR per version chain Asking for another version of an implementation — which is how a review's diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c5963ac..487e24b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -4,6 +4,7 @@ export * from './materializer.js' export * from './order.js' export * from './records.js' export * from './store.js' +export * from './thread.js' export * from './timeline.js' export * from './turn-protocol.js' export * from './validation.js' diff --git a/packages/core/src/thread.ts b/packages/core/src/thread.ts new file mode 100644 index 0000000..fc012cc --- /dev/null +++ b/packages/core/src/thread.ts @@ -0,0 +1,84 @@ +// The thread fold. +// +// `materialize()` gives a target's messages as one flat, trusted list. A conversation is not a flat +// list — it is things said and things said back — and `parent` is the field that says which is +// which. Turning the one into the other is this file. +// +// It lives in core for the same reason `timeline()` does: two consumers read the same thread (a +// browser tab drawing it, `radial message list` printing it), and "what answers what" must not come +// to mean two different things in two places. It is a pure function over a view's messages, so both +// compute the identical exchange. + +import type { MessageRecord } from './generated/records.js' +import type { IndexedRecord } from './materializer.js' + +export type ThreadMessage = IndexedRecord + +const byCreatedAt = (left: ThreadMessage, right: ThreadMessage): number => + left.value.createdAt.localeCompare(right.value.createdAt) || left.uri.localeCompare(right.uri) + +export interface ThreadReply { + message: ThreadMessage + /** + * Set when this reply answers another REPLY rather than the row's own message. The thread nests one + * level and no further (a conversation is not a tree a person can read), so a deeper chain is + * flattened onto the row and says who it was actually answering. + */ + inReplyTo?: ThreadMessage +} + +export interface ThreadRow { + message: ThreadMessage + replies: ThreadReply[] +} + +/** + * The thread as an exchange rather than a list: top-level messages, each carrying its replies. + * + * A message whose `parent` names something this view does not hold (a reply to a message on another + * target, say) is shown as a top-level row rather than dropped — the view's job is to show what was + * said, and hiding a message because its parent is elsewhere would be the reader's loss. + * + * Structurally typed on `messages` alone so a `GoalView` and a `ProjectView` both pass: a thread is + * the messages filed under a target, and nothing here needs the rest of the view. + */ +export function threadRows(view: { messages: ThreadMessage[] }): ThreadRow[] { + const byUri = new Map(view.messages.map((message) => [message.uri, message])) + const rows = new Map() + const ordered = [...view.messages].sort(byCreatedAt) + + /** The top-level ancestor of a message within this view; itself when it has none. Cycle-safe. */ + const ancestorOf = (message: ThreadMessage): ThreadMessage => { + const seen = new Set([message.uri]) + let cursor = message + while (cursor.value.parent) { + const parent = byUri.get(cursor.value.parent.uri) + if (!parent || seen.has(parent.uri)) break + seen.add(parent.uri) + cursor = parent + } + return cursor + } + + for (const message of ordered) { + const ancestor = ancestorOf(message) + if (ancestor.uri === message.uri) { + rows.set(message.uri, { message, replies: [] }) + continue + } + const row = rows.get(ancestor.uri) + if (!row) { + // The ancestor sorts after this reply only if timestamps disagree with threading; keep the + // reply visible as its own row rather than losing it. + rows.set(message.uri, { message, replies: [] }) + continue + } + const parent = message.value.parent ? byUri.get(message.value.parent.uri) : undefined + row.replies.push({ + message, + ...(parent && parent.uri !== row.message.uri ? { inReplyTo: parent } : {}), + }) + } + + return [...rows.values()].sort((left, right) => byCreatedAt(left.message, right.message)) +} diff --git a/packages/sidecar/README.md b/packages/sidecar/README.md index 3c10077..1403799 100644 --- a/packages/sidecar/README.md +++ b/packages/sidecar/README.md @@ -67,6 +67,29 @@ HANDLE|DID` (repeatable) records who it addresses, as DIDs. Replying to an agent's question is what clears the request it blocked — the fold only counts a reply from somebody other than the asker. +`message list` is how you find the `REF` to pass it. Like `type list` it is a +read: it materializes the target's space and prints the thread, oldest first, one +tab-separated row per message — pinned `at://…#CID` locator, author DID, +timestamp, flags, and the first line of the body. A goal's thread includes +messages anchored to its artifacts, which is where the fold files them: + +```sh +radial message list --profile alice --goal "$GOAL" +at://…/com.disnetdev.radial.message/3lbq…#bafy… did:plc:reviewer 2026-07-21T10:02:30Z - v2 resolves the strongref properly… +at://…/com.disnetdev.radial.message/3lbq…#bafy… did:plc:tim 2026-07-21T10:11:00Z reply Reads the index twice how? If it is… + +QUESTION=$(radial message list --profile alice --goal "$GOAL" --unanswered | cut -f1) +radial message post --profile alice --goal "$GOAL" --parent "$QUESTION" --body 'The openRequests scan.' +``` + +The flags say what a row is: `reply` (it has a `parent`), `question` (it carries +`re` and is not itself a reply — what a turn wrote back to its requester), +`unanswered` (…and the request it names is parked on it), `declined`. +`--unanswered` keeps only the last kind, which makes the locator of the thing +blocking a request a one-liner. `--json` prints the same fields, bodies in full, +with `depth` and `inReplyTo` for the nesting. `--project REF` reads a project's +own thread instead. + Inside a turn container the allowlist is narrower and the daemon anchors every record it writes: `artifact submit`, `review submit`, `answer submit` (an answer turn's reply in the goal thread) and `message post` (the can't-complete diff --git a/packages/sidecar/src/cli.ts b/packages/sidecar/src/cli.ts index b4f8a35..521f96a 100644 --- a/packages/sidecar/src/cli.ts +++ b/packages/sidecar/src/cli.ts @@ -17,6 +17,7 @@ import { COLLECTIONS, MemoryRecordStore } from '@radial/core' import { MemorySyncStateStore, RepoPoller, SpaceIngestor } from '@radial/ingest' import { runCli } from './commands.js' import { buildTurnRpc, sendTurnRpc } from './socket.js' +import { formatThreadLine, threadListing } from './thread.js' export const help = `radial — human CLI for Radial records @@ -47,6 +48,10 @@ export const help = `radial — human CLI for Radial records --parent makes the message a REPLY to another message on the same target; the thread root is derived from it. Replying to an agent's question is what unblocks a request that is awaiting input. + radial message list (--goal REF | --project REF) [--unanswered] + the target's thread, oldest first, each message with the pinned locator + --parent and --subject take. A goal's thread includes messages anchored to + its artifacts. --unanswered narrows it to the questions blocking a request. radial review post --subject REF --verdict approve|request_changes [--request REF] [--findings-file PATH | --findings-json JSON] @@ -148,6 +153,41 @@ export async function main(args = argv.slice(2)): Promise { return } + // `message list` is the other read command, and it is what makes the reply commands usable: every + // write that threads a message (`message post --parent`, `request create --type answer --subject`) + // takes a message's pinned uri#cid, and until this existed the only way to learn one outside the UI + // was to open the daemon's record store with a sqlite client. Same shape as `type list` — resolve + // the ref, materialize the space, print — because a thread is a fold over the space, not a query. + if (args[0] === 'message' && args[1] === 'list') { + const goalLocator = value(args, '--goal') + const projectLocator = value(args, '--project') + if (Boolean(goalLocator) === Boolean(projectLocator)) { + throw new Error('message list requires exactly one of --goal or --project') + } + const target = await resolver.resolve( + (goalLocator ?? projectLocator) as string, + goalLocator ? COLLECTIONS.goal : COLLECTIONS.project, + ) + // Both records name the space they belong to, so resolving the target is the whole lookup. + const spaceUri = (target.value as { space: { uri: string } }).space.uri + const records = new MemoryRecordStore() + const index = await new SpaceIngestor( + spaceUri, + new RepoPoller(transport, records, new MemorySyncStateStore()), + records, + ).sync() + const view = [...index.goals, ...index.projects].find((candidate) => candidate.target.uri === target.uri) + // A target the fold does not hold is a real answer, not an empty thread: it was written by + // somebody the space does not trust, or archived out of the live view. Say so rather than print + // nothing, which a caller would read as "no messages yet". + if (!view) throw new Error(`${target.uri} is not in the trusted view of ${spaceUri}`) + const unansweredOnly = args.includes('--unanswered') + const lines = threadListing(view).filter((line) => !unansweredOnly || line.unanswered) + if (args.includes('--json')) console.log(JSON.stringify(lines)) + else for (const line of lines) console.log(formatThreadLine(line)) + return + } + const result = await runCli(args, { writer: client, resolver, diff --git a/packages/sidecar/src/index.ts b/packages/sidecar/src/index.ts index e9cd08e..9a68a8a 100644 --- a/packages/sidecar/src/index.ts +++ b/packages/sidecar/src/index.ts @@ -2,3 +2,4 @@ // process. The browser drives the exact same record-construction code the CLI and daemon drive by // handing `runCli` an OAuth-backed writer. The turn socket (`node:net`) lives in `./node`. export * from './commands.js' +export * from './thread.js' diff --git a/packages/sidecar/src/thread.ts b/packages/sidecar/src/thread.ts new file mode 100644 index 0000000..0d795ac --- /dev/null +++ b/packages/sidecar/src/thread.ts @@ -0,0 +1,96 @@ +// Reading a thread back, for a terminal. +// +// Every other read path in the product is the UI. That was survivable while a thread was something +// humans wrote and agents only read — but `message post --parent` and `request create --type answer` +// both take a message's pinned `uri#cid`, and until now the only way to learn one from the CLI was +// to open the daemon's sqlite store and eyeball `value_json`. A command that writes a reply is not a +// CLI surface if finding what to reply to needs a database client. +// +// The exchange itself is core's `threadRows` — the same fold the UI draws — flattened back to one +// line per message so the output stays greppable and `cut -f1`-able. Structure survives the +// flattening in the ORDER (a row, then its replies) and in the `reply` flag, not in indentation: +// indenting column one would put whitespace in front of the locator, which is the one column a +// script actually wants. + +import { threadRows, type ThreadMessage } from '@radial/core' +import type { GoalView, ProjectView } from '@radial/core' + +/** What `message list` knows about one message. `--json` prints these verbatim. */ +export interface ThreadLine { + /** The pinned `at://…#cid` locator `--parent` and `--subject` take. */ + locator: string + /** The author. DIDs, not handles: a record stores the DID, which outlives any handle. */ + did: string + createdAt: string + body: string + /** 0 for a top-level message, 1 for a reply. Core's fold nests one level and no further. */ + depth: number + /** When this reply answers another reply rather than the row it is filed under. */ + inReplyTo?: string + /** The request this message is about, when it carries `re`. */ + re?: string + /** A message carrying `re` that is not itself a reply: what a turn wrote back to its requester. */ + question: boolean + /** …and nobody has answered it, so the request it names is parked. This is the one to reply to. */ + unanswered: boolean + /** The turn refused the request outright rather than asking about it. */ + declined: boolean +} + +/** + * One line per message in the target's thread, top-level messages in time order and each one's + * replies behind it. + * + * `unanswered` is read off the view's own `awaitingInput` rather than recomputed here: that set is + * the fold's answer to "which requests are blocked on a person", and a second implementation of the + * rule in a CLI is exactly how a command comes to disagree with the daemon it is meant to explain. A + * question is attributed to its request, so two questions on one parked request both read + * `unanswered` — the same approximation the UI's thread makes, and it errs toward showing a reply + * target rather than hiding one. + */ +export function threadListing(view: GoalView | ProjectView): ThreadLine[] { + const awaiting = new Set(view.awaitingInput) + const line = (message: ThreadMessage, depth: number, inReplyTo?: ThreadMessage): ThreadLine => { + const re = message.value.re?.uri + const question = Boolean(re) && message.value.parent === undefined + return { + locator: `${message.uri}#${message.cid}`, + did: message.did, + createdAt: message.value.createdAt, + body: message.value.body, + depth, + ...(inReplyTo ? { inReplyTo: `${inReplyTo.uri}#${inReplyTo.cid}` } : {}), + ...(re ? { re } : {}), + question, + unanswered: question && message.value.declines !== true && awaiting.has(re as string), + declined: message.value.declines === true, + } + } + return threadRows(view).flatMap((row) => [ + line(row.message, 0), + ...row.replies.map((reply) => line(reply.message, 1, reply.inReplyTo)), + ]) +} + +/** The flag column: what this message is, in the words the docs and the UI use for it. */ +export function threadFlags(line: ThreadLine): string { + const flags = [ + ...(line.depth > 0 ? ['reply'] : []), + ...(line.question ? ['question'] : []), + ...(line.unanswered ? ['unanswered'] : []), + ...(line.declined ? ['declined'] : []), + ] + return flags.length > 0 ? flags.join(',') : '-' +} + +/** + * One tab-separated row: locator, author, time, flags, and the first line of the body. + * + * Truncated the way `type list` truncates a brief — enough to recognise which message this is, on the + * understanding that reading it in full is what the UI and `--json` are for. + */ +export function formatThreadLine(line: ThreadLine, width = 80): string { + const first = line.body.split('\n')[0] ?? '' + const summary = first.length > width ? `${first.slice(0, width - 1)}…` : first + return [line.locator, line.did, line.createdAt, threadFlags(line), summary].join('\t') +} diff --git a/packages/sidecar/test/thread-list.test.mjs b/packages/sidecar/test/thread-list.test.mjs new file mode 100644 index 0000000..2f1ca1f --- /dev/null +++ b/packages/sidecar/test/thread-list.test.mjs @@ -0,0 +1,146 @@ +import assert from 'node:assert/strict' +import { describe, it } from 'node:test' +import { + CredentialClient, + FetchRepoTransport, + StrongRefResolver, + createSession, +} from '../../atproto/dist/index.js' +import { LocalPds } from '../../atproto/test/local-pds.mjs' +import { FIXTURE_DIDS, fixtureSpace } from '../../core/dist/fixture.js' +import { COLLECTIONS, MemoryRecordStore, materialize } from '../../core/dist/index.js' +import { MemorySyncStateStore, RepoPoller, SpaceIngestor } from '../../ingest/dist/index.js' +import { formatThreadLine, runCli, threadFlags, threadListing } from '../dist/index.js' + +const fixture = fixtureSpace() +const named = Object.fromEntries(Object.entries(FIXTURE_DIDS).map(([name, did]) => [did, name])) +const store = new MemoryRecordStore() +fixture.records.forEach((record) => store.put(record)) +const index = materialize(store, { spaceUri: fixture.spaceUri, asOf: fixture.asOf }) +const goal = (uri) => index.goals.find((view) => view.target.uri === uri) + +describe('message list', () => { + it('flattens the exchange the UI draws: a row, then its replies, in that order', () => { + // The review queue's thread is one conversation in four messages: a reviewer's note, a human's + // question threaded under it, and the agent's answer threaded under THAT — a second-level reply, + // which the fold flattens onto the row and marks with the message it was actually answering. + const lines = threadListing(goal(fixture.goals.reviewQueue)) + assert.deepEqual( + lines.map((line) => [line.depth, named[line.did], threadFlags(line)]), + [ + [0, 'tim', '-'], + [0, 'reviewer', '-'], + [1, 'tim', 'reply'], + [1, 'reviewer', 'reply'], + ], + ) + // The reply order is the thread's, not the flat list's: the agent's answer follows the question + // it answers even though a later top-level message would sort between them. + const answer = lines.at(-1) + assert.equal(answer.inReplyTo, lines[2].locator) + // It carries `re` — an answer turn's reply closes its request — but it is not a question: the + // `parent` half is what tells the two apart, and getting that wrong is what would make a turn + // look like it had asked something nobody answered. + assert.match(answer.re, /artifactRequest/) + assert.equal(answer.question, false) + assert.equal(answer.unanswered, false) + }) + + it('marks the question a request is parked on, and --unanswered keeps only those', () => { + const view = goal(fixture.goals.capture) + const lines = threadListing(view) + const parked = lines.filter((line) => line.unanswered) + assert.equal(parked.length, 1) + const [question] = parked + assert.equal(question.question, true) + assert.equal(question.depth, 0) + assert.equal(question.declined, false) + // The flag is read off the fold's own awaitingInput rather than recomputed, so the command and + // the daemon cannot disagree about which request is blocked. + assert.ok(view.awaitingInput.includes(question.re)) + // And the locator is the pinned uri#cid the reply commands take, not a bare URI. + assert.match(question.locator, /^at:\/\/.+\/com\.disnetdev\.radial\.message\/.+#.+/) + }) + + it('prints one greppable row per message: locator first, body last', () => { + const [line] = threadListing(goal(fixture.goals.reviewQueue)) + const columns = formatThreadLine(line).split('\t') + assert.equal(columns.length, 5) + assert.equal(columns[0], line.locator) + assert.equal(columns[1], line.did) + assert.equal(columns[2], line.createdAt) + assert.equal(columns[3], '-') + // A body is truncated to the width, never wrapped: a message spanning lines would otherwise emit + // rows with no locator in column one, which is the column a script reads. + const long = { ...line, body: `${'x'.repeat(200)}\nsecond line` } + const summary = formatThreadLine(long).split('\t')[4] + assert.equal(summary.length, 80) + assert.ok(summary.endsWith('…')) + assert.equal(formatThreadLine(long).includes('\n'), false) + }) +}) + +describe('message list, over records the CLI itself wrote', () => { + it('finds the space from the target alone and threads a reply under its parent', async () => { + const did = 'did:plc:threadalice' + const pds = new LocalPds(did) + const fetcher = (input, init) => pds.fetch(input, init) + const transport = new FetchRepoTransport(async () => pds.service, fetcher) + const resolver = new StrongRefResolver(transport) + const writer = new CredentialClient(await createSession(pds.service, pds.handle, 'password', fetcher), fetcher) + let minute = 0 + const at = () => ({ + writer, + resolver, + now: () => `2026-07-26T00:${String((minute += 1)).padStart(2, '0')}:00Z`, + readText: async () => { + throw new Error('unexpected file read') + }, + }) + const locator = (result) => `${result.primary.uri}#${result.primary.cid}` + + const space = await runCli(['space', 'create', '--name', 'Threads', '--description', 'reading one'], at()) + const project = await runCli( + ['project', 'create', '--space', locator(space), '--name', 'radial', '--git-url', 'https://example.test/r.git'], + at(), + ) + const goal = await runCli( + ['goal', 'create', '--project', locator(project), '--title', 'Read a thread', '--body', 'from a terminal'], + at(), + ) + const first = await runCli( + ['message', 'post', '--goal', locator(goal), '--body', 'Which database does the migration target?'], + at(), + ) + await runCli( + ['message', 'post', '--goal', locator(goal), '--parent', locator(first), '--body', 'Staging.'], + at(), + ) + + // The command's whole lookup: resolve the target, read the space off it. Both goal and project + // records carry it, which is why one resolve is enough — assert that rather than assume it. + const target = await resolver.resolve(goal.primary.uri, COLLECTIONS.goal) + assert.equal(target.value.space.uri, space.primary.uri) + + const records = new MemoryRecordStore() + const index = await new SpaceIngestor( + target.value.space.uri, + new RepoPoller(transport, records, new MemorySyncStateStore()), + records, + ).sync() + const view = [...index.goals, ...index.projects].find((candidate) => candidate.target.uri === target.uri) + const lines = threadListing(view) + assert.deepEqual( + lines.map((line) => [line.depth, line.body]), + [ + [0, 'Which database does the migration target?'], + [1, 'Staging.'], + ], + ) + // The locator this prints is exactly what `--parent` took, fragment and all — the round trip the + // command exists for. + assert.equal(lines[0].locator, locator(first)) + // Nothing here is a question: neither message carries `re`, so nothing reads as blocked. + assert.deepEqual(lines.filter((line) => line.unanswered), []) + }) +}) diff --git a/packages/ui/src/lib/replies.ts b/packages/ui/src/lib/replies.ts index 30e8e8c..51c57db 100644 --- a/packages/ui/src/lib/replies.ts +++ b/packages/ui/src/lib/replies.ts @@ -23,8 +23,8 @@ import type { GoalView, IndexedRecord, MaterializedIndex, - MessageRecord, StrongRef, + ThreadMessage, } from '@radial/core' import type { Actor, Directory } from './directory.js' import { locator } from './requests.js' @@ -35,10 +35,7 @@ import { isGoalView, liveTargets, type AnswerContext } from './units.js' * reason: an admin must not be able to redefine what replying to somebody means. */ export const ANSWER_TYPE = 'answer' -export type ThreadMessage = IndexedRecord - -const byCreatedAt = (left: ThreadMessage, right: ThreadMessage): number => - left.value.createdAt.localeCompare(right.value.createdAt) || left.uri.localeCompare(right.uri) +export type { ThreadMessage } from '@radial/core' // ── writing one ───────────────────────────────────────────────────────────────────────────────── @@ -247,65 +244,11 @@ export function parkedAnswers(index: MaterializedIndex): AnswerContext[] { ) } -export interface ThreadReply { - message: ThreadMessage - /** - * Set when this reply answers another REPLY rather than the row's own message. The thread nests one - * level and no further (a conversation is not a tree a person can read), so a deeper chain is - * flattened onto the row and says who it was actually answering. - */ - inReplyTo?: ThreadMessage -} - -export interface ThreadRow { - message: ThreadMessage - replies: ThreadReply[] -} - /** - * The thread as an exchange rather than a list: top-level messages, each carrying its replies. + * The thread as an exchange rather than a list — top-level messages, each carrying its replies. * - * A message whose `parent` names something this view does not hold (a reply to a message on another - * target, say) is shown as a top-level row rather than dropped — the view's job is to show what was - * said, and hiding a message because its parent is elsewhere would be the reader's loss. + * The fold itself is in core, because `radial message list` prints the same exchange this draws and + * "what answers what" must not be two functions. Re-exported here so the components that read a + * thread keep reading it from one module. */ -export function threadRows(view: GoalView): ThreadRow[] { - const byUri = new Map(view.messages.map((message) => [message.uri, message])) - const rows = new Map() - const ordered = [...view.messages].sort(byCreatedAt) - - /** The top-level ancestor of a message within this view; itself when it has none. Cycle-safe. */ - const ancestorOf = (message: ThreadMessage): ThreadMessage => { - const seen = new Set([message.uri]) - let cursor = message - while (cursor.value.parent) { - const parent = byUri.get(cursor.value.parent.uri) - if (!parent || seen.has(parent.uri)) break - seen.add(parent.uri) - cursor = parent - } - return cursor - } - - for (const message of ordered) { - const ancestor = ancestorOf(message) - if (ancestor.uri === message.uri) { - rows.set(message.uri, { message, replies: [] }) - continue - } - const row = rows.get(ancestor.uri) - if (!row) { - // The ancestor sorts after this reply only if timestamps disagree with threading; keep the - // reply visible as its own row rather than losing it. - rows.set(message.uri, { message, replies: [] }) - continue - } - const parent = message.value.parent ? byUri.get(message.value.parent.uri) : undefined - row.replies.push({ - message, - ...(parent && parent.uri !== row.message.uri ? { inReplyTo: parent } : {}), - }) - } - - return [...rows.values()].sort((left, right) => byCreatedAt(left.message, right.message)) -} +export { threadRows, type ThreadReply, type ThreadRow } from '@radial/core' -- 2.51.2