Public: anyone can read this; the crew can change it.
The forge toolbar in a preview carries the pull as context into a tell #11425
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/11423-the-forge-toolbar-in-a-preview-carries-t → main 4 commits fork 1 thread · 1 unresolved
10 files
changed +108
−13
docs/features/toolbar-tell.md added +16
| @@ -0,0 +1,16 @@ | ||
| 1 | # A tell from a preview's toolbar carries its pull | |
| 2 | ||
| 3 | A tell sent from the forge toolbar of an app or a preview arrives with where it was said. | |
| 4 | On a preview the issue's "Where this was told from" block opens with one line, | |
| 5 | `from preview of #N, page /x`, in place of the plain page and pull lines. | |
| 6 | ||
| 7 | - **Which doors say it.** The toolbar's tell form (`tellFromPage`, `src/web/studio/actions.ts`) | |
| 8 | and the terminal's "Tell the crew from a page" (`src/tui/screens/tell.ts`), both of which ask for | |
| 9 | a preview's pull. They pass `{ preview: true }` to `contextNote`. The MCP `tell` tool and the | |
| 10 | API keep the plain `- page:` and `- pull:` lines: they never claim a preview. | |
| 11 | - **Checked at the boundary.** Page and pull go through `parseContext`; the line is built by | |
| 12 | `previewLine` (`src/actions/tell_from.ts`) from the page's path alone (no host, query or | |
| 13 | fragment), every markup character percent-encoded. It is context, never authority; the | |
| 14 | same-origin check on the POST is unchanged. | |
| 15 | - Tests: `test/apps-toolbar-tell.test.ts`, `test/tui-tell.test.ts`, `test/tell-context.test.ts`, | |
| 16 | `test/tell_from.test.ts`. |
src/actions/tell_context.ts +14 −4
| @@ -6,6 +6,8 @@ | ||
| 6 | 6 | * the tell as a plain block the issue shows and the agent reads. |
| 7 | 7 | */ |
| 8 | 8 | |
| 9 | import { previewLine } from "./tell_from.ts"; | |
| 10 | ||
| 9 | 11 | export interface TellContext { |
| 10 | 12 | readonly page?: string; |
| 11 | 13 | readonly pull?: number; |
| @@ -124,12 +126,20 @@ export function parseContext(v: unknown): TellContext | undefined | null { | ||
| 124 | 126 | return out; |
| 125 | 127 | } |
| 126 | 128 | |
| 127 | /** The block appended to a tell; empty when there is nothing to say. */ | |
| 128 | export function contextNote(c: TellContext | undefined): string { | |
| 129 | /** | |
| 130 | * The block appended to a tell; empty when there is nothing to say. `preview` | |
| 131 | * is said only by a door that asked for a preview's pull (the forge toolbar, | |
| 132 | * the terminal's "from a page"); then one line, "from preview of #N, page | |
| 133 | * /x", takes the place of the page and pull lines. Any other door (MCP, API) | |
| 134 | * keeps the plain page and pull, so no door claims a preview it did not see. | |
| 135 | */ | |
| 136 | export function contextNote(c: TellContext | undefined, opts: { preview?: boolean } = {}): string { | |
| 129 | 137 | if (c === undefined) return ""; |
| 138 | const preview = opts.preview === true ? previewLine(c.pull, c.page) : ""; | |
| 130 | 139 | const lines = [ |
| 131 | c.page === undefined ? "" : `- page: ${c.page}`, | |
| 132 | c.pull === undefined ? "" : `- pull: #${c.pull}`, | |
| 140 | preview !== "" ? `- ${preview}` : "", | |
| 141 | preview !== "" || c.page === undefined ? "" : `- page: ${c.page}`, | |
| 142 | preview !== "" || c.pull === undefined ? "" : `- pull: #${c.pull}`, | |
| 133 | 143 | c.run === undefined ? "" : `- run: ${c.run}`, |
| 134 | 144 | c.commit === undefined ? "" : `- commit: ${c.commit}`, |
| 135 | 145 | c.said === undefined || c.said === "" |
src/actions/tell_from.ts +20
| @@ -29,3 +29,23 @@ export function parseFrom(raw: unknown): string | null | undefined { | ||
| 29 | 29 | const end = h === -1 ? raw.length : h; |
| 30 | 30 | return `${raw.slice(0, q)}${scrubQuery(raw.slice(q, end))}${raw.slice(end)}`; |
| 31 | 31 | } |
| 32 | ||
| 33 | /** | |
| 34 | * The plain line a tell from a preview's toolbar opens its issue with: | |
| 35 | * "from preview of #N, page /x". The path is only the path of the page the | |
| 36 | * person was on (never its host, query or fragment), with every character | |
| 37 | * `parseFrom` refuses percent-encoded, so it cannot become markup. It is | |
| 38 | * context for a reader, never authority: nothing is granted by it. | |
| 39 | * Empty when there is no pull or no usable page. | |
| 40 | */ | |
| 41 | export function previewLine(pull: number | undefined, page: string | undefined): string { | |
| 42 | if (pull === undefined || page === undefined) return ""; | |
| 43 | const url = URL.parse(page); | |
| 44 | if (url === null) return ""; | |
| 45 | const path = url.pathname.replace( | |
| 46 | /[\\\s\x00-\x1f\x7f<>()\[\]`"'*_~|!#&]/g, | |
| 47 | (c) => `%${c.charCodeAt(0).toString(16).toUpperCase().padStart(2, "0")}`, | |
| 48 | ); | |
| 49 | if (parseFrom(path) !== path) return ""; | |
| 50 | return `from preview of #${pull}, page ${path}`; | |
| 51 | } |
src/tui/screens/tell.ts +1 −1
| @@ -116,7 +116,7 @@ export async function tellFromPage( | ||
| 116 | 116 | ctx.toast(NOT_TOLD_FROM_WORDS, "danger"); |
| 117 | 117 | return; |
| 118 | 118 | } |
| 119 | const answer = tellAnswer(await tell(f.app, f.user, repo, `${said}${contextNote(where)}`)); | |
| 119 | const answer = tellAnswer(await tell(f.app, f.user, repo, `${said}${contextNote(where, { preview: true })}`)); | |
| 120 | 120 | ctx.toast(answer.text, answer.tone); |
| 121 | 121 | } |
| 122 | 122 |
src/web/studio/actions.ts +1 −1
| @@ -174,7 +174,7 @@ async function tellFromPage(ctx: Ctx, repo: Repo, form: Form): Promise<Response> | ||
| 174 | 174 | return flashTo(ctx, back, { tone: "danger", message: NOT_TOLD_FROM_WORDS }); |
| 175 | 175 | const text = field(form, "text").trim(); |
| 176 | 176 | if (text === "") return flashTo(ctx, back, { tone: "danger", message: SAY_SOMETHING_WORDS }); |
| 177 | const told = await tell(ctx.app, ctx.auth.user, repo, `${text}${contextNote(where)}`); | |
| 177 | const told = await tell(ctx.app, ctx.auth.user, repo, `${text}${contextNote(where, { preview: true })}`); | |
| 178 | 178 | if (!told.ok) |
| 179 | 179 | return flashTo(ctx, back, { |
| 180 | 180 | tone: "danger", |
test/apps-toolbar-tell.test.ts +18 −2
| @@ -170,8 +170,24 @@ describe("tell the crew from the pill", () => { | ||
| 170 | 170 | }); |
| 171 | 171 | expect(res.status).toBe(200); |
| 172 | 172 | const issue = listIssues(app.db, repoId, {}).find((i) => i.title === "Bigger button")!; |
| 173 | expect(issue.body).toContain("- page: https://notes-pull-3.apps.test/b"); | |
| 174 | expect(issue.body).toContain("- pull: #3"); | |
| 173 | expect(issue.body).toContain("- from preview of #3, page /b"); | |
| 174 | expect(issue.body).not.toContain("- page:"); | |
| 175 | expect(issue.body).not.toContain("- pull:"); | |
| 176 | }); | |
| 177 | ||
| 178 | test("a hostile page path is encoded in the filed line, and a hostile pull is refused", async () => { | |
| 179 | const res = await post({ | |
| 180 | text: "Hostile path", | |
| 181 | page: "https://notes-pull-3.apps.test/x)[evil](https://e.test)*<b>", | |
| 182 | pull: "3", | |
| 183 | }); | |
| 184 | expect(res.status).toBe(200); | |
| 185 | const issue = listIssues(app.db, repoId, {}).find((i) => i.title === "Hostile path")!; | |
| 186 | const line = issue.body.split("\n").find((l) => l.startsWith("- from preview"))!; | |
| 187 | expect(line).not.toMatch(/[()\[\]*<>]/); | |
| 188 | const before = told().length; | |
| 189 | expect((await post({ text: "x", page: "https://a.test/", pull: "3; drop" })).status).toBe(303); | |
| 190 | expect(told().length).toBe(before); | |
| 175 | 191 | }); |
| 176 | 192 | |
| 177 | 193 | test("a bad page or pull opens nothing", async () => { |
test/tell-context.test.ts +14
| @@ -12,3 +12,17 @@ test("a page without a fragment is kept as it was", () => { | ||
| 12 | 12 | page: "https://app.example.test/a", |
| 13 | 13 | }); |
| 14 | 14 | }); |
| 15 | ||
| 16 | test("only a door that asked for a preview's pull says 'from preview'", () => { | |
| 17 | const where = parseContext({ page: "https://n.apps.test/a?x=1", pull: 7 }) ?? undefined; | |
| 18 | const plain = contextNote(where); | |
| 19 | expect(plain).not.toContain("from preview"); | |
| 20 | expect(plain).toContain("- page: https://n.apps.test/a?x=1"); | |
| 21 | expect(plain).toContain("- pull: #7"); | |
| 22 | const said = contextNote(where, { preview: true }); | |
| 23 | expect(said).toContain("- from preview of #7, page /a"); | |
| 24 | expect(said).not.toContain("- page:"); | |
| 25 | expect(said).not.toContain("- pull:"); | |
| 26 | // A preview with no pull is no preview. | |
| 27 | expect(contextNote(parseContext({ page: "https://n.apps.test/a" }) ?? undefined, { preview: true })).toContain("- page:"); | |
| 28 | }); |
test/tell_from.test.ts +21 −1
| @@ -6,7 +6,7 @@ import { join } from "node:path"; | ||
| 6 | 6 | import { createApp, type App } from "../src/app.ts"; |
| 7 | 7 | import { setIssueState } from "../src/actions/issues.ts"; |
| 8 | 8 | import { createRepo } from "../src/actions/repos.ts"; |
| 9 | import { parseFrom } from "../src/actions/tell_from.ts"; | |
| 9 | import { parseFrom, previewLine } from "../src/actions/tell_from.ts"; | |
| 10 | 10 | import { tell } from "../src/actions/tell.ts"; |
| 11 | 11 | import { createApp as insertApp } from "../src/core/apps.ts"; |
| 12 | 12 | import { getIssue, listIssues } from "../src/core/issues.ts"; |
| @@ -177,3 +177,23 @@ describe("tell from a page, through the box", () => { | ||
| 177 | 177 | close(listIssues(app.db, repo.id, {}).map((i) => i.number)); |
| 178 | 178 | }); |
| 179 | 179 | }); |
| 180 | ||
| 181 | describe("previewLine", () => { | |
| 182 | test("says the pull and the path, nothing else", () => { | |
| 183 | expect(previewLine(7, "https://n-pull-7.apps.test/a/b?token=x#h")).toBe("from preview of #7, page /a/b"); | |
| 184 | }); | |
| 185 | test("no pull or no page is no line", () => { | |
| 186 | expect(previewLine(undefined, "https://a.test/")).toBe(""); | |
| 187 | expect(previewLine(3, undefined)).toBe(""); | |
| 188 | expect(previewLine(3, "not a url")).toBe(""); | |
| 189 | }); | |
| 190 | test("a hostile path cannot carry markup", () => { | |
| 191 | const l = previewLine(3, "https://a.test/x)[y](http://e.test)*_`<i>"); | |
| 192 | expect(l).not.toMatch(/[()\[\]*`<>]/); | |
| 193 | expect(l.startsWith("from preview of #3, page /x%29")).toBe(true); | |
| 194 | }); | |
| 195 | test("a hostile from value is refused", () => { | |
| 196 | expect(parseFrom("//evil.test")).toBeUndefined(); | |
| 197 | expect(parseFrom("/a)[b](c)")).toBeUndefined(); | |
| 198 | }); | |
| 199 | }); |
test/toolbar-actions.test.ts +2 −2
| @@ -282,8 +282,8 @@ describe("toolbar actions", () => { | ||
| 282 | 282 | pull: String(pullNumber), |
| 283 | 283 | }); |
| 284 | 284 | const two = listIssues(app.db, repoId, {}).find((i) => i.title === "Page tell preview")!; |
| 285 | expect(two.body).toContain("- page: https://notes.example.test/b"); | |
| 286 | expect(two.body).toContain(`- pull: #${pullNumber}`); | |
| 285 | expect(two.body).toContain(`- from preview of #${pullNumber}, page /b`); | |
| 286 | expect(two.body).not.toContain("- pull:"); | |
| 287 | 287 | }); |
| 288 | 288 | |
| 289 | 289 | test("an off-origin or over-long page is dropped from the form, and escaped", async () => { |
test/tui-tell.test.ts +1 −2
| @@ -93,8 +93,7 @@ describe("tell the crew in the terminal", () => { | ||
| 93 | 93 | const text = await palette(alice, keys); |
| 94 | 94 | expect(text).toContain("Nobody is on the crew to take it yet"); |
| 95 | 95 | const issue = listIssues(app.db, repo.id, {}).find((i) => i.title === "move the button")!; |
| 96 | expect(issue.body).toContain("- page: https://notes.apps.test/a"); | |
| 97 | expect(issue.body).toContain("- pull: #7"); | |
| 96 | expect(issue.body).toContain("- from preview of #7, page /a"); | |
| 98 | 97 | expect(await palette(bob, [":", "from a page"])).not.toContain("Tell the crew from a page"); |
| 99 | 98 | }); |
| 100 | 99 |
Calls previewLine twice, and claims "preview" for any tell carrying pull and page, from any door. Compute once and gate it on a real preview signal. It also duplicates the page and pull lines below.