Public: anyone can read this; the crew can change it.
A tell carries the page it was told from: `from` on the tell action, the API and the MCP, shown on the issue #1744
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1625-a-tell-carries-the-page-it-was-told-from → main 3 commits fork 3 threads · 3 unresolved
7 files
changed +187
−5
docs/FEED.md +3 −1
| @@ -88,7 +88,9 @@ a card, and paging past the first 20. | ||
| 88 | 88 | - Browser sessions now honour `Idempotency-Key` on `/api/v1/` mutations, scoped |
| 89 | 89 | to the person (as tokens' are to the token): a replay returns the first |
| 90 | 90 | answer. `POST /api/v1/feed/tells { app, text }` is the JSON tell the outbox |
| 91 | sends. `variants` (an integer, 1 to 4; anything else is a 400 and opens | |
| 91 | sends. `from` (optional) is the page the tell was told from: a path on this | |
| 92 | forge, at most 512 characters; anything else is a 400 `invalid_from` and | |
| 93 | opens nothing. The issue reads "Told from <link>". `variants` (an integer, 1 to 4; anything else is a 400 and opens | |
| 92 | 94 | nothing) opens that many issues from the one tell, each handed to a builder |
| 93 | 95 | run, all in one server-made group their pulls carry (`variant_group`), kept |
| 94 | 96 | when a run is queued again. With more than one, the 201 also carries |
docs/MCP.md +5 −1
| @@ -177,6 +177,10 @@ answers | ||
| 177 | 177 | when the manifest names a route it lacks. |
| 178 | 178 | - **Where a tell came from.** `tell` takes `context`: `page` (http/https), |
| 179 | 179 | `run`, `commit` (hex) and `said` (the person's words). Malformed context is |
| 180 | refused; the rest is appended to the issue body. | |
| 180 | refused; the rest is appended to the issue body. `tell` also takes `from`: a | |
| 181 | same-forge path (at most 512 characters, like `/alice/notes/pulls/3`) that | |
| 182 | becomes the "Told from" link on the issue; a bad one is refused. It differs | |
| 183 | from `context.page`, which is any http(s) address appended as context, not | |
| 184 | linked as the origin. | |
| 181 | 185 | - **Plugins.** `plugins/opencode` and `plugins/claude-code` register this |
| 182 | 186 | server and add a `forge` command (tell, status, decide). They hold no logic. |
scripts/gate.ts +1
| @@ -180,6 +180,7 @@ export const GATE: readonly string[] = [ | ||
| 180 | 180 | "test/forge-cloudflare.test.ts", |
| 181 | 181 | "test/forge-cloudflare-celld.test.ts", |
| 182 | 182 | "test/branch_tell.test.ts", |
| 183 | "test/tell_from.test.ts", | |
| 183 | 184 | "test/usage.test.ts", |
| 184 | 185 | "test/tokens.test.ts", |
| 185 | 186 | "test/credentials.test.ts", |
src/actions/tell.ts +26 −1
| @@ -43,8 +43,30 @@ export function parseVariants(raw: unknown): number | undefined { | ||
| 43 | 43 | return raw >= 1 && raw <= VARIANTS_MAX ? raw : undefined; |
| 44 | 44 | } |
| 45 | 45 | |
| 46 | /** The longest `from` a tell may carry. */ | |
| 47 | export const FROM_MAX = 512; | |
| 48 | ||
| 49 | /** | |
| 50 | * `from` as the doors take it: absent is none; otherwise a same-forge path | |
| 51 | * (leading single `/`, no scheme, no backslash, no control or space) of at | |
| 52 | * most 512 characters. Undefined for no. | |
| 53 | */ | |
| 54 | export function parseFrom(raw: unknown): string | null | undefined { | |
| 55 | if (raw === undefined || raw === null) return null; | |
| 56 | if (typeof raw !== "string" || raw.length === 0 || raw.length > FROM_MAX) | |
| 57 | return undefined; | |
| 58 | if (!raw.startsWith("/") || raw.startsWith("//")) return undefined; | |
| 59 | if (/[\\\s\x00-\x1f\x7f<>()\[\]`"'*]/.test(raw)) return undefined; | |
| 60 | return raw; | |
| 61 | } | |
| 62 | ||
| 63 | /** What a bad `from` is refused with, at every door. */ | |
| 64 | export const FROM_WORDS = | |
| 65 | "from is a path on this forge, like /alice/notes/pulls/3, at most 512 characters"; | |
| 66 | ||
| 46 | 67 | export type TellError = |
| 47 | 68 | | "empty" |
| 69 | | "bad_from" | |
| 48 | 70 | | "not_found" |
| 49 | 71 | | "unauthorized" |
| 50 | 72 | /** Someone who may not direct the crew here (`crew/direct.ts`). */ |
| @@ -61,6 +83,7 @@ export async function tell( | ||
| 61 | 83 | repo: Repo, |
| 62 | 84 | said: string, |
| 63 | 85 | variants = 1, |
| 86 | from: string | null = null, | |
| 64 | 87 | ): Promise<Result<Told, TellError>> { |
| 65 | 88 | // A tell is a run: only someone who may direct the crew here says one, |
| 66 | 89 | // and a refused tell opens nothing (`crew/direct.ts`). |
| @@ -72,6 +95,7 @@ export async function tell( | ||
| 72 | 95 | if (!requireVerified(app, participant.value).ok) return err("unverified"); |
| 73 | 96 | const text = said.trim(); |
| 74 | 97 | if (text === "") return err("empty"); |
| 98 | if (from !== null && parseFrom(from) !== from) return err("bad_from"); | |
| 75 | 99 | const firstLine = text.split(/\n/)[0]!.trim(); |
| 76 | 100 | const title = |
| 77 | 101 | firstLine.length <= 80 ? firstLine : `${firstLine.slice(0, 77).trimEnd()}…`; |
| @@ -90,10 +114,11 @@ export async function tell( | ||
| 90 | 114 | const assignedNumbers: number[] = []; |
| 91 | 115 | for (let k = 1; k <= variants; k++) { |
| 92 | 116 | const note = many ? `Variant ${k} of ${variants}.` : ""; |
| 117 | const origin = from === null ? "" : `Told from [${from}](${from})`; | |
| 93 | 118 | const rest = text === firstLine ? "" : text; |
| 94 | 119 | const opened = openIssue(app, actor, repo, { |
| 95 | 120 | title, |
| 96 | body: [rest, note].filter((s) => s !== "").join("\n\n"), | |
| 121 | body: [rest, note, origin].filter((s) => s !== "").join("\n\n"), | |
| 97 | 122 | }); |
| 98 | 123 | if (!opened.ok) return issues.length === 0 ? opened : ok(told()); |
| 99 | 124 | issues.push(opened.value); |
src/http/feed.ts +5 −1
| @@ -5,6 +5,8 @@ | ||
| 5 | 5 | |
| 6 | 6 | import { |
| 7 | 7 | parseVariants, |
| 8 | parseFrom, | |
| 9 | FROM_WORDS, | |
| 8 | 10 | tell, |
| 9 | 11 | UNVERIFIED_WORDS, |
| 10 | 12 | VARIANTS_MAX, |
| @@ -185,13 +187,15 @@ export function feedRoutes(router: Router<Handler>): void { | ||
| 185 | 187 | 400, |
| 186 | 188 | `variants is a whole number, 1 to ${VARIANTS_MAX}`, |
| 187 | 189 | ); |
| 190 | const from = parseFrom(input.from); | |
This adds |
||
| 191 | if (from === undefined) return apiError("invalid_from", 400, FROM_WORDS); | |
| 188 | 192 | const [owner, name, extra] = str(input.app).split("/"); |
| 189 | 193 | const repo = |
| 190 | 194 | owner && name && extra === undefined |
| 191 | 195 | ? getRepo(ctx.app.db, owner, name) |
| 192 | 196 | : undefined; |
| 193 | 197 | if (!repo || !canRead(ctx.app.db, repo, viewer)) return notFound(); |
| 194 | const told = await tell(ctx.app, viewer, repo, text, variants); | |
| 198 | const told = await tell(ctx.app, viewer, repo, text, variants, from); | |
| 195 | 199 | if (!told.ok) |
| 196 | 200 | return told.error === "forbidden" |
| 197 | 201 | ? apiError("forbidden", 403, DIRECTION_NOTE) |
src/http/mcp.ts +17 −1
| @@ -51,8 +51,14 @@ import { | ||
| 51 | 51 | cardFor, |
| 52 | 52 | } from "../web/studio/cards.ts"; |
| 53 | 53 | import { keepThought } from "../actions/thought.ts"; |
| 54 | import { | |
| 55 | FROM_WORDS, | |
| 56 | parseFrom, | |
| 57 | tell, | |
| 58 | toldWords, | |
| 59 | UNVERIFIED_WORDS, | |
| 60 | } from "../actions/tell.ts"; | |
| 54 | 61 | import { placementJson, placementOf, thoughtAuthor } from "../core/placements.ts"; |
| 55 | import { tell, toldWords, UNVERIFIED_WORDS } from "../actions/tell.ts"; | |
| 56 | 62 | import { readReturns, reportSpend } from "../actions/returns.ts"; |
| 57 | 63 | import { |
| 58 | 64 | readUsage, |
| @@ -306,6 +312,12 @@ const TOOLS: readonly Tool[] = [ | ||
| 306 | 312 | properties: { |
| 307 | 313 | repo: REPO_ARG, |
| 308 | 314 | text: { type: "string", maxLength: 8000 }, |
outdated
|
||
| 315 | from: { | |
| 316 | type: "string", | |
| 317 | maxLength: 512, | |
| 318 | description: | |
| 319 | "The page this was told from: a path on this forge, like /alice/notes/pulls/3. The issue links to it.", | |
| 320 | }, | |
| 309 | 321 | context: { |
| 310 | 322 | type: "object", |
| 311 | 323 | description: |
| @@ -339,6 +351,8 @@ const TOOLS: readonly Tool[] = [ | ||
| 339 | 351 | "invalid", |
| 340 | 352 | "context is { page?: http(s) address, run?: id, commit?: hex, said?: words }", |
| 341 | 353 | ); |
| 354 | const from = parseFrom(args["from"]); | |
| 355 | if (from === undefined) return bad("invalid", FROM_WORDS); | |
| 342 | 356 | const told = await tell( |
| 343 | 357 | ctx.app, |
| 344 | 358 | user, |
| @@ -346,6 +360,8 @@ const TOOLS: readonly Tool[] = [ | ||
| 346 | 360 | str(args["text"]) === "" |
| 347 | 361 | ? "" |
| 348 | 362 | : `${str(args["text"])}${contextNote(where)}`, |
| 363 | 1, | |
| 364 | from, | |
| 349 | 365 | ); |
| 350 | 366 | if (!told.ok) |
| 351 | 367 | return bad( |
test/tell_from.test.ts added +130
| @@ -0,0 +1,130 @@ | ||
| 1 | /** A tell carries the page it was told from (#1625). */ | |
| 2 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { setIssueState } from "../src/actions/issues.ts"; | |
| 8 | import { createRepo } from "../src/actions/repos.ts"; | |
| 9 | import { parseFrom, tell } from "../src/actions/tell.ts"; | |
| 10 | import { getIssue } from "../src/core/issues.ts"; | |
| 11 | import type { Repo } from "../src/core/repos.ts"; | |
| 12 | import { createToken } from "../src/core/tokens.ts"; | |
| 13 | import { createUser, type User } from "../src/core/users.ts"; | |
| 14 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 15 | ||
| 16 | let dir: string; | |
| 17 | let app: App; | |
| 18 | let servers: Servers; | |
| 19 | let alice: User; | |
| 20 | let repo: Repo; | |
| 21 | let token: string; | |
| 22 | ||
| 23 | beforeAll(async () => { | |
| 24 | dir = await mkdtemp(join(tmpdir(), "forge-tellfrom-")); | |
| 25 | app = createApp({ dataDir: dir, port: 0 }); | |
| 26 | servers = await serve(app); | |
| 27 | const u = await createUser(app.db, { | |
| 28 | trusted: true, | |
| 29 | username: "alice", | |
| 30 | email: "[email protected]", | |
| 31 | password: "a-good-passphrase", | |
| 32 | adminIfFirst: true, | |
| 33 | }); | |
| 34 | if (!u.ok) throw new Error(u.error); | |
| 35 | alice = u.value; | |
| 36 | const r = await createRepo(app, alice, { name: "notes", visibility: "public" }); | |
| 37 | if (!r.ok) throw new Error(String(r.error)); | |
| 38 | repo = r.value; | |
| 39 | token = createToken(app.db, alice.id, "rw", "rw").token; | |
| 40 | }); | |
| 41 | ||
| 42 | afterAll(async () => { | |
| 43 | await servers.stop(); | |
| 44 | await rm(dir, { recursive: true, force: true }); | |
| 45 | }); | |
| 46 | ||
| 47 | /** Close what a test opened, so nothing is left asking. */ | |
| 48 | function close(numbers: readonly number[]): void { | |
| 49 | for (const n of numbers) { | |
| 50 | const i = getIssue(app.db, repo.id, n); | |
| 51 | if (i) setIssueState(app, alice, repo, i, "closed"); | |
| 52 | } | |
| 53 | } | |
| 54 | ||
| 55 | describe("parseFrom", () => { | |
| 56 | test("takes same-forge paths and nothing else", () => { | |
| 57 | expect(parseFrom(undefined)).toBeNull(); | |
| 58 | expect(parseFrom("/alice/notes/pulls/3")).toBe("/alice/notes/pulls/3"); | |
| 59 | for (const bad of [ | |
| 60 | "https://evil.example/x", | |
| 61 | "//evil.example", | |
| 62 | "javascript:alert(1)", | |
| 63 | "alice/notes", | |
| 64 | "", | |
| 65 | "/a b", | |
| 66 | "/a)(b", | |
| 67 | "/*x*", | |
| 68 | "/a\\b", | |
| 69 | "/" + "x".repeat(512), | |
| 70 | 42, | |
| 71 | ]) | |
| 72 | expect(parseFrom(bad)).toBeUndefined(); | |
| 73 | expect(parseFrom("/" + "x".repeat(511))).toBeDefined(); | |
| 74 | }); | |
| 75 | }); | |
| 76 | ||
| 77 | describe("tell with from", () => { | |
| 78 | test("the issue says where it was told from", async () => { | |
| 79 | const t = await tell(app, alice, repo, "Make it purple", 1, "/alice/notes/pulls/3"); | |
| 80 | expect(t.ok).toBe(true); | |
| 81 | if (!t.ok) return; | |
| 82 | expect(t.value.issue.body).toContain("Told from [/alice/notes/pulls/3](/alice/notes/pulls/3)"); | |
| 83 | close([t.value.issue.number]); | |
| 84 | }); | |
| 85 | test("a bad from opens nothing", async () => { | |
| 86 | const t = await tell(app, alice, repo, "Hi", 1, "https://evil.example"); | |
| 87 | expect(t.ok).toBe(false); | |
| 88 | }); | |
| 89 | test("the API refuses a bad from and accepts a good one", async () => { | |
| 90 | const post = (from: unknown) => | |
| 91 | fetch(`${servers.url}/api/v1/feed/tells`, { | |
| 92 | method: "POST", | |
| 93 | headers: { authorization: `Bearer ${token}`, "content-type": "application/json" }, | |
| 94 | body: JSON.stringify({ app: "alice/notes", text: "Via API", from }), | |
| 95 | }); | |
| 96 | const bad = await post("https://evil.example/"); | |
| 97 | expect(bad.status).toBe(400); | |
| 98 | expect(((await bad.json()) as { error: string }).error).toBe("invalid_from"); | |
| 99 | const good = await post("/alice/notes"); | |
| 100 | expect(good.status).toBe(201); | |
| 101 | close([((await good.json()) as { issue: number }).issue]); | |
| 102 | }); | |
| 103 | test("the MCP tool refuses a bad from and accepts a good one", async () => { | |
| 104 | const call = async (from: string) => { | |
| 105 | const res = await fetch(`${servers.url}/mcp`, { | |
| 106 | method: "POST", | |
| 107 | headers: { | |
| 108 | authorization: `Bearer ${token}`, | |
| 109 | "content-type": "application/json", | |
| 110 | accept: "application/json, text/event-stream", | |
| 111 | }, | |
| 112 | body: JSON.stringify({ | |
| 113 | jsonrpc: "2.0", | |
| 114 | id: 1, | |
| 115 | method: "tools/call", | |
| 116 | params: { name: "tell", arguments: { repo: "alice/notes", text: "Via MCP", from } }, | |
| 117 | }), | |
| 118 | }); | |
| 119 | return (await res.json()) as { | |
| 120 | result: { isError?: boolean; content: { text: string }[] }; | |
| 121 | }; | |
| 122 | }; | |
| 123 | const bad = await call("http://evil.example"); | |
| 124 | expect(bad.result.isError).toBe(true); | |
| 125 | expect(bad.result.content[0]!.text).toContain("from is a path"); | |
| 126 | const good = await call("/alice/notes/issues"); | |
| 127 | expect(good.result.isError).toBeFalsy(); | |
| 128 | close([(JSON.parse(good.result.content[0]!.text) as { issue: number }).issue]); | |
| 129 | }); | |
| 130 | }); |
new above
offline
Low severity, not blocking:
passes this check.markdown.tsstyles emphasis before it builds links, so a from like/x*turns into<em>inside both the link text and the href. Escaping keeps that safe, but the link comes out wrong. Consider refusing*here, or keeping the path out of inline styling.