diff --git a/CHANGELOG.md b/CHANGELOG.md index cfbd693..28870f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ ## Unreleased +- **Artifacts have a short title, and rows show it.** `com.disnetdev.radial.artifact` gains an + optional `title` (at most 200 characters): the phrase that names the record wherever it is listed + rather than read. The System page used to derive that line from the first suitable paragraph of the + body, which made an architecture document or an ADR look as though a long opening sentence were its + name and made a list of them hard to scan. Now the type stays the row's category and the current + version's title is the line beside it. What an operator should know: + + - **The field is optional on the wire, and required of every writer.** A signed record cannot be + given a title after the fact, so making it required would make every existing artifact fail + validation — and a materializer ignores what fails validation, which would read the requests they + answered as open and dispatch them again. `radial artifact post` and the daemon's turn socket both + require `--title`, trim it, and refuse a blank or over-long one. Old records stay valid and keep + showing the body-derived line; that fallback is display-only and is never written back. + - **`radial artifact post --title TITLE` is required, and so is `radial artifact submit --title` + inside a turn.** The agent prompts ask for one in all three artifact-turn variants (plan-style, + GitHub implementation, tangled implementation). A submit without one is refused with a message + saying so, and nothing is written — the turn can submit again. + - **Titles ride in the turn bundle.** `bundle.json`'s artifacts carry `title`, and `bundle.md` names + each `basedOn` and system artifact by it, so a turn can tell two ADRs apart without reading every + body it was handed. A bundle built from untitled records is unchanged. + - **Duplicate-submit safety includes the title.** A retried turn adopts the record already at its + deterministic rkey only when the title matches along with the body, criteria, links and anchors; + a title-only difference is refused rather than silently adopted. + - Browser OAuth scope is unaffected: the collections the app writes have not changed. + - **A goal has one ending, and it is the additive `closeGoal` overlay.** Any active member can end a goal as `completed`, `dropped`, `superseded` or `parked`, and can reopen it — including a goal whose author is unavailable, which the old author-only closure made impossible. The two controls the goal diff --git a/DESIGN.md b/DESIGN.md index 522e668..20f6c28 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -362,6 +362,7 @@ fill *is* the state (a pressed version button, an "on" auto-review switch). ### Rows - **Unit row:** state circle, type name, an optional version chip, a one-line summary, then a tail of badges, the actor's disc and an age. Rows are separated by a soft hairline drawn by the *next* row, so the first has none and an open row's neighbours drop theirs. +- **That one line, and when it is ink rather than ink 2:** an artifact carries its own short title, and where the line IS that title it is the record's name and reads in **ink**. Everything else in that slot is metadata and stays **ink 2**: a word about what an agent is doing, and — for a record written before artifacts had titles — the sentence lifted out of its body. It ellipsises either way; a title is capped at 200 characters and a row is narrower than that. - **Open state:** the row keeps its place and the drawer below it animates open (`grid-template-rows: 0fr → 1fr`, 400ms); the whole unit lifts onto `raised` with the drawer shadow. Nothing navigates. - **Admin row:** the same object five times over — a name, what the space knows about it, and at most one thing you can do to it — used for members, registry entries, projects, checks and switches. diff --git a/docs/design.md b/docs/design.md index 59a0d2f..6ccb3fd 100644 --- a/docs/design.md +++ b/docs/design.md @@ -78,6 +78,9 @@ com.disnetdev.radial.artifactRequest{goal | project, com.disnetdev.radial.claim {request, expiresAt} — open requests only; lease, renewable com.disnetdev.radial.artifact {request: strongref, goal | project, type, prev?: strongref, — supersedes an earlier version + title?, — short name a row shows; optional on + the wire (pre-title records stay + valid), required of every writer body, links: {branch?, commit?, pr?}, — implementation-type artifacts criteria?: [string]} — plan-type artifacts; human-judged diff --git a/docs/phase6-ui-plan.md b/docs/phase6-ui-plan.md index b3d2ee4..08c9ecf 100644 --- a/docs/phase6-ui-plan.md +++ b/docs/phase6-ui-plan.md @@ -270,10 +270,13 @@ Four things the drawing could not settle, which drawing it from records did: carries a `basedOn` too; matching on the ref alone labelled every implementation "distilled from plan". A capture is a request under a *project* whose `basedOn` names a *goal* artifact, and both directions are resolved that way. -- **An artifact has no title.** The comp gives every system row one; the lexicon has only `body`, so - a row's line is derived (`summarize()`: first paragraph that is neither a heading nor an ADR's - `Status:` line, cut at a sentence). It reads well and it is honest, but if System rows should show - a title, that is a lexicon change and it belongs in a step, not in a renderer. +- **An artifact had no title.** The comp gives every system row one; at 6.1 the lexicon had only + `body`, so a row's line was derived (`summarize()`: first paragraph that is neither a heading nor an + ADR's `Status:` line, cut at a sentence). It read well and it was honest, but scanning a list of + ADRs by opening sentence is not scanning, so the lexicon change it asked for was made: `artifact` + now carries an optional short `title`, every writer requires one, and `artifactLabel()` in + `format.ts` is the one place that shows it — with `summarize()` kept behind it as the display-only + fallback for the immutable records written before the field existed. - **A human's handle is not in the index.** Membership carries a DID and a kind; only agents publish a handle. Humans render as a short DID until 6.2 resolves handles — in fixture mode a two-entry map next to the fixture wiring supplies the comp's two, and nothing else in the app may hardcode one. diff --git a/packages/core/src/bundle.ts b/packages/core/src/bundle.ts index c048c32..974d750 100644 --- a/packages/core/src/bundle.ts +++ b/packages/core/src/bundle.ts @@ -23,6 +23,10 @@ export interface BundleArtifact { uri: string cid: string type: string + /** The artifact's own short title, so a turn can tell two `adr` records apart — or find the one + * system document it needs to revise — without reading every body in the bundle. Absent for a + * record written before the field existed; a bundle built from one is otherwise unchanged. */ + title?: string body: string criteria?: string[] links: ArtifactLinks @@ -78,6 +82,7 @@ function toBundleArtifact(artifact: IndexedRecord): BundleArtifa uri: artifact.uri, cid: artifact.cid, type: artifact.value.type, + ...(artifact.value.title !== undefined ? { title: artifact.value.title } : {}), body: artifact.value.body, ...(artifact.value.criteria !== undefined ? { criteria: artifact.value.criteria } : {}), links: artifact.value.links, diff --git a/packages/core/src/fixture.ts b/packages/core/src/fixture.ts index 7322257..dda940b 100644 --- a/packages/core/src/fixture.ts +++ b/packages/core/src/fixture.ts @@ -346,6 +346,13 @@ export function fixtureSpace(): FixtureSpace { request: StoredRecord anchor: { goal: StrongRef } | { project: StrongRef } type: string + /** + * The record's own short title — what every row and chip calls it. Optional here for one reason: + * the field is optional in the lexicon because records signed before it existed cannot be given + * one, and the demo carries a couple of those on purpose (ADR 0004, and the ignored-records plan) + * so the fallback a real space will show for years is on screen rather than only in a test. + */ + title?: string body: string at: string prev?: StoredRecord @@ -359,6 +366,7 @@ export function fixtureSpace(): FixtureSpace { ...input.anchor, type: input.type, ...(input.prev ? { prev: ref(input.prev) } : {}), + ...(input.title ? { title: input.title } : {}), body: input.body, links: input.links ?? {}, ...(input.criteria ? { criteria: input.criteria } : {}), @@ -470,6 +478,7 @@ export function fixtureSpace(): FixtureSpace { request: u1r1, anchor: g1Ref, type: 'plan', + title: 'Queue route and verdict form', at: '2026-07-21T09:31:00Z', body: '## Approach\n\nAdd a `/queue` route that filters the materialized index for open requests of type `review` whose assignee is the signed-in DID. Render each as a card with the subject artifact inlined, and a two-button verdict form.\n\nThe verdict form posts straight to the PDS from the browser session.', @@ -513,6 +522,10 @@ export function fixtureSpace(): FixtureSpace { request: u1r2, anchor: g1Ref, type: 'plan', + // A v2 addressing findings is usually the same document, so it keeps the same title: the version + // rail says which version is being read, and renaming a plan because it was revised would make + // the chain look like two different plans. + title: 'Queue route and verdict form', prev: u1v1, at: '2026-07-21T09:58:00Z', body: @@ -679,6 +692,7 @@ export function fixtureSpace(): FixtureSpace { request: u5r, anchor: g3Ref, type: 'plan', + title: 'Drop the egress allowlist for possession', at: '2026-07-18T11:22:00Z', body: '## Approach\n\nDelete `egress.ts` and its wiring. Turn containers join the default bridge network. Containment stops being a network property and becomes a possession property: a container can reach anything, and holds nothing worth stealing.\n\n## What a turn possesses\n\nThe operator’s forge token and a spend-capped model key. What it never possesses is any atproto credential. Validation and signing stay daemon-side behind the sidecar socket, so a turn physically cannot emit a record of a type it was not asked for.', @@ -707,6 +721,7 @@ export function fixtureSpace(): FixtureSpace { request: u6r, anchor: g3Ref, type: 'implementation', + title: 'Removed egress.ts; turns on the bridge network', at: '2026-07-19T10:55:00Z', body: 'Removed `packages/daemon/src/egress.ts` and the internal-network wiring in `container.ts`. Turn containers now attach to the default bridge, and `--network host` is rejected at the call site rather than by convention.\n\nThe env a turn receives is built by one function, `turnEnv()`, which takes the forge token and the model key and nothing else. There is no path by which a session credential reaches it, and the test asserts the exact key set rather than asserting an absence.', @@ -775,6 +790,7 @@ export function fixtureSpace(): FixtureSpace { request: u9r, anchor: g5Ref, type: 'plan', + title: 'One-hop auto-review, per type per project', at: '2026-07-14T10:28:00Z', body: '## Approach\n\nPer-type per-project config, overridable per request. The daemon writes the review request the moment a matching artifact lands, under one of its own agent identities.\n\nTerminal by construction rather than by a bound: the trigger matches on `artifact` records only, and the record a review turn emits is a `review`. It cannot re-fire, so there is no iteration counter to get wrong.', @@ -800,6 +816,7 @@ export function fixtureSpace(): FixtureSpace { request: u10r, anchor: g5Ref, type: 'implementation', + title: 'Auto-review trigger in the index-diff loop', at: '2026-07-15T10:20:00Z', body: "The trigger lives in the daemon's index-diff loop: on each ingestion cycle, any artifact new to the index whose type has `autoReview` enabled gets a review request written under the daemon's agent identity.\n\nTerminality is structural rather than guarded — the diff only inspects `com.disnetdev.radial.artifact` records, and the record a review turn emits is a `com.disnetdev.radial.review`. There is no counter to get wrong.", @@ -850,6 +867,7 @@ export function fixtureSpace(): FixtureSpace { request: u11r, anchor: g6Ref, type: 'implementation', + title: 'Six golden scenarios, shuffled 500 ways', at: '2026-07-09T14:40:00Z', body: 'Six golden scenarios — pinned versions, edits, removal ignoring the removed member’s records, re-add restoring them, competing claims, cross-goal system artifacts — each shuffled 500 ways and asserted structurally equal.\n\nThe shuffle is seeded and the seed prints on failure, so a red run reproduces exactly. This is the property the whole design leans on: two materializers that ingested the same records compute the same view.', @@ -933,6 +951,11 @@ export function fixtureSpace(): FixtureSpace { }) // Landed, and no verdict pins it. Nobody was asked — this is the flavour of the queue that exists // because landing is not done, not because anyone requested anything. + // + // Deliberately UNTITLED, and one of only two records here that are: a title is optional in the + // lexicon because a signed record cannot be given one after the fact, so a real space keeps rows + // like this for as long as its history lasts. Its row shows the line derived from its body, its + // drawer draws no heading, and both are what the fallback is supposed to look like. const u13v1 = artifact({ author: planner, rkey: '3lbr7cc3dd50f', @@ -964,6 +987,7 @@ export function fixtureSpace(): FixtureSpace { request: u14r, anchor: g8Ref, type: 'implementation', + title: 'Ignored records in a closed disclosure', body: '## What landed\n\n`Diagnostics.svelte` renders `index.ignored` and `index.edits` inside a `
` at the foot of the pane. Grouping and the reason strings come from the fold — the component reads them, it does not compose them.\n\n## Note\n\nBuilt on plan v1, which has no verdict yet. If the verdict changes the grouping this is a small change; if it changes the reason strings it is a change in `core`, not here.', links: { branch: 'radial/impl-91c4de08b7a2', @@ -1057,10 +1081,19 @@ export function fixtureSpace(): FixtureSpace { // turn's bundle — which is the real reason this section exists. const ngRef = { project: ref(radialNg) } + /** + * One version of a system document, with the request that asked for it and the verdict on it. + * + * `title` is the field the System page leads with — a project has one `architecture` chain but + * several `adr` rows, and "adr" plus the first sentence of a decision record is not something a + * human scans. It sits next to the body because the two belong together; `undefined` is passed for + * exactly one record here, ADR 0004 below, which predates the field. + */ const sysVersion = ( rkeyRequest: string, rkeyArtifact: string, typeName: string, + title: string | undefined, body: string, requestedBy: string, at: string, @@ -1083,6 +1116,7 @@ export function fixtureSpace(): FixtureSpace { request: req, anchor: ngRef, type: typeName, + ...(title ? { title } : {}), body, at, ...(prev ? { prev } : {}), @@ -1096,6 +1130,7 @@ export function fixtureSpace(): FixtureSpace { '3lbf7bb0jj58z', '3lbf7cc1kk58a', 'architecture', + 'Three processes over one shared fold', '## Shape\n\nThree processes and one shared library. The daemon watches, the sidecar writes, `core` computes the view both of them read.', tim, '2026-06-24T10:02:00Z', @@ -1105,6 +1140,7 @@ export function fixtureSpace(): FixtureSpace { '3lbf9cc3mm58b', '3lbf9dd4nn58c', 'architecture', + 'Three processes and the possession boundary', '## Shape\n\nThree processes and one shared library, plus the possession boundary between them.', tim, '2026-07-01T09:40:00Z', @@ -1116,6 +1152,9 @@ export function fixtureSpace(): FixtureSpace { '3lbg1aa2mm60p', '3lbg2ee5pp60q', 'architecture', + // Renamed at v3, and the demo needs it to be: a living document's title belongs to the version + // that carries it, so browsing back to v1 in the drawer has to show v1's name, not this one. + 'Four moving parts and the write boundary', '## Shape\n\nRadial is four moving parts over one shared library. `ingest` pulls member repos into a record store. `core` folds those records into an index — the same fold the daemon, the CLI, and this UI all run, which is why two materializers that saw the same records agree. `daemon` watches its own index for open requests naming its agents and launches a container per turn. `sidecar` is the only thing inside that container that can reach the protocol.\n\n## The boundary that matters\n\nValidation and signing live daemon-side, behind a local socket. A turn can emit exactly one record type — the one it was asked for — plus messages. No atproto credential ever enters a container.\n\n## Why the index is not a state machine\n\nThere is no derived status anywhere. A request is open because no artifact or review references it, not because something wrote `status: open`. Reviews pin exact CIDs, so an approval of v2 says nothing about v3 and nothing has to be invalidated when a v3 lands.\n\n## What this document is for\n\nThis is the first thing every turn on this project reads. If it is wrong, every plan starts wrong.', tim, '2026-07-08T14:44:00Z', @@ -1152,6 +1191,7 @@ export function fixtureSpace(): FixtureSpace { '3lbj8ff1qq66c', '3lbj8gg2rr66d', 'conventions', + 'Permutation coverage before anything consumes the index', '## Tests\n\nAnything touching the index gets permutation coverage before the daemon or UI consumes it.', ana, '2026-07-05T16:20:00Z', @@ -1161,6 +1201,7 @@ export function fixtureSpace(): FixtureSpace { '3lbm4ff7qq70r', '3lbm5gg8ss70s', 'conventions', + 'Tests, reviews and commit attribution', '## Tests\n\nAnything touching the index lands in `core` with permutation-property coverage before the daemon or the UI consumes it. A shuffled-arrival test that does not print its seed on failure is not a test, it is a rumour.\n\n## Reviews\n\nA review names the exact file and line when it can. Severity is a claim about consequence, not about confidence — if you are unsure whether something is real, say so in the body rather than downgrading it to `low`.\n\n## Commits\n\nThe agent and its DID go in the `Co-Authored-By:` trailer. On-protocol records stay the authoritative attribution; the forge only mirrors it.', ana, '2026-07-16T11:32:00Z', @@ -1174,6 +1215,7 @@ export function fixtureSpace(): FixtureSpace { '3lbnb5r7x309k', '3lbnc6s8y309m', 'adr', + '0007 · Containment by possession, not by egress', 'Status: accepted\n\n## Context\n\nTurn containers originally ran behind an egress allowlist naming the git remote, the model API, and the sidecar. Every new capability an agent needed arrived as a proxy rule. The allowlist drifted into being a second, worse specification of what agents are for.\n\n## Decision\n\nContainment is enforced by what a container possesses, not by what it can reach. Turns run on an ordinary bridge network. They hold the operator’s forge token and a spend-capped model key. They never hold an atproto credential.\n\n## Consequences\n\nA compromised turn can read the public internet and can push to the forge as the operator. It cannot forge a signed record, cannot emit a record type it was not asked for, and cannot spend past the model key’s cap.', tim, '2026-07-19T16:31:00Z', @@ -1184,10 +1226,15 @@ export function fixtureSpace(): FixtureSpace { // ADR 0004 → 0008. The comp draws "supersedes 0004" as one row with two entries; that is a // `prev` chain on a project-scoped type, which only works because §3.2 generalized prev. + // 0004 is the second deliberately UNTITLED record (see u13v1): the oldest thing in the demo, from + // before the field existed, and an append-only chain whose successor has a title while it does not. + // That is what a real ADR list will look like for years, and it is what the drawer's version rail has + // to handle — v1 draws no heading, v2 draws its own. const adr0004 = sysVersion( '3lbc2ii9tt30d', '3lbc2jj0uu30e', 'adr', + undefined, 'Status: superseded by 0008\n\n## Decision\n\nPoll every member repo on a short interval. Spaces are tens of DIDs; a subscription is machinery we have not earned yet.', tim, '2026-06-12T09:15:00Z', @@ -1197,6 +1244,7 @@ export function fixtureSpace(): FixtureSpace { '3lbr7hh1tt42b', '3lbr8ii2vv42c', 'adr', + '0008 · Jetstream first, polling as backfill', 'Status: accepted · supersedes 0004\n\n## Context\n\n0004 chose polling on the grounds that a subscription was machinery we had not earned. That was right at the time and is no longer: merge observation is the latency a human actually feels, and a poll interval short enough to hide it is a poll interval that wastes most of its requests.\n\n## Decision\n\nSubscribe to Jetstream filtered to `com.disnetdev.radial.*`. Polling stays, demoted to backfill.\n\n## Consequences\n\nTwo ingestion paths can now disagree about a record’s earliest observed revision. The fold already keeps the earliest revision seen and flags later CID changes as edit annotations, so this is a reconciliation the index handles rather than a new rule.', tim, '2026-07-25T09:38:00Z', @@ -1209,6 +1257,7 @@ export function fixtureSpace(): FixtureSpace { '3lbd3kk4ww31f', '3lbd4ll5xx31g', 'adr', + '0005 · Artifact types are registry data, not lexicon', 'Status: accepted\n\n## Decision\n\n`plan` and `implementation` ship as built-in registry records, not as lexicon. A space admin adds `security-review` by writing a registry record with a brief template and an output spec.\n\n## Consequences\n\nNew capabilities are configuration. The UI grows a button; nothing grows a schema. The cost is that a bad registry entry is now a deploy-free way to waste tokens, so registry writes stay admin-only.', tim, '2026-07-02T14:02:00Z', @@ -1218,6 +1267,7 @@ export function fixtureSpace(): FixtureSpace { '3lbc9mm6yy28h', '3lbca0n7zz28j', 'adr', + '0003 · Reviews annotate, they do not gate', 'Status: accepted\n\n## Decision\n\nA `request_changes` verdict blocks nothing mechanically. It is information on the card. A team wanting "no implementation without an approved plan" gets it as a soft UI warning, not a protocol guarantee.\n\n## Why this is safe\n\nThe thing a gate would protect against is an agent autonomously spending tokens past an unapproved plan. That cannot happen when every generation has a human click behind it.', ana, '2026-06-28T10:44:00Z', diff --git a/packages/core/src/generated/records.ts b/packages/core/src/generated/records.ts index 87281d2..890935a 100644 --- a/packages/core/src/generated/records.ts +++ b/packages/core/src/generated/records.ts @@ -81,6 +81,7 @@ export interface ArtifactRecord { project?: StrongRef type: string prev?: StrongRef + title?: string body: string bodyBlob?: BlobRef links: ArtifactLinks @@ -499,6 +500,10 @@ export const lexiconSchemas = [ "type": "ref", "ref": "com.atproto.repo.strongRef" }, + "title": { + "type": "string", + "maxLength": 200 + }, "body": { "type": "string", "maxLength": 100000 diff --git a/packages/core/src/turn-protocol.ts b/packages/core/src/turn-protocol.ts index a8920c6..dd4fb64 100644 --- a/packages/core/src/turn-protocol.ts +++ b/packages/core/src/turn-protocol.ts @@ -1,6 +1,6 @@ import type { ReviewFinding, StrongRef } from './generated/records.js' -export interface SubmitArtifactRpc { method: 'submitArtifact'; token: string; body: string; criteria?: string[]; branch?: string; commit?: string; pr?: string } +export interface SubmitArtifactRpc { method: 'submitArtifact'; token: string; title: string; body: string; criteria?: string[]; branch?: string; commit?: string; pr?: string } export interface AskQuestionRpc { method: 'askQuestion'; token: string; body: string } /** A review turn's terminal RPC. No `body` field — a ReviewRecord has none; the verdict plus * structured findings are the whole payload. The daemon owns `subject` and `request` (from turn @@ -14,6 +14,9 @@ export type TurnRpcRequest = SubmitArtifactRpc | AskQuestionRpc | SubmitReviewR export type TurnRpcResponse = { ok: true; ref: StrongRef } | { ok: false; error: string } export const TURN_LIMITS = { artifactBodyChars: 100_000, + /** `com.disnetdev.radial.artifact`'s `title` ceiling. A record over it is one every materializer + * would drop, so both writers refuse it rather than write it — see `normalizeArtifactTitle`. */ + artifactTitleChars: 200, messageBodyChars: 30_000, // The review lexicon caps findings at 100 entries × 10,000-char bodies (~1MB of body text alone, // more once JSON-encoded with per-finding path/line/severity). 2MB comfortably covers the largest @@ -21,3 +24,22 @@ export const TURN_LIMITS = { maxFrameBytes: 2_000_000, frameTimeoutMs: 30_000, } as const + +/** + * An artifact's `title` as it will be STORED, or `undefined` when the value cannot be stored. + * + * `title` is optional in the lexicon — records signed before the field existed cannot be rewritten, + * and a required field would put every one of them in `index.ignored` — but a writer that has the + * field must not produce a blank or over-long one. So the normalization lives here, shared by both + * writers (the sidecar's direct `artifact post` and the daemon's turn socket), and each surfaces its + * own error for the `undefined`: they have very different audiences, but there is only one rule about + * what a title is. + * + * Whitespace is trimmed before the ceiling is applied, because trimmed is what gets written. + */ +export function normalizeArtifactTitle(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined + const title = value.trim() + if (title === '' || title.length > TURN_LIMITS.artifactTitleChars) return undefined + return title +} diff --git a/packages/core/test/bundle.test.mjs b/packages/core/test/bundle.test.mjs index 6715b3c..1cb656a 100644 --- a/packages/core/test/bundle.test.mjs +++ b/packages/core/test/bundle.test.mjs @@ -148,6 +148,9 @@ function planBundleScenario() { goal: ref(goal), type: 'plan', prev: ref(artifactV1), + // v2 is titled and v1 is not, on purpose: a chain can begin before the field existed, and a + // bundle has to carry each version as it actually is (see the title test below). + title: 'Queue route and verdict form', body: 'Plan v2', links: {}, criteria: ['covers basics', 'covers edge cases'], @@ -479,6 +482,18 @@ describe('buildPlanTurnBundle', () => { assert.deepEqual(bundle.currentSystemArtifacts, []) }) + it('carries each version\'s own title, and omits the field for one written without it', () => { + // The point of the field in a bundle: a turn can tell two `adr` records apart, or find the system + // document it was asked to revise, without reading every body it was handed. A legacy artifact + // has no title to carry and the key is absent rather than derived — the daemon does not infer a + // title from Markdown, so a bundle must not either. + const scenario = planBundleScenario() + const bundle = buildBundle(scenario.records, scenario) + const [v1, v2] = bundle.basedOn + assert.equal('title' in v1, false) + assert.equal(v2.title, 'Queue route and verdict form') + }) + it('reports an unresolved basedOn ref in missingRefs and fetches nothing for it', () => { const scenario = planBundleScenario() const bundle = buildBundle(scenario.records, scenario) diff --git a/packages/core/test/validation.test.mjs b/packages/core/test/validation.test.mjs index b4fb13c..2493783 100644 --- a/packages/core/test/validation.test.mjs +++ b/packages/core/test/validation.test.mjs @@ -3,6 +3,7 @@ import { readFile } from 'node:fs/promises' import { describe, it } from 'node:test' import { resolve } from 'node:path' import { COLLECTIONS } from '../dist/generated/records.js' +import { normalizeArtifactTitle } from '../dist/turn-protocol.js' import { validateRecord } from '../dist/validation.js' const fixtures = resolve(import.meta.dirname, 'fixtures') @@ -104,3 +105,40 @@ it('accepts an at:// record URI where a lexicon says format: uri, and still reje assert.equal(validateRecord(COLLECTIONS.artifact, links('at://did:plc:agent/sh.tangled.repo.pull/has a space')).success, false) assert.equal(validateRecord(COLLECTIONS.artifact, links('not a uri')).success, false) }) + +it('keeps an untitled artifact valid while bounding a title that is present', () => { + // `title` is additive and OPTIONAL on the wire, and it has to stay that way: records signed before + // the field existed cannot be rewritten, and requiring it would put every one of them in + // `index.ignored` — where a materializer reads the request they answered as still open. Writers + // require it instead (sidecar `artifact post`, the daemon's turn socket). + const artifact = (extra) => ({ + $type: COLLECTIONS.artifact, + request: { uri: 'at://did:plc:human/com.disnetdev.radial.artifactRequest/r1', cid: 'c' }, + project: { uri: 'at://did:plc:human/com.disnetdev.radial.project/p1', cid: 'c' }, + type: 'adr', + body: 'Status: accepted\n\n## Decision\n\nDo the thing.', + links: {}, + createdAt: '2026-07-19T00:00:00.000Z', + ...extra, + }) + assert.equal(validateRecord(COLLECTIONS.artifact, artifact({})).success, true) + assert.equal( + validateRecord(COLLECTIONS.artifact, artifact({ title: '0008 · Jetstream first' })).success, + true, + ) + assert.equal(validateRecord(COLLECTIONS.artifact, artifact({ title: 'x'.repeat(200) })).success, true) + assert.equal(validateRecord(COLLECTIONS.artifact, artifact({ title: 'x'.repeat(201) })).success, false) + assert.equal(validateRecord(COLLECTIONS.artifact, artifact({ title: 12 })).success, false) +}) + +it('normalizes an artifact title the same way for every writer', () => { + // One rule, shared by the sidecar's direct write and the daemon's socket, so an artifact written + // inside a turn and one written from a shell cannot disagree about what a title is. + assert.equal(normalizeArtifactTitle(' Four moving parts \n'), 'Four moving parts') + assert.equal(normalizeArtifactTitle('x'.repeat(200)).length, 200) + assert.equal(normalizeArtifactTitle('x'.repeat(201)), undefined) + assert.equal(normalizeArtifactTitle(' '), undefined) + assert.equal(normalizeArtifactTitle(''), undefined) + assert.equal(normalizeArtifactTitle(undefined), undefined) + assert.equal(normalizeArtifactTitle(7), undefined) +}) diff --git a/packages/daemon/README.md b/packages/daemon/README.md index b7c891c..c4f1bb7 100644 --- a/packages/daemon/README.md +++ b/packages/daemon/README.md @@ -73,7 +73,7 @@ branch, commits with the agent/DID attribution trailer, pushes, and creates or u PR body includes the deterministic Radial artifact URI. It then calls: ``` -radial artifact submit --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)" --pr "$(gh pr view --json url -q .url)" --body-file summary.md +radial artifact submit --title "" --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)" --pr "$(gh pr view --json url -q .url)" --body-file summary.md ``` The daemon synchronously verifies a lowercase full SHA and canonical same-repository PR URL, then diff --git a/packages/daemon/src/bundle-writer.ts b/packages/daemon/src/bundle-writer.ts index 10e4725..abaabc1 100644 --- a/packages/daemon/src/bundle-writer.ts +++ b/packages/daemon/src/bundle-writer.ts @@ -9,6 +9,13 @@ export interface WrittenBundle { briefPath: string } +/** One line for an artifact in a list: its type, its short title where it has one, and its uri. The + * title is the whole point of the field — a section of seven `adr` uris says nothing about which + * decision is which — and it is omitted rather than derived for a record written before the field + * existed, exactly as the UI omits it. */ +const artifactLine = (artifact: { type: string; title?: string; uri: string }): string => + `- ${artifact.type}${artifact.title ? ` — ${artifact.title}` : ''} ${artifact.uri}` + function summarizeBundle(bundle: PlanTurnBundle): string { const lines = [ // A goal-scoped request (or a project-scoped capture with a source goal) has a goal header; a @@ -20,7 +27,11 @@ function summarizeBundle(bundle: PlanTurnBundle): string { '## Project', `${bundle.project.name} — ${bundle.project.gitUrl} (${bundle.project.defaultBranch})`, ...(bundle.subject - ? ['', '## Under review', `${bundle.subject.type} ${bundle.subject.uri}`] + ? [ + '', + '## Under review', + `${bundle.subject.type}${bundle.subject.title ? ` — ${bundle.subject.title}` : ''} ${bundle.subject.uri}`, + ] : []), // The one message an answer turn was commissioned to reply to, in full: the thread below carries // it again in order, but a turn must never have to work out which line it is answering. @@ -35,15 +46,13 @@ function summarizeBundle(bundle: PlanTurnBundle): string { : []), '', '## Based on', - ...(bundle.basedOn.length > 0 - ? bundle.basedOn.map((artifact) => `- ${artifact.type} ${artifact.uri}`) - : ['(none)']), + ...(bundle.basedOn.length > 0 ? bundle.basedOn.map(artifactLine) : ['(none)']), '', // The current version of each project-scoped system artifact rides in EVERY bundle — the agents' // long-term memory (design §8). bundle.json already carries them; render them here too. '## System artifacts', ...(bundle.currentSystemArtifacts.length > 0 - ? bundle.currentSystemArtifacts.map((artifact) => `- ${artifact.type} ${artifact.uri}`) + ? bundle.currentSystemArtifacts.map(artifactLine) : ['(none)']), '', '## Review findings', diff --git a/packages/daemon/src/harness.ts b/packages/daemon/src/harness.ts index 94eb33a..dd78207 100644 --- a/packages/daemon/src/harness.ts +++ b/packages/daemon/src/harness.ts @@ -94,6 +94,12 @@ export function buildPrompt(input: { `Read your brief at ${input.bundleDir}/brief.md — the artifact-type brief plus this request's extra instructions.`, `For full context, also read ${input.bundleDir}/bundle.md (a human-readable summary of the goal, project, based-on artifacts, prior review findings, and thread) and ${input.bundleDir}/bundle.json (the same data, structured).`, ] + // Said once, in the same words, for every artifact-producing prompt below. The title is a protocol + // field a human reads in a list of a hundred records, so what it must NOT be is worth stating: the + // daemon deliberately does not infer one from the body's Markdown, because a signed field an agent + // chose on purpose is the only kind that stays predictable. + const titleRule = + 'The `--title` is a short phrase naming what you delivered — how a row in a list should read, at most 200 characters. Not a sentence, not the opening line of your body, and not a restatement of the request.' if (input.answer) { return [ ...context, @@ -142,7 +148,8 @@ export function buildPrompt(input: { '5. Commit your work with a `Co-Authored-By: $RADIAL_AGENT_NAME ($RADIAL_AGENT_DID) <$RADIAL_AGENT_EMAIL>` trailer.', '6. Push it: `git push "$RADIAL_PUSH_REMOTE" "HEAD:refs/heads/$RADIAL_BRANCH"`. The ssh key for this is already configured (`$GIT_SSH_COMMAND`); do not generate one, and do not disable host-key checking. If the push is rejected because this agent is not a collaborator on the repository, do NOT commit around it — run `radial message post --body ""` so a human can grant it, and stop.', '7. Do NOT try to open a pull request — tangled has no CLI for it and Radial opens it for you from the branch you pushed, as the same identity that signs your artifact.', - '8. Write a concise implementation summary to a file, then run `radial artifact submit --body-file --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)"`. There is no `--pr` on this forge.', + '8. Write a concise implementation summary to a file, then run `radial artifact submit --title "" --body-file --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)"`. There is no `--pr` on this forge.', + ` ${titleRule}`, '', 'If you cannot complete the brief and need input first: run `radial message post --body ""` instead, do NOT commit, and stop.', '', @@ -163,7 +170,8 @@ export function buildPrompt(input: { 'When your implementation is complete:', '5. Commit your work with a `Co-Authored-By: $RADIAL_AGENT_NAME ($RADIAL_AGENT_DID) <$RADIAL_AGENT_EMAIL>` trailer and push `$RADIAL_BRANCH`.', '6. Find an existing PR for `$RADIAL_BRANCH`; use `gh pr edit` when one exists, otherwise `gh pr create --base "$RADIAL_BASE_BRANCH"`. Its body must include the literal Markdown link `[Radial artifact]($RADIAL_ARTIFACT_URI)`. Never open a second pull request for a branch that already has one — a reused branch means this turn continues the pull request its predecessor opened.', - '7. Write a concise implementation summary to a file, then run `radial artifact submit --body-file --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)" --pr "$(gh pr view --json url -q .url)"`.', + '7. Write a concise implementation summary to a file, then run `radial artifact submit --title "" --body-file --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)" --pr "$(gh pr view --json url -q .url)"`.', + ` ${titleRule}`, '', 'If you cannot complete the brief and need input first: run `radial message post --body ""` instead, do NOT commit, and stop.', '', @@ -175,7 +183,8 @@ export function buildPrompt(input: { `The project's read-only checkout is at ${input.workdir} — inspect it as needed to complete the brief.`, '', 'When you are ready, do exactly ONE of the following:', - '- If you can complete the brief: write your result to a file, then deliver it by running `radial artifact submit --body-file `.', + '- If you can complete the brief: write your result to a file, then deliver it by running `radial artifact submit --title "" --body-file `.', + ` ${titleRule}`, '- If you cannot complete the brief and need input first: run `radial message post --body ""` instead, and stop.', '', 'Do not print your result to stdout — it is ignored. `radial` is on PATH and is already authenticated for this turn (unix-socket mode); do not look for or ask for credentials.', diff --git a/packages/daemon/src/turn-socket.ts b/packages/daemon/src/turn-socket.ts index b9dad78..c092df8 100644 --- a/packages/daemon/src/turn-socket.ts +++ b/packages/daemon/src/turn-socket.ts @@ -8,6 +8,7 @@ import { urlOnlyForgeAdapters } from './forge-github.js' import { COLLECTIONS, TURN_LIMITS, + normalizeArtifactTitle, type ArtifactRecord, type Collection, type MessageRecord, @@ -220,7 +221,7 @@ function concatBytes(chunks: Uint8Array[]): Uint8Array { } type ParsedEnvelope = - | { method: 'submitArtifact'; token: string; body: string; criteria?: string[]; branch?: string; commit?: string; pr?: string } + | { method: 'submitArtifact'; token: string; title?: string; body: string; criteria?: string[]; branch?: string; commit?: string; pr?: string } | { method: 'askQuestion'; token: string; body: string } | { method: 'submitAnswer'; token: string; body: string } | { method: 'submitReview'; token: string; verdict: 'approve' | 'request_changes'; findings: ReviewFinding[] } @@ -269,9 +270,13 @@ function parseEnvelope(raw: unknown): ParsedEnvelope | undefined { if (typeof token !== 'string') return undefined if (method === 'submitReview') return parseReviewEnvelope(raw, token) if (method !== 'submitArtifact' && method !== 'askQuestion' && method !== 'submitAnswer') return undefined - const { body, criteria, branch, commit, pr } = raw + const { body, criteria, branch, commit, pr, title } = raw if (typeof body !== 'string') return undefined if (method === 'submitArtifact') { + // Shape only: a `title` that is present but not a string is a malformed envelope. Whether it is + // usable — present at all, non-blank, within the lexicon's ceiling — is #submitArtifact's + // decision, so that a turn gets an error message saying what to do instead of "invalid envelope". + if (title !== undefined && typeof title !== 'string') return undefined let parsedCriteria: string[] | undefined if (criteria !== undefined) { if (!Array.isArray(criteria) || criteria.some((entry) => typeof entry !== 'string')) return undefined @@ -286,6 +291,7 @@ function parseEnvelope(raw: unknown): ParsedEnvelope | undefined { method, token, body, + ...(title !== undefined ? { title: title as string } : {}), ...(parsedCriteria ? { criteria: parsedCriteria } : {}), ...(commit !== undefined ? { commit: commit as string } : {}), ...(branch !== undefined ? { branch: branch as string } : {}), @@ -465,6 +471,21 @@ export class TurnSocketServer { if (this.#observation.artifact || this.#observation.review || this.#observation.answer || this.#submitReserved) { return { ok: false, error: 'an artifact was already submitted for this request' } } + // The title, checked BEFORE the reservation below rather than after it: nothing has been written + // and nothing is in flight, so a turn that mis-titled its submit may simply submit again. (The + // paths that release the reservation on failure are the ones that had to reserve it first because + // they await.) `title` stays optional on the wire so historical records remain valid, and required + // of a writer, which is what keeps every NEW artifact scannable. + const title = normalizeArtifactTitle(envelope.title) + if (title === undefined) { + return { + ok: false, + error: + envelope.title === undefined + ? 'artifact submit requires --title: a short phrase naming this artifact, not its first paragraph' + : `--title must be non-blank and at most ${TURN_LIMITS.artifactTitleChars} characters`, + } + } // Reserve synchronously, before the first `await` below, so a second concurrent submit (a // different connection/socket on this same server) sees the reservation and is rejected the // same way a sequential second submit would be — it can never race past this check and @@ -552,6 +573,7 @@ export class TurnSocketServer { request: { uri: request.uri, cid: request.cid }, ...anchor, type, + title, body: envelope.body, links, ...(prev ? { prev } : {}), @@ -594,6 +616,10 @@ export class TurnSocketServer { JSON.stringify(current.links) !== JSON.stringify(links) || JSON.stringify(current.prev) !== JSON.stringify(prev) || JSON.stringify(current.criteria ?? []) !== JSON.stringify(envelope.criteria ?? []) || + // The title is part of the payload, so it is part of what makes the record at this rkey the + // one this submit would have written. Omitting it would let a retry adopt — and report as + // its own — an artifact carrying a different title than the one it just submitted. + current.title !== title || current.body !== envelope.body ) { this.#submitReserved = false diff --git a/packages/daemon/test/bundle-writer.test.mjs b/packages/daemon/test/bundle-writer.test.mjs index 0c4fcd0..a26ddd8 100644 --- a/packages/daemon/test/bundle-writer.test.mjs +++ b/packages/daemon/test/bundle-writer.test.mjs @@ -104,6 +104,28 @@ it('writeBundle summary renders the "## System artifacts" section (currentSystem assert.match(summaryB, /## System artifacts\n\(none\)/) }) +it('writeBundle summary names a titled artifact by its title, and an untitled one exactly as before', async () => { + // The reason `title` is in the bundle at all: a "## System artifacts" section listing seven `adr` + // uris tells a turn nothing about which decision is which. A record written before the field + // existed has no title to show, and the line is left as it was rather than deriving one. + const mixed = { + ...bundle, + basedOn: [ + { uri: 'at://did:plc:agent/com.disnetdev.radial.artifact/plan-1', cid: 'plan-cid', type: 'plan', body: 'the plan', links: {}, createdAt: '2026-01-01T00:03:00Z' }, + ], + currentSystemArtifacts: [ + { uri: 'at://did:plc:agent/com.disnetdev.radial.artifact/adr-1', cid: 'adr-cid', type: 'adr', title: '0008 · Jetstream first', body: 'the adr', links: {}, createdAt: '2026-01-01T00:05:00Z' }, + ], + subject: { uri: 'at://did:plc:agent/com.disnetdev.radial.artifact/impl-1', cid: 'subj-cid', type: 'implementation', title: 'Removed egress.ts', body: 'the implementation', links: {}, createdAt: '2026-01-01T00:04:00Z' }, + } + const dir = await mkdtemp(join(tmpdir(), 'radial-bundle-titles-')) + await writeBundle({ bundle: mixed, brief: '# brief\n', dir: join(dir, 'turn') }) + const summary = await readFile(join(dir, 'turn', 'bundle.md'), 'utf8') + assert.match(summary, /- adr — 0008 · Jetstream first at:\/\/did:plc:agent\/com\.disnetdev\.radial\.artifact\/adr-1/) + assert.match(summary, /implementation — Removed egress\.ts at:\/\/did:plc:agent/) + assert.match(summary, /- plan at:\/\/did:plc:agent\/com\.disnetdev\.radial\.artifact\/plan-1/) +}) + it('assertGitUrlAllowed rejects file:// by default but allows https://, and allows file:// when opted in', () => { assert.throws(() => assertGitUrlAllowed('file:///tmp/repo', ['https']), /not allowed/) assert.doesNotThrow(() => assertGitUrlAllowed('https://example.test/repo.git', ['https'])) diff --git a/packages/daemon/test/dispatch.test.mjs b/packages/daemon/test/dispatch.test.mjs index 08ed37d..e23f26a 100644 --- a/packages/daemon/test/dispatch.test.mjs +++ b/packages/daemon/test/dispatch.test.mjs @@ -1390,6 +1390,7 @@ it('fulfilled turn, driven through TurnDispatcher.pump/drain, transitions the le const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'the plan', }) assert.equal(response.ok, true) @@ -1725,6 +1726,7 @@ it('E2E: a project-scoped adr request lands a project-anchored artifact that a f const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'the ADR body', }) assert.equal(response.ok, true) diff --git a/packages/daemon/test/docker-smoke.test.mjs b/packages/daemon/test/docker-smoke.test.mjs index 81cd591..d7a5385 100644 --- a/packages/daemon/test/docker-smoke.test.mjs +++ b/packages/daemon/test/docker-smoke.test.mjs @@ -108,7 +108,11 @@ describe('docker smoke: radial-turn against a real Docker daemon', { skip: !RUN_ const result = await runner.run({ label: 'radial-docker-smoke-submit', image: TURN_IMAGE, - argv: ['sh', '-c', 'id -u > /marker/uid-marker && radial artifact submit --body-file /bundle/brief.md'], + argv: [ + 'sh', + '-c', + 'id -u > /marker/uid-marker && radial artifact submit --title "Docker smoke artifact" --body-file /bundle/brief.md', + ], env: { RADIAL_SIDECAR_SOCKET: sidecarSocket, RADIAL_TURN_TOKEN: token, @@ -302,7 +306,7 @@ describe('docker smoke: synchronous implementation submit', { skip: !RUN_DOCKER_ 'git add -A', 'git commit -q -m impl', 'printf "impl summary" > /tmp/body.md', - `radial artifact submit --body-file /tmp/body.md --branch ${branch} --commit "$(git rev-parse HEAD)" --pr https://github.com/acme/widget/pull/7`, + `radial artifact submit --title "Docker smoke implementation" --body-file /tmp/body.md --branch ${branch} --commit "$(git rev-parse HEAD)" --pr https://github.com/acme/widget/pull/7`, ].join(' && ') const result = await new DockerRunner().run({ label: 'radial-docker-smoke-impl', diff --git a/packages/daemon/test/harness.test.mjs b/packages/daemon/test/harness.test.mjs index 86fb33b..8abff03 100644 --- a/packages/daemon/test/harness.test.mjs +++ b/packages/daemon/test/harness.test.mjs @@ -150,6 +150,21 @@ it('implementation prompt owns the GitHub branch, commit, push, and PR workflow' assert.match(prompt, /Never open a second pull request for a branch that already has one/) }) +it('every artifact-producing prompt asks for a short title, and says what it must not be', () => { + // The submit is refused without one (turn-socket.ts), so a prompt that did not ask for a title + // would be a turn that fails at its own last step. It is stated the same way in all three prompts: + // the daemon deliberately infers nothing from the body's Markdown, so the words the agent is given + // are the whole of what makes the signed field deliberate. + const harness = new ClaudeCodeHarness() + const base = { briefPath: '/bundle/brief.md', bundleDir: '/bundle', workdir: '/work', models: [] } + for (const variant of [{}, { implementation: true }, { implementation: true, forge: 'tangled' }]) { + const prompt = harness.invocation({ ...base, ...variant }).argv[2] + assert.match(prompt, /radial artifact submit --title ""/) + assert.match(prompt, /at most 200 characters/) + assert.match(prompt, /not the opening line of your body/) + } +}) + it('answer prompt directs the agent to reply in the thread, and never to write an artifact', () => { const invocation = new ClaudeCodeHarness().invocation({ briefPath: '/bundle/brief.md', diff --git a/packages/daemon/test/turn-socket.test.mjs b/packages/daemon/test/turn-socket.test.mjs index 33b09a9..64c4b12 100644 --- a/packages/daemon/test/turn-socket.test.mjs +++ b/packages/daemon/test/turn-socket.test.mjs @@ -70,6 +70,7 @@ it('submitArtifact writes a plan artifact anchored to the request and goal', asy const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'the plan body', criteria: ['criterion one'], }) @@ -83,6 +84,7 @@ it('submitArtifact writes a plan artifact anchored to the request and goal', asy assert.deepEqual(record.request, { uri: REQUEST.uri, cid: REQUEST.cid }) assert.deepEqual(record.goal, REQUEST.goal) assert.equal(record.type, 'plan') + assert.equal(record.title, 'A short title') assert.equal(record.body, 'the plan body') assert.deepEqual(record.links, {}) assert.deepEqual(record.criteria, ['criterion one']) @@ -94,6 +96,108 @@ it('submitArtifact writes a plan artifact anchored to the request and goal', asy }) }) +it('requires a usable title at the untrusted edge, and writes nothing without one', async () => { + // The lexicon leaves `title` optional so historical records stay valid; the WRITER requires it, and + // the socket is the writer for every turn. Each refusal here leaves the turn retryable: nothing is + // written, and the in-flight reservation is not taken, so a corrected submit still lands. + await withTempDir(async (directory) => { + const pds = new LocalPds('did:plc:agent-title') + const client = await makeClient(pds) + const { server, socketPath } = await startServer(directory, client) + try { + const missing = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'no title' }) + assert.equal(missing.ok, false) + assert.match(missing.error, /requires --title/) + + const blank = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: ' \n ', body: 'x' }) + assert.equal(blank.ok, false) + assert.match(blank.error, /non-blank/) + + const long = await sendLine(socketPath, { + method: 'submitArtifact', + token: TOKEN, + title: 'x'.repeat(201), + body: 'x', + }) + assert.equal(long.ok, false) + assert.match(long.error, /200 characters/) + + // A title that is not a string at all is a malformed envelope, refused before anything else. + const wrongType = await sendLine(socketPath, { + method: 'submitArtifact', + token: TOKEN, + title: { short: 'no' }, + body: 'x', + }) + assert.equal(wrongType.ok, false) + assert.match(wrongType.error, /invalid turn RPC envelope/) + + assert.equal(pds.records.size, 0) + + // Not a spent turn: the same connection sequence ends in a real artifact once a title is given. + const good = await sendLine(socketPath, { + method: 'submitArtifact', + token: TOKEN, + title: ' Four moving parts \n', + body: 'x', + }) + assert.equal(good.ok, true) + const stored = [...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact) + assert.equal(stored.length, 1) + // Stored trimmed — the trimmed form is what every reader compares and displays. + assert.equal(stored[0].value.title, 'Four moving parts') + } finally { + await server.close() + } + }) +}) + +it('refuses to adopt a deterministic record whose only difference is its title', async () => { + // The title is part of the payload, so it is part of what makes the record at the request's rkey + // the one this submit would have written. Adopting on a title mismatch would report an artifact as + // this turn's own while the space showed a different name for it. + await withTempDir(async (directory) => { + const pds = new LocalPds('did:plc:agent-title2') + const client = await makeClient(pds) + + const first = await startServer(directory, client) + const firstResponse = await sendLine(first.socketPath, { + method: 'submitArtifact', + token: TOKEN, + title: 'The title that landed', + body: 'same body', + }) + await first.server.close() + assert.equal(firstResponse.ok, true) + + const same = await startServer(directory, client) + const sameResponse = await sendLine(same.socketPath, { + method: 'submitArtifact', + token: TOKEN, + title: 'The title that landed', + body: 'same body', + }) + await same.server.close() + assert.equal(sameResponse.ok, true) + assert.deepEqual(sameResponse.ref, firstResponse.ref) + + const renamed = await startServer(directory, client) + const renamedResponse = await sendLine(renamed.socketPath, { + method: 'submitArtifact', + token: TOKEN, + title: 'A different title', + body: 'same body', + }) + await renamed.server.close() + assert.equal(renamedResponse.ok, false) + assert.match(renamedResponse.error, /refusing to adopt tampered record/) + + const stored = [...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact) + assert.equal(stored.length, 1) + assert.equal(stored[0].value.title, 'The title that landed') + }) +}) + it('askQuestion writes a message anchored to the request goal', async () => { await withTempDir(async (directory) => { const pds = new LocalPds('did:plc:agent2') @@ -127,6 +231,7 @@ it('rejects an oversized artifact body and writes no record', async () => { const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'this body is definitely longer than ten characters', }) assert.equal(response.ok, false) @@ -148,6 +253,7 @@ it('rejects a frame that exceeds the hard byte cap and writes no record', async const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'a modestly sized body that is well under the per-field limit but the frame as a whole exceeds the tiny cap', }) assert.equal(response.ok, false) @@ -165,9 +271,9 @@ it('accepts only one terminal artifact per request', async () => { const client = await makeClient(pds) const { server, socketPath } = await startServer(directory, client) try { - const first = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'first' }) + const first = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'first' }) assert.equal(first.ok, true) - const second = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'second' }) + const second = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'second' }) assert.equal(second.ok, false) const stored = [...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact) assert.equal(stored.length, 1) @@ -187,6 +293,7 @@ it('adopts the existing record on RecordAlreadyExists instead of duplicating it' const firstResponse = await sendLine(first.socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'idempotent body', }) await first.server.close() @@ -198,6 +305,7 @@ it('adopts the existing record on RecordAlreadyExists instead of duplicating it' const secondResponse = await sendLine(second.socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'idempotent body', }) await second.server.close() @@ -218,6 +326,7 @@ it('rejects a mismatched turn token and writes no record', async () => { const response = await sendLine(socketPath, { method: 'submitArtifact', token: 'wrong-token', + title: 'A short title', body: 'should not land', }) assert.equal(response.ok, false) @@ -244,7 +353,7 @@ it('tcp transport: submitArtifact over a raw TCP connection to boundPort writes let data = '' socket.on('connect', () => socket.write( - `${JSON.stringify({ method: 'submitArtifact', token: TOKEN, body: 'the plan body', criteria: ['criterion one'] })}\n`, + `${JSON.stringify({ method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'the plan body', criteria: ['criterion one'] })}\n`, ), ) socket.on('data', (chunk) => { @@ -290,7 +399,7 @@ it('implementation submit synchronously writes the deterministic artifact with v context: { mode: { kind: 'artifact', type: 'implementation' }, implementation: { gitUrl: 'https://github.com/acme/widget.git', branch }, prev }, }) try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'done', branch, commit: 'a'.repeat(40), pr: 'https://github.com/acme/widget/pull/7' }) + const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'done', branch, commit: 'a'.repeat(40), pr: 'https://github.com/acme/widget/pull/7' }) assert.equal(response.ok, true) assert.equal(response.ref.uri, `at://${pds.did}/${COLLECTIONS.artifact}/${implArtifactRkey(REQUEST.uri, REQUEST.cid)}`) const record = [...pds.records.values()].find((entry) => entry.value.$type === COLLECTIONS.artifact).value @@ -315,6 +424,7 @@ it('implementation submit rejects the wrong branch, noncanonical PR URLs, and cr sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'done', branch, commit: 'a'.repeat(40), @@ -353,6 +463,7 @@ it('implementation adoption refuses an existing deterministic record with differ const frame = { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'done', branch, commit: 'a'.repeat(40), @@ -457,6 +568,7 @@ it('mutual exclusion: submitReview is rejected on an artifact turn, submitArtifa const artifactOnReview = await sendLine(reviewTurn.socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'sneaky artifact', }) await reviewTurn.server.close() @@ -624,7 +736,7 @@ it('project-scoped submitArtifact writes an artifact anchored to the project (no context: { request: PROJECT_REQUEST, mode: { kind: 'artifact', type: 'adr' } }, }) try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'the adr' }) + const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'the adr' }) assert.equal(response.ok, true) const record = [...pds.records.values()].find((r) => r.value.$type === COLLECTIONS.artifact).value assert.deepEqual(record.project, PROJECT) @@ -644,12 +756,12 @@ it('project-scoped submitArtifact adopts on retry, comparing the project anchor' const context = { request: PROJECT_REQUEST, mode: { kind: 'artifact', type: 'adr' } } const first = await startServer(directory, client, { context }) - const firstResponse = await sendLine(first.socketPath, { method: 'submitArtifact', token: TOKEN, body: 'adr body' }) + const firstResponse = await sendLine(first.socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'adr body' }) await first.server.close() assert.equal(firstResponse.ok, true) const second = await startServer(directory, client, { context }) - const secondResponse = await sendLine(second.socketPath, { method: 'submitArtifact', token: TOKEN, body: 'adr body' }) + const secondResponse = await sendLine(second.socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'adr body' }) await second.server.close() assert.equal(secondResponse.ok, true) assert.deepEqual(secondResponse.ref, firstResponse.ref) @@ -686,7 +798,7 @@ it('project-scoped submitArtifact refuses to adopt an existing record carrying a context: { request: PROJECT_REQUEST, mode: { kind: 'artifact', type: 'adr' } }, }) try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'the adr' }) + const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'the adr' }) // Adoption is refused. Two guards enforce this: getOwnRecord validates the fetched record // (exactly-one anchor) and rejects a dual-anchor record on read, and #submitArtifact's adoption // comparison additionally requires the stray anchor to be absent (defense in depth if read-side @@ -926,6 +1038,7 @@ it('a brokered submit needs no --pr: the daemon opens the pull and stamps the li const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'done', branch, commit: 'a'.repeat(40), @@ -1015,6 +1128,7 @@ it('mutual exclusion: submitAnswer only on an answer turn, and an answer turn su const artifactOnAnswer = await sendLine(answerTurn.socketPath, { method: 'submitArtifact', token: TOKEN, + title: 'A short title', body: 'sneaky artifact', }) const reviewOnAnswer = await sendLine(answerTurn.socketPath, { @@ -1131,7 +1245,7 @@ it('a broker failure is a submit error and writes NO artifact, leaving the turn }) const { server, socketPath } = await startServer(directory, client, { context: TANGLED_IMPL(adapter, branch) }) try { - const failed = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'done', branch, commit: 'a'.repeat(40) }) + const failed = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'done', branch, commit: 'a'.repeat(40) }) assert.equal(failed.ok, false) assert.match(failed.error, /could not open the pull request.*knot unreachable/) assert.equal(pds.records.size, 0) @@ -1139,7 +1253,7 @@ it('a broker failure is a submit error and writes NO artifact, leaving the turn // The reservation is released, so the same server can still accept a real submit. const { adapter: working } = brokeringForge() server.observation.pull = undefined - const retried = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'done', branch, commit: 'a'.repeat(40) }) + const retried = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'done', branch, commit: 'a'.repeat(40) }) assert.equal(retried.ok, false, 'the adapter on this server still throws') assert.ok(working.openPullRequest) } finally { @@ -1160,7 +1274,7 @@ it('a GitHub submit with no --pr is still rejected: only a brokering forge relax }, }) try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'done', branch, commit: 'a'.repeat(40) }) + const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, title: 'A short title', body: 'done', branch, commit: 'a'.repeat(40) }) assert.equal(response.ok, false) assert.match(response.error, /requires --branch, --commit, and --pr/) assert.equal(pds.records.size, 0) diff --git a/packages/daemon/test/turn.test.mjs b/packages/daemon/test/turn.test.mjs index 107cdca..182ad83 100644 --- a/packages/daemon/test/turn.test.mjs +++ b/packages/daemon/test/turn.test.mjs @@ -163,6 +163,7 @@ it('fulfilled: submitArtifact over the derived socket produces a fulfilled outco const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'PLAN', }) assert.equal(response.ok, true) @@ -227,6 +228,7 @@ it('implementation turn injects only operator GitHub/model credentials and synch const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'Implemented phase 4.5.', branch, commit, @@ -533,6 +535,7 @@ it('idempotent adoption: two turns for the same request converge on one artifact const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'idempotent plan body', }) assert.equal(response.ok, true) @@ -754,6 +757,7 @@ it('project-scoped turn: default-branch read-only checkout and a project-anchore const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'the ADR', }) assert.equal(response.ok, true) @@ -1105,6 +1109,7 @@ it('a tangled implementation turn holds its ssh key and nothing else — no GH_T const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'Implemented it.', branch, commit, @@ -1160,6 +1165,7 @@ it('a GitHub turn is unchanged when a registry is present: still GH_TOKEN, still const response = await sendLine(hostSocketPath(spec), { method: 'submitArtifact', token: spec.env.RADIAL_TURN_TOKEN, + title: 'A short title', body: 'Implemented it.', branch, commit: 'a'.repeat(40), diff --git a/packages/lexicons/README.md b/packages/lexicons/README.md index f768fd8..825f1b0 100644 --- a/packages/lexicons/README.md +++ b/packages/lexicons/README.md @@ -39,6 +39,31 @@ Artifact records always carry a short `body`. When the full content would overflow an atproto record, `body` is the summary and `bodyBlob` points to the complete body. +## An artifact's `title` is optional on the wire and required of a writer + +`com.disnetdev.radial.artifact` carries an optional `title`: a short phrase (at +most 200 characters) naming the artifact wherever it is listed rather than read — +a row, a chip, a bundle's "System artifacts" section. It is not in `required`, +and it must not become required. Records signed before the field existed cannot +be rewritten, so a required `title` would fail validation for all of them, and a +conforming materializer must ignore a record that fails validation — which would +read the requests they answered as still open and keep dispatching them. + +The obligation therefore lives in the writers, and both of Radial's have it: the +sidecar's `radial artifact post` and the daemon's turn socket each require a +`--title`, trim it, and refuse a blank one or one over 200 characters rather than +writing a record every materializer would drop. So: + +- a record **without** `title` is valid, and stays valid forever; +- a record **with** `title` carries a non-blank, trimmed value within the ceiling; +- a reader shows the title where there is one and may fall back to something + derived from the body where there is not. That fallback is display-only: it is + not protocol data, and nothing may persist it back into a record. + +Nothing infers a title from the body's Markdown at write time. An explicit value +chosen by whoever wrote the artifact is what keeps the signed field deliberate, +and keeps two implementations from disagreeing about what a record is called. + ## Claim leases are durations, and they are bounded A `claim` record is the one place where one implementation's clock changes diff --git a/packages/lexicons/lexicons/com.disnetdev.radial.artifact.json b/packages/lexicons/lexicons/com.disnetdev.radial.artifact.json index 2658746..0aa8ced 100644 --- a/packages/lexicons/lexicons/com.disnetdev.radial.artifact.json +++ b/packages/lexicons/lexicons/com.disnetdev.radial.artifact.json @@ -22,6 +22,7 @@ "project": {"type": "ref", "ref": "com.atproto.repo.strongRef"}, "type": {"type": "string", "maxLength": 100}, "prev": {"type": "ref", "ref": "com.atproto.repo.strongRef"}, + "title": {"type": "string", "maxLength": 200}, "body": {"type": "string", "maxLength": 100000}, "bodyBlob": {"type": "blob", "accept": ["text/markdown", "text/plain", "application/json"], "maxSize": 10000000}, "links": {"type": "ref", "ref": "#links"}, diff --git a/packages/sidecar/README.md b/packages/sidecar/README.md index 1403799..df649f5 100644 --- a/packages/sidecar/README.md +++ b/packages/sidecar/README.md @@ -33,19 +33,26 @@ GOAL=$(radial goal create --profile alice --project "$PROJECT" \ REQUEST_V1=$(radial request create --profile alice --goal "$GOAL" \ --type plan --assignee bob.example) PLAN_V1=$(radial artifact post --profile bob --request "$REQUEST_V1" \ - --body-file plan-v1.md) + --title 'Queue route and verdict form' --body-file plan-v1.md) radial review post --profile alice --subject "$PLAN_V1" \ --verdict request_changes REQUEST_V2=$(radial request create --profile alice --goal "$GOAL" \ --type plan --assignee bob.example --based-on "$PLAN_V1") PLAN_V2=$(radial artifact post --profile bob --request "$REQUEST_V2" \ - --prev "$PLAN_V1" --body-file plan-v2.md) + --prev "$PLAN_V1" --title 'Queue route and verdict form' --body-file plan-v2.md) radial review post --profile alice --subject "$PLAN_V2" --verdict approve radial member remove --profile alice --space "$SPACE" --actor bob.example ``` +`artifact post` requires `--title`: a short phrase naming the artifact (at most +200 characters), which is what a row, a chip and a bundle show instead of the +first paragraph of the body. It is trimmed, and a blank or over-long one is +refused rather than written. The field itself is optional in the lexicon — +records signed before it existed cannot be given one, and readers fall back to a +line derived from the body for those — but every writer requires it. + `space create` also publishes the built-in artifact-type registry records: the goal-scoped `plan` and `implementation`, and the project-scoped system artifacts `architecture`, `architecture-inventory`, `glossary` and `conventions` (all diff --git a/packages/sidecar/src/cli.ts b/packages/sidecar/src/cli.ts index b5aed00..852589c 100644 --- a/packages/sidecar/src/cli.ts +++ b/packages/sidecar/src/cli.ts @@ -43,8 +43,11 @@ export const help = `radial — human CLI for Radial records [--assignee HANDLE|DID] [--subject REF] [--based-on REF]... [--brief TEXT] radial request retract --request REF - radial artifact post --request REF (--body TEXT | --body-file PATH) [--prev REF] - [--criterion TEXT]... + radial artifact post --request REF --title TITLE + (--body TEXT | --body-file PATH) [--prev REF] [--criterion TEXT]... + --title is a short phrase naming the artifact (at most 200 characters) — + what a row shows instead of the first paragraph of the body. Required, and + it must not be blank. radial message post (--goal REF | --artifact REF) (--body TEXT | --body-file PATH) [--re REF] [--parent REF] [--mention HANDLE|DID]... --parent makes the message a REPLY to another message on the same target; diff --git a/packages/sidecar/src/commands.ts b/packages/sidecar/src/commands.ts index 4de1deb..08c6bb6 100644 --- a/packages/sidecar/src/commands.ts +++ b/packages/sidecar/src/commands.ts @@ -1,6 +1,8 @@ import { COLLECTIONS, + TURN_LIMITS, joinRkey, + normalizeArtifactTitle, type Collection, type RecordByCollection, type StrongRef, @@ -293,6 +295,31 @@ async function optionalInputText( return inputText(args, deps, name) } +/** + * `--title`, the short phrase that names the artifact wherever it is listed rather than read. + * + * Required of every writer even though the lexicon leaves it optional (see `normalizeArtifactTitle`): + * the field exists because a row deriving its own label from the first paragraph of a body is hard to + * scan, and a writer allowed to omit it would keep producing exactly those rows. The ceiling is a + * refusal rather than a truncation — a title a materializer would drop takes the whole record with + * it, and silently cutting somebody's sentence at 200 characters is its own kind of wrong. + */ +export function artifactTitle(args: string[]): string { + const raw = optional(args, '--title') + if (raw === undefined) { + throw new Error('Missing --title (a short phrase naming this artifact, not its first paragraph)') + } + const title = normalizeArtifactTitle(raw) + if (title === undefined) { + throw new Error( + raw.trim() === '' + ? '--title cannot be blank' + : `--title must be at most ${TURN_LIMITS.artifactTitleChars} characters (got ${raw.trim().length})`, + ) + } + return title +} + const ref = (record: ResolvedRecord): StrongRef => ({ uri: record.uri, cid: record.cid }) export async function runCli(args: string[], deps: CliDependencies): Promise { @@ -637,12 +664,14 @@ export async function runCli(args: string[], deps: CliDependencies): Promise { at(aliceDid, 4), ) const planV1 = await runCli( - ['artifact', 'post', '--request', locator(requestV1), '--body', 'Plan version one'], + ['artifact', 'post', '--request', locator(requestV1), '--title', 'Plan v1', '--body', 'Plan version one'], at(bobDid, 5), ) await runCli( @@ -97,7 +97,7 @@ describe('Phase 2 human-only artifact loop', () => { at(aliceDid, 7), ) const planV2 = await runCli( - ['artifact', 'post', '--request', locator(requestV2), '--prev', locator(planV1), '--body', 'Plan version two'], + ['artifact', 'post', '--request', locator(requestV2), '--prev', locator(planV1), '--title', 'Plan v2', '--body', 'Plan version two'], at(bobDid, 8), ) await runCli( @@ -114,6 +114,34 @@ describe('Phase 2 human-only artifact loop', () => { assert.equal(view.artifacts.every((artifact) => artifact.did === bobDid), true) assert.equal(index.ignored.length, 0) + // Every artifact this writer produced carries its own short title. The lexicon leaves the field + // optional only so records signed before it existed stay valid — a writer that omitted it would + // go on producing rows that have to derive a name from the first paragraph of a body. So absent, + // blank and over-long are refusals, and none of them writes a record. + const titleOf = (uri) => view.artifacts.find((artifact) => artifact.uri === uri).value.title + assert.equal(titleOf(planV1.primary.uri), 'Plan v1') + assert.equal(titleOf(planV2.primary.uri), 'Plan v2') + const before = pdss.get(bobDid).records.size + await assert.rejects( + runCli(['artifact', 'post', '--request', locator(requestV2), '--body', 'untitled'], at(bobDid, 8)), + /Missing --title/, + ) + await assert.rejects( + runCli( + ['artifact', 'post', '--request', locator(requestV2), '--title', ' ', '--body', 'blank'], + at(bobDid, 8), + ), + /--title cannot be blank/, + ) + await assert.rejects( + runCli( + ['artifact', 'post', '--request', locator(requestV2), '--title', 'x'.repeat(201), '--body', 'long'], + at(bobDid, 8), + ), + /at most 200 characters/, + ) + assert.equal(pdss.get(bobDid).records.size, before) + await runCli( ['member', 'remove', '--space', locator(space), '--did', bobDid], { @@ -327,7 +355,7 @@ describe('Phase 2 human-only artifact loop', () => { at(aliceDid, 5), ) const plan = await runCli( - ['artifact', 'post', '--request', locator(request), '--body', 'Plan version one'], + ['artifact', 'post', '--request', locator(request), '--title', 'Plan v1', '--body', 'Plan version one'], at(bobDid, 6), ) const unapproved = [] @@ -521,7 +549,7 @@ describe('sanctioned goal closure and inline review findings', () => { at(aliceDid, 7), ) const plan = await runCli( - ['artifact', 'post', '--request', locator(request), '--body', 'Plan version one'], + ['artifact', 'post', '--request', locator(request), '--title', 'Plan v1', '--body', 'Plan version one'], at(bobDid, 8), ) const findings = [ diff --git a/packages/sidecar/test/socket.test.mjs b/packages/sidecar/test/socket.test.mjs index f1db7c0..137786c 100644 --- a/packages/sidecar/test/socket.test.mjs +++ b/packages/sidecar/test/socket.test.mjs @@ -13,31 +13,43 @@ const readText = async () => { describe('buildTurnRpc mapping', () => { it('maps artifact submit with an inline body', async () => { - const rpc = await buildTurnRpc(['artifact', 'submit', '--body', 'hi'], 'tok', readText) - assert.deepEqual(rpc, { method: 'submitArtifact', token: 'tok', body: 'hi' }) + const rpc = await buildTurnRpc(['artifact', 'submit', '--title', 'A short name', '--body', 'hi'], 'tok', readText) + assert.deepEqual(rpc, { method: 'submitArtifact', token: 'tok', title: 'A short name', body: 'hi' }) }) it('threads --commit (implementation submit) through to the RPC', async () => { const rpc = await buildTurnRpc( - ['artifact', 'submit', '--body', 'summary', '--commit', '9b81c449326f'], + ['artifact', 'submit', '--title', 'Dropped the allowlist', '--body', 'summary', '--commit', '9b81c449326f'], 'tok', readText, ) - assert.deepEqual(rpc, { method: 'submitArtifact', token: 'tok', body: 'summary', commit: '9b81c449326f' }) + assert.deepEqual(rpc, { + method: 'submitArtifact', + token: 'tok', + title: 'Dropped the allowlist', + body: 'summary', + commit: '9b81c449326f', + }) }) it('omits commit when --commit is not given (plan-style submit)', async () => { - const rpc = await buildTurnRpc(['artifact', 'submit', '--body', 'plan'], 'tok', readText) + const rpc = await buildTurnRpc(['artifact', 'submit', '--title', 'A plan', '--body', 'plan'], 'tok', readText) assert.equal('commit' in rpc, false) }) it('collects repeated --criterion flags', async () => { const rpc = await buildTurnRpc( - ['artifact', 'submit', '--body', 'hi', '--criterion', 'a', '--criterion', 'b'], + ['artifact', 'submit', '--title', 'A short name', '--body', 'hi', '--criterion', 'a', '--criterion', 'b'], 'tok', readText, ) - assert.deepEqual(rpc, { method: 'submitArtifact', token: 'tok', body: 'hi', criteria: ['a', 'b'] }) + assert.deepEqual(rpc, { + method: 'submitArtifact', + token: 'tok', + title: 'A short name', + body: 'hi', + criteria: ['a', 'b'], + }) }) it('reads --body-file via the supplied reader', async () => { @@ -46,16 +58,59 @@ describe('buildTurnRpc mapping', () => { calls.push(path) return 'from file' } - const rpc = await buildTurnRpc(['artifact', 'submit', '--body-file', '/tmp/body.txt'], 'tok', rt) - assert.deepEqual(rpc, { method: 'submitArtifact', token: 'tok', body: 'from file' }) + const rpc = await buildTurnRpc( + ['artifact', 'submit', '--title', 'A short name', '--body-file', '/tmp/body.txt'], + 'tok', + rt, + ) + assert.deepEqual(rpc, { method: 'submitArtifact', token: 'tok', title: 'A short name', body: 'from file' }) assert.deepEqual(calls, ['/tmp/body.txt']) }) it('rejects when both or neither of --body/--body-file are given', async () => { - await assert.rejects(buildTurnRpc(['artifact', 'submit'], 'tok', readText)) + await assert.rejects(buildTurnRpc(['artifact', 'submit', '--title', 'A short name'], 'tok', readText)) + await assert.rejects( + buildTurnRpc( + ['artifact', 'submit', '--title', 'A short name', '--body', 'a', '--body-file', 'b'], + 'tok', + readText, + ), + ) + }) + + // The title is required of a writer even though the lexicon leaves it optional: the field exists so + // a row has a name to show, and a submit allowed to omit it would go on producing rows that derive + // one from the body. Trimmed, because trimmed is what gets written. + it('requires a non-blank --title, trims it, and refuses one over the lexicon ceiling', async () => { await assert.rejects( - buildTurnRpc(['artifact', 'submit', '--body', 'a', '--body-file', 'b'], 'tok', readText), + buildTurnRpc(['artifact', 'submit', '--body', 'hi'], 'tok', readText), + /Missing --title/, + ) + await assert.rejects( + buildTurnRpc(['artifact', 'submit', '--title', ' ', '--body', 'hi'], 'tok', readText), + /--title cannot be blank/, + ) + await assert.rejects( + buildTurnRpc( + ['artifact', 'submit', '--title', 'x'.repeat(TURN_LIMITS.artifactTitleChars + 1), '--body', 'hi'], + 'tok', + readText, + ), + /at most 200 characters/, + ) + const trimmed = await buildTurnRpc( + ['artifact', 'submit', '--title', ' Padded name \n', '--body', 'hi'], + 'tok', + readText, + ) + assert.equal(trimmed.title, 'Padded name') + // Exactly at the ceiling is valid — the refusal is for what the lexicon cannot carry. + const atLimit = await buildTurnRpc( + ['artifact', 'submit', '--title', 'x'.repeat(TURN_LIMITS.artifactTitleChars), '--body', 'hi'], + 'tok', + readText, ) + assert.equal(atLimit.title.length, TURN_LIMITS.artifactTitleChars) }) it('maps message post to askQuestion', async () => { diff --git a/packages/ui/src/app.css b/packages/ui/src/app.css index b5b83ed..dadb1d0 100644 --- a/packages/ui/src/app.css +++ b/packages/ui/src/app.css @@ -371,6 +371,10 @@ a { color: var(--accent); text-underline-offset: 2px; } .row .name { font-weight: 500; white-space: nowrap; } .row.dead .name, .row.dead .sub { color: var(--ink-2); } .row .sub { color: var(--ink-2); min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; flex: 1; } +/* An artifact's own short title, as opposed to a line derived from its body or a word about what an + agent is doing. It is the record's name, so it reads as text rather than as metadata — and it still + ellipsises in the same cell, because a valid 200-character title must not widen the row. */ +.row .sub.titled { color: var(--ink); } .row .tail { display: flex; align-items: center; gap: 7px; flex: none; } .vchip { font: 600 11px/1 var(--mono); color: var(--ink-2); @@ -423,6 +427,13 @@ a { color: var(--accent); text-underline-offset: 2px; } font-size: 13.5px; line-height: 1.55; color: var(--ink-2); max-width: 64ch; white-space: pre-wrap; } +/* The artifact's own title, over the version being read. Set above the body's own headings — it names + the whole record, and the body's `# ` heading (where it has one) is a heading inside it. It wraps + rather than ellipsising: there is room for a line here, unlike in a row. */ +.atitle { + font-size: 15.5px; font-weight: 600; letter-spacing: -0.004em; + line-height: 1.35; margin: 0 0 8px; max-width: 66ch; color: var(--ink); +} .body { font-size: 13.5px; line-height: 1.62; max-width: 66ch; margin: 0 0 14px; color: var(--ink); } .body p { margin: 0 0 0.9em; } .body p:last-child { margin-bottom: 0; } diff --git a/packages/ui/src/lib/components/AskRow.svelte b/packages/ui/src/lib/components/AskRow.svelte index d7469e1..539db8c 100644 --- a/packages/ui/src/lib/components/AskRow.svelte +++ b/packages/ui/src/lib/components/AskRow.svelte @@ -1,5 +1,5 @@
  • @@ -44,9 +49,9 @@ v{entry.pinned.version.version} {/if} + reader has no way to tell two replies owed on one goal apart. For a review, the artifact. --> - {where}{entry.subject ? ` · ${summarize(entry.subject.value.body, 80)}` : ''} + {where}{entry.subject ? ` · ${summarize(entry.subject.value.body, 80)}` : ''}{what ? ` · ${what}` : ''} {#each askBadges(entry, { now: space.asOf }) as badge (badge.text)} diff --git a/packages/ui/src/lib/components/UnitDetail.svelte b/packages/ui/src/lib/components/UnitDetail.svelte index 3c4b9d0..70347d6 100644 --- a/packages/ui/src/lib/components/UnitDetail.svelte +++ b/packages/ui/src/lib/components/UnitDetail.svelte @@ -7,7 +7,7 @@ ownsRequestDraft, requestDialogId, } from '$lib/compose.svelte.js' - import { shortCommit, stamp } from '$lib/format.js' + import { artifactLabel, shortCommit, stamp } from '$lib/format.js' import { grow } from '$lib/grow.js' import { inline } from '$lib/prose.js' import { @@ -33,6 +33,7 @@ pullRequest, typeLabel, versionLabel, + versionTitle, } from '$lib/units.js' import { messageArgs, wroteReply } from '$lib/replies.js' import { goalHref, projectOf, systemHref, unitHref, type Space } from '$lib/space.js' @@ -69,6 +70,11 @@ ) const version = $derived(unit.versions[index]) const isCurrent = $derived(version !== undefined && version === unit.current) + // The title of the version being READ, not the unit's — a living document can be renamed at v3, and + // browsing back to v1 must show what v1 was actually called. Empty for a version written before the + // field existed, and then no heading is drawn at all: the body follows immediately, which is exactly + // how this drawer has always looked. + const title = $derived(versionTitle(version)) const requestFor = $derived(version?.request ?? unit.openRequest ?? unit.requests[0]) const requester = $derived(requestFor ? space.directory.get(requestFor.did) : undefined) @@ -262,26 +268,36 @@ {:else if claimant} claimed by {claimant.handle ?? claimant.name} · {claimExpiry(unit, space.asOf)} {/if} + {#each basedOn as reference (reference.uri)} {@const named = findVersion(space.index, reference)} + {@const label = named ? artifactLabel(named.version.artifact.value, 80) : ''} based on {#if named}{versionLabel(named.unit, named.version)}{/if} + {#if label}— {label}{/if} {/each} {#if capture && isGoalView(capture.target)} + {@const label = capture.unit.current ? artifactLabel(capture.unit.current.artifact.value, 80) : ''} distilled from {capture.unit.type} + {#if label}— {label}{/if} in {capture.target.target.value.title} {/if} {#each distilled as into (into.unit.key)} {#if !isGoalView(into.target)} + {@const label = into.unit.current ? artifactLabel(into.unit.current.artifact.value, 80) : ''} distilled into {into.unit.type} + {#if label}— {label}{/if} {/if} {/each} @@ -383,6 +399,10 @@ {/if} +{#if title} +

    {title}

    +{/if} + {#if version} {/if} diff --git a/packages/ui/src/lib/components/UnitRow.svelte b/packages/ui/src/lib/components/UnitRow.svelte index 74842e0..6d829b8 100644 --- a/packages/ui/src/lib/components/UnitRow.svelte +++ b/packages/ui/src/lib/components/UnitRow.svelte @@ -1,7 +1,16 @@
    @@ -63,7 +83,7 @@ v{unit.current.version} {/if} {/if} - {unitSummary(unit, space.directory, space.asOf)} + {summary} {#if drift > 0} diff --git a/packages/ui/src/lib/format.test.ts b/packages/ui/src/lib/format.test.ts index 1d8135b..b5d940d 100644 --- a/packages/ui/src/lib/format.test.ts +++ b/packages/ui/src/lib/format.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { collectionOf, dayLabel, didOf, relativeTime, rkeyOf, summarize } from './format.js' +import { artifactLabel, collectionOf, dayLabel, didOf, relativeTime, rkeyOf, summarize } from './format.js' const NOW = '2026-07-25T14:18:00Z' @@ -37,8 +37,36 @@ describe('AT URI parts', () => { }) }) +describe('artifactLabel', () => { + const body = '## Context\n\n0004 chose polling. It was right at the time.' + + it('uses the artifact’s own title, whitespace and all', () => { + expect(artifactLabel({ title: ' 0008 · Jetstream first ', body })).toBe('0008 · Jetstream first') + }) + + it("falls back to the body's heading for a record written before titles existed", () => { + // Immutable and un-retitleable, so this is the row a real space keeps showing for years. It is + // display-only — nothing persists it, and nothing treats it as protocol data. + expect(artifactLabel({ body })).toBe('Context') + expect(artifactLabel({ title: ' ', body })).toBe('Context') + }) + + it('uses the derived summary when a legacy body has no heading', () => { + const headingless = '0004 chose polling. It was right at the time.' + expect(artifactLabel({ body: headingless })).toBe(summarize(headingless)) + }) + + it('truncates a long-but-valid title rather than letting it run the width of a row', () => { + // 200 characters is valid on the wire; a row is narrower than that. + const label = artifactLabel({ title: 'x'.repeat(200), body }, 80) + expect(label.length).toBe(80) + expect(label.endsWith('…')).toBe(true) + }) +}) + describe('summarize', () => { - // An artifact has a body, not a title, so a row's one line has to come out of the prose. + // The legacy fallback: an artifact written before `title` existed has no name of its own, so a + // row's one line has to come out of the prose. it('skips headings and takes the first sentence', () => { expect(summarize('## Approach\n\nAdd a `/queue` route. It filters the index.')).toBe( 'Add a /queue route.', diff --git a/packages/ui/src/lib/format.ts b/packages/ui/src/lib/format.ts index 485b835..4c0073d 100644 --- a/packages/ui/src/lib/format.ts +++ b/packages/ui/src/lib/format.ts @@ -78,12 +78,36 @@ export function collectionOf(uri: string): string { export const shortCommit = (commit: string, length = 10): string => commit.slice(0, length) /** - * The line a row shows for a landed artifact. + * What to CALL a landed artifact anywhere it is named rather than read: its own short title. * - * Artifacts carry a body, not a title — so the row's one line has to come out of the prose. Take - * the first block of running text: not a heading (the body's own title is not a summary of it), - * not a fence or a rule, and not the `Status:` convention an ADR opens with (phase6-ui-plan §3.4 - * keeps that prose rather than making it a field). Then cut it at the first sentence. + * An artifact carries an explicit `title` now, and it is the label wherever there is one. Records + * signed before the field existed cannot be given one — they are immutable, and nothing may rewrite + * them — so those fall back to their body's first Markdown heading, then `summarize(body)` when + * there is no heading. That fallback is display-only: it is derived on the way to the screen, it is + * never written back, and nothing treats it as protocol data. + * + * One helper for every such place, so one screen cannot show the explicit title while the next + * quietly goes on extracting prose from the same record. + */ +export function artifactLabel(artifact: { title?: string; body: string }, limit = 140): string { + const title = artifact.title?.trim() + if (title) return title.length > limit ? `${title.slice(0, limit - 1).trimEnd()}…` : title + const heading = prose(artifact.body).find((block) => block.type === 'heading') + if (heading) { + const text = plain([heading]).replace(/\s+/g, ' ').trim() + if (text) return text.length > limit ? `${text.slice(0, limit - 1).trimEnd()}…` : text + } + return summarize(artifact.body, limit) +} + +/** + * The line a row shows for an artifact that has no title of its own — the legacy fallback behind + * `artifactLabel`, and the shape of every row before the field existed. + * + * The row's one line has to come out of the prose. Take the first block of running text: not a + * heading (the body's own title is not a summary of it), not a fence or a rule, and not the `Status:` + * convention an ADR opens with (phase6-ui-plan §3.4 keeps that prose rather than making it a field). + * Then cut it at the first sentence. */ export function summarize(body: string, limit = 140): string { let paragraph = '' diff --git a/packages/ui/src/lib/units.test.ts b/packages/ui/src/lib/units.test.ts new file mode 100644 index 0000000..3a5e6c3 --- /dev/null +++ b/packages/ui/src/lib/units.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from 'vitest' +import { buildFixtureSpace } from './fixture.js' +import { artifactLabel } from './format.js' +import { projectByName, unitsOf } from './space.js' +import { unitSearchText, unitSummary, versionTitle } from './units.js' + +// What a row CALLS an artifact. The record carries a short `title` now, so the line is the record's +// own name wherever it has one — and a body-derived label wherever it does not, because a signed record +// cannot be given a title after the fact and a real space keeps those for as long as its history lasts. +// The fixture carries both on purpose (`fixture.ts`), which is what makes this testable at all. +const space = buildFixtureSpace() +const project = projectByName(space.index, 'radial-ng') +if (!project) throw new Error('fixture project missing: radial-ng') +const systemUnits = unitsOf(space.index, project) +// The demo's untitled tip is a goal artifact (the ignored-records plan), so the fallback cases are +// asked of goal units: an untitled record's SUCCESSOR can carry a title, which is exactly why ADR +// 0004 does not make an untitled `current` anywhere in System. +const goalUnits = space.index.goals.flatMap((goal) => unitsOf(space.index, goal)) + +describe('versionTitle', () => { + it('answers per version, so a renamed document keeps each version’s own name', () => { + const architecture = systemUnits.find((unit) => unit.type === 'architecture') + expect(architecture).toBeDefined() + const versions = architecture?.versions ?? [] + expect(versions.length).toBeGreaterThan(1) + // The demo renames the architecture document at v3. Browsing back in the drawer has to show what + // v1 was called, not what the tip is called. + expect(versionTitle(versions[0])).toBe('Three processes over one shared fold') + expect(versionTitle(versions.at(-1))).toBe('Four moving parts and the write boundary') + expect(versionTitle(versions[0])).not.toBe(versionTitle(versions.at(-1))) + }) + + it('is empty — never derived — for a version written before titles existed', () => { + // The drawer prints no heading for it: the body is already on screen, and a heading lifted out of + // that body would be its first sentence printed twice. + const adrs = systemUnits.filter((unit) => unit.type === 'adr') + const legacy = adrs.flatMap((unit) => unit.versions).filter((version) => versionTitle(version) === '') + expect(legacy.length).toBeGreaterThan(0) + expect(legacy.every((version) => version.artifact.value.title === undefined)).toBe(true) + }) +}) + +describe('unitSummary', () => { + it('is the landed artifact’s own title', () => { + const adr = systemUnits.find((unit) => versionTitle(unit.current) !== '') + expect(adr).toBeDefined() + if (!adr) return + expect(unitSummary(adr, space.directory, space.asOf)).toBe(versionTitle(adr.current)) + }) + + it('falls back to the body-derived label for an untitled artifact', () => { + const legacy = goalUnits.find( + (unit) => unit.current !== undefined && versionTitle(unit.current) === '' && !unit.openRequest, + ) + expect(legacy).toBeDefined() + if (!legacy?.current) return + expect(unitSummary(legacy, space.directory, space.asOf)).toBe( + artifactLabel(legacy.current.artifact.value), + ) + }) + + it('still says what an agent is doing rather than naming a document, when that is the news', () => { + // A unit with an open request is with somebody; the title of what last landed is not the line. + const moving = systemUnits.find((unit) => unit.state === 'claimed' || unit.state === 'open') + if (!moving) return + expect(unitSummary(moving, space.directory, space.asOf)).not.toBe(versionTitle(moving.current)) + }) +}) + +describe('unitSearchText', () => { + const matching = (query: string) => + systemUnits.filter((unit) => unitSearchText(unit).toLowerCase().includes(query.toLowerCase())) + + it('finds a system record by its short title', () => { + const hits = matching('Jetstream first') + expect(hits.length).toBe(1) + expect(hits[0]?.type).toBe('adr') + }) + + it('keeps the full-text search that existed before titles did', () => { + // A phrase that appears only in a body, and one only in a type name. + expect(matching('demoted to backfill').length).toBe(1) + expect(matching('conventions').length).toBeGreaterThan(0) + }) + + it('does not match a record whose title merely resembles another’s', () => { + expect(matching('Containment by possession').map((unit) => unit.type)).toEqual(['adr']) + expect(matching('no such record anywhere').length).toBe(0) + }) + + it('searches an untitled record by type and body alone', () => { + const legacy = goalUnits.find( + (unit) => unit.current !== undefined && versionTitle(unit.current) === '', + ) + expect(legacy).toBeDefined() + if (!legacy?.current) return + const text = unitSearchText(legacy) + expect(text).toContain(legacy.type) + expect(text).toContain(legacy.current.artifact.value.body) + }) +}) diff --git a/packages/ui/src/lib/units.ts b/packages/ui/src/lib/units.ts index 80ad3ee..9e37dde 100644 --- a/packages/ui/src/lib/units.ts +++ b/packages/ui/src/lib/units.ts @@ -23,7 +23,7 @@ import type { } from '@radial/core' import { activeGoals, activeProjects, artifactTypes, claimDeadline, staleness, timeline } from '@radial/core' import type { Directory } from './directory.js' -import { relativeTime, summarize } from './format.js' +import { artifactLabel, relativeTime } from './format.js' export type BadgeKind = 'ok' | 'bad' | 'warn' | 'flat' | 'accent' @@ -260,10 +260,30 @@ export function unitSummary(unit: UnitView, directory: Directory, now: string): case 'retracted': return 'retracted' default: - return unit.current ? summarize(unit.current.artifact.value.body) : '' + return unit.current ? artifactLabel(unit.current.artifact.value) : '' } } +/** + * The title of one exact VERSION, and only if that version has one. + * + * No fallback here, deliberately: this is what the drawer prints above the body it is already + * showing, and deriving a line from that same body would be printing the body's own first sentence + * twice. A version chain can change title between versions — a living document renamed at v3 — so it + * is asked per version rather than per unit. + */ +export const versionTitle = (version: UnitVersion | undefined): string => + version?.artifact.value.title?.trim() ?? '' + +/** + * What a System row is searched by: the type, the current version's explicit title, and its body. + * + * All three, not the title alone — a short title is what makes a record findable by name, and it must + * not cost the full-text search that was there before it existed. + */ +export const unitSearchText = (unit: UnitView): string => + [unit.type, versionTitle(unit.current), unit.current?.artifact.value.body ?? ''].join(' ') + export function checkTally(version: UnitVersion | undefined): { pass: number; total: number } | undefined { if (!version || version.checkruns.length === 0) return undefined const results = version.checkruns.flatMap((run) => run.value.results) diff --git a/packages/ui/src/routes/p/[project]/system/+page.svelte b/packages/ui/src/routes/p/[project]/system/+page.svelte index 75a0d43..474c01b 100644 --- a/packages/ui/src/routes/p/[project]/system/+page.svelte +++ b/packages/ui/src/routes/p/[project]/system/+page.svelte @@ -7,6 +7,7 @@ import UnitRow from '$lib/components/UnitRow.svelte' import { currentSpace } from '$lib/session.svelte.js' import { projectByName, unitsOf } from '$lib/space.js' + import { unitSearchText } from '$lib/units.js' import { matches, ui } from '$lib/ui.svelte.js' // The project's standing memory (design §8). The current version of each of these rides in every @@ -24,12 +25,10 @@ const byNewest = (left: UnitView, right: UnitView): number => newest(right).localeCompare(newest(left)) || left.key.localeCompare(right.key) + // Searched by type, short title and body together (`unitSearchText`): the title is how a record is + // found by name now, and the body is how it was found before titles existed. const units = $derived( - project - ? unitsOf(space.index, project).filter((unit) => - matches(`${unit.type} ${unit.current?.artifact.value.body ?? ''}`), - ) - : [], + project ? unitsOf(space.index, project).filter((unit) => matches(unitSearchText(unit))) : [], ) const living = $derived( units.filter((unit) => durabilityOf(space.index, unit.type) === 'living').sort(byNewest),