diff --git a/packages/client/src/contract.ts b/packages/client/src/contract.ts index 2808fc86b4..65666f7289 100644 --- a/packages/client/src/contract.ts +++ b/packages/client/src/contract.ts @@ -33,6 +33,7 @@ export const groupNames = { "server.event": "events", "server.pty": "ptys", "server.question": "questions", + "server.browser": "browserRequests", "server.reference": "references", "server.projectCopy": "projectCopies", } as const @@ -48,6 +49,9 @@ export const endpointNames = { "permission.saved.list": "listSaved", "permission.saved.remove": "removeSaved", "question.request.list": "listRequests", + // PA-10: both browser endpoints end in ".list", and the client method name is the last + // dot segment, so the location-wide one needs an explicit name or generation fails. + "browser.request.list": "listRequests", } as const export const omitEndpoints = new Set(["fs.read", "pty.connect", "pty.connectToken"]) diff --git a/packages/client/src/generated-effect/client.ts b/packages/client/src/generated-effect/client.ts index 77b6b52a9d..84a4787678 100644 --- a/packages/client/src/generated-effect/client.ts +++ b/packages/client/src/generated-effect/client.ts @@ -652,78 +652,122 @@ const adaptGroup16 = (raw: RawClient["server.question"]) => ({ reject: Endpoint16_3(raw), }) -type Endpoint17_0Request = Parameters[0] +type Endpoint17_0Request = Parameters[0] type Endpoint17_0Input = { readonly location?: Endpoint17_0Request["query"]["location"] } -const Endpoint17_0 = (raw: RawClient["server.reference"]) => (input?: Endpoint17_0Input) => +const Endpoint17_0 = (raw: RawClient["server.browser"]) => (input?: Endpoint17_0Input) => + raw["browser.request.list"]({ query: { location: input?.["location"] } }).pipe(Effect.mapError(mapClientError)) + +type Endpoint17_1Request = Parameters[0] +type Endpoint17_1Input = { readonly sessionID: Endpoint17_1Request["params"]["sessionID"] } +const Endpoint17_1 = (raw: RawClient["server.browser"]) => (input: Endpoint17_1Input) => + raw["session.browser.list"]({ params: { sessionID: input["sessionID"] } }).pipe( + Effect.mapError(mapClientError), + Effect.map((value) => value.data), + ) + +type Endpoint17_2Request = Parameters[0] +type Endpoint17_2Input = { + readonly sessionID: Endpoint17_2Request["params"]["sessionID"] + readonly requestID: Endpoint17_2Request["params"]["requestID"] + readonly value: Endpoint17_2Request["payload"]["value"] +} +const Endpoint17_2 = (raw: RawClient["server.browser"]) => (input: Endpoint17_2Input) => + raw["session.browser.reply"]({ + params: { sessionID: input["sessionID"], requestID: input["requestID"] }, + payload: { value: input["value"] }, + }).pipe(Effect.mapError(mapClientError)) + +type Endpoint17_3Request = Parameters[0] +type Endpoint17_3Input = { + readonly sessionID: Endpoint17_3Request["params"]["sessionID"] + readonly requestID: Endpoint17_3Request["params"]["requestID"] + readonly reason: Endpoint17_3Request["payload"]["reason"] +} +const Endpoint17_3 = (raw: RawClient["server.browser"]) => (input: Endpoint17_3Input) => + raw["session.browser.refuse"]({ + params: { sessionID: input["sessionID"], requestID: input["requestID"] }, + payload: { reason: input["reason"] }, + }).pipe(Effect.mapError(mapClientError)) + +const adaptGroup17 = (raw: RawClient["server.browser"]) => ({ + listRequests: Endpoint17_0(raw), + list: Endpoint17_1(raw), + reply: Endpoint17_2(raw), + refuse: Endpoint17_3(raw), +}) + +type Endpoint18_0Request = Parameters[0] +type Endpoint18_0Input = { readonly location?: Endpoint18_0Request["query"]["location"] } +const Endpoint18_0 = (raw: RawClient["server.reference"]) => (input?: Endpoint18_0Input) => raw["reference.list"]({ query: { location: input?.["location"] } }).pipe(Effect.mapError(mapClientError)) -const adaptGroup17 = (raw: RawClient["server.reference"]) => ({ list: Endpoint17_0(raw) }) +const adaptGroup18 = (raw: RawClient["server.reference"]) => ({ list: Endpoint18_0(raw) }) -type Endpoint18_0Request = Parameters[0] -type Endpoint18_0Input = { - readonly projectID: Endpoint18_0Request["params"]["projectID"] - readonly location?: Endpoint18_0Request["query"]["location"] - readonly strategy: Endpoint18_0Request["payload"]["strategy"] - readonly directory: Endpoint18_0Request["payload"]["directory"] - readonly name?: Endpoint18_0Request["payload"]["name"] +type Endpoint19_0Request = Parameters[0] +type Endpoint19_0Input = { + readonly projectID: Endpoint19_0Request["params"]["projectID"] + readonly location?: Endpoint19_0Request["query"]["location"] + readonly strategy: Endpoint19_0Request["payload"]["strategy"] + readonly directory: Endpoint19_0Request["payload"]["directory"] + readonly name?: Endpoint19_0Request["payload"]["name"] } -const Endpoint18_0 = (raw: RawClient["server.projectCopy"]) => (input: Endpoint18_0Input) => +const Endpoint19_0 = (raw: RawClient["server.projectCopy"]) => (input: Endpoint19_0Input) => raw["projectCopy.create"]({ params: { projectID: input["projectID"] }, query: { location: input["location"] }, payload: { strategy: input["strategy"], directory: input["directory"], name: input["name"] }, }).pipe(Effect.mapError(mapClientError)) -type Endpoint18_1Request = Parameters[0] -type Endpoint18_1Input = { - readonly projectID: Endpoint18_1Request["params"]["projectID"] - readonly location?: Endpoint18_1Request["query"]["location"] - readonly directory: Endpoint18_1Request["payload"]["directory"] - readonly force: Endpoint18_1Request["payload"]["force"] +type Endpoint19_1Request = Parameters[0] +type Endpoint19_1Input = { + readonly projectID: Endpoint19_1Request["params"]["projectID"] + readonly location?: Endpoint19_1Request["query"]["location"] + readonly directory: Endpoint19_1Request["payload"]["directory"] + readonly force: Endpoint19_1Request["payload"]["force"] } -const Endpoint18_1 = (raw: RawClient["server.projectCopy"]) => (input: Endpoint18_1Input) => +const Endpoint19_1 = (raw: RawClient["server.projectCopy"]) => (input: Endpoint19_1Input) => raw["projectCopy.remove"]({ params: { projectID: input["projectID"] }, query: { location: input["location"] }, payload: { directory: input["directory"], force: input["force"] }, }).pipe(Effect.mapError(mapClientError)) -type Endpoint18_2Request = Parameters[0] -type Endpoint18_2Input = { - readonly projectID: Endpoint18_2Request["params"]["projectID"] - readonly location?: Endpoint18_2Request["query"]["location"] +type Endpoint19_2Request = Parameters[0] +type Endpoint19_2Input = { + readonly projectID: Endpoint19_2Request["params"]["projectID"] + readonly location?: Endpoint19_2Request["query"]["location"] } -const Endpoint18_2 = (raw: RawClient["server.projectCopy"]) => (input: Endpoint18_2Input) => +const Endpoint19_2 = (raw: RawClient["server.projectCopy"]) => (input: Endpoint19_2Input) => raw["projectCopy.refresh"]({ params: { projectID: input["projectID"] }, query: { location: input["location"] }, }).pipe(Effect.mapError(mapClientError)) -const adaptGroup18 = (raw: RawClient["server.projectCopy"]) => ({ - create: Endpoint18_0(raw), - remove: Endpoint18_1(raw), - refresh: Endpoint18_2(raw), +const adaptGroup19 = (raw: RawClient["server.projectCopy"]) => ({ + create: Endpoint19_0(raw), + remove: Endpoint19_1(raw), + refresh: Endpoint19_2(raw), }) -type Endpoint19_0Request = Parameters[0] -type Endpoint19_0Input = { - readonly model: Endpoint19_0Request["payload"]["model"] - readonly messages: Endpoint19_0Request["payload"]["messages"] - readonly tools?: Endpoint19_0Request["payload"]["tools"] - readonly tool_choice?: Endpoint19_0Request["payload"]["tool_choice"] - readonly stream?: Endpoint19_0Request["payload"]["stream"] - readonly max_tokens?: Endpoint19_0Request["payload"]["max_tokens"] - readonly max_completion_tokens?: Endpoint19_0Request["payload"]["max_completion_tokens"] - readonly temperature?: Endpoint19_0Request["payload"]["temperature"] - readonly top_p?: Endpoint19_0Request["payload"]["top_p"] - readonly stop?: Endpoint19_0Request["payload"]["stop"] - readonly seed?: Endpoint19_0Request["payload"]["seed"] - readonly frequency_penalty?: Endpoint19_0Request["payload"]["frequency_penalty"] - readonly presence_penalty?: Endpoint19_0Request["payload"]["presence_penalty"] - readonly reasoning_effort?: Endpoint19_0Request["payload"]["reasoning_effort"] - readonly user?: Endpoint19_0Request["payload"]["user"] -} -const Endpoint19_0 = (raw: RawClient["server.chat"]) => (input: Endpoint19_0Input) => +type Endpoint20_0Request = Parameters[0] +type Endpoint20_0Input = { + readonly model: Endpoint20_0Request["payload"]["model"] + readonly messages: Endpoint20_0Request["payload"]["messages"] + readonly tools?: Endpoint20_0Request["payload"]["tools"] + readonly tool_choice?: Endpoint20_0Request["payload"]["tool_choice"] + readonly stream?: Endpoint20_0Request["payload"]["stream"] + readonly max_tokens?: Endpoint20_0Request["payload"]["max_tokens"] + readonly max_completion_tokens?: Endpoint20_0Request["payload"]["max_completion_tokens"] + readonly temperature?: Endpoint20_0Request["payload"]["temperature"] + readonly top_p?: Endpoint20_0Request["payload"]["top_p"] + readonly stop?: Endpoint20_0Request["payload"]["stop"] + readonly seed?: Endpoint20_0Request["payload"]["seed"] + readonly frequency_penalty?: Endpoint20_0Request["payload"]["frequency_penalty"] + readonly presence_penalty?: Endpoint20_0Request["payload"]["presence_penalty"] + readonly reasoning_effort?: Endpoint20_0Request["payload"]["reasoning_effort"] + readonly user?: Endpoint20_0Request["payload"]["user"] +} +const Endpoint20_0 = (raw: RawClient["server.chat"]) => (input: Endpoint20_0Input) => raw["chat.completions"]({ payload: { model: input["model"], @@ -744,7 +788,7 @@ const Endpoint19_0 = (raw: RawClient["server.chat"]) => (input: Endpoint19_0Inpu }, }).pipe(Effect.mapError(mapClientError)) -const adaptGroup19 = (raw: RawClient["server.chat"]) => ({ completions: Endpoint19_0(raw) }) +const adaptGroup20 = (raw: RawClient["server.chat"]) => ({ completions: Endpoint20_0(raw) }) const adaptClient = (raw: RawClient) => ({ health: adaptGroup0(raw["server.health"]), @@ -764,9 +808,10 @@ const adaptClient = (raw: RawClient) => ({ events: adaptGroup14(raw["server.event"]), ptys: adaptGroup15(raw["server.pty"]), questions: adaptGroup16(raw["server.question"]), - references: adaptGroup17(raw["server.reference"]), - projectCopies: adaptGroup18(raw["server.projectCopy"]), - "server.chat": adaptGroup19(raw["server.chat"]), + browserRequests: adaptGroup17(raw["server.browser"]), + references: adaptGroup18(raw["server.reference"]), + projectCopies: adaptGroup19(raw["server.projectCopy"]), + "server.chat": adaptGroup20(raw["server.chat"]), }) export const make = (options?: { readonly baseUrl?: URL | string }) => diff --git a/packages/client/src/generated/client.ts b/packages/client/src/generated/client.ts index 211fad5430..860f1fa632 100644 --- a/packages/client/src/generated/client.ts +++ b/packages/client/src/generated/client.ts @@ -108,6 +108,14 @@ import type { QuestionsReplyOutput, QuestionsRejectInput, QuestionsRejectOutput, + BrowserRequestsListRequestsInput, + BrowserRequestsListRequestsOutput, + BrowserRequestsListInput, + BrowserRequestsListOutput, + BrowserRequestsReplyInput, + BrowserRequestsReplyOutput, + BrowserRequestsRefuseInput, + BrowserRequestsRefuseOutput, ReferencesListInput, ReferencesListOutput, ProjectCopiesCreateInput, @@ -965,6 +973,55 @@ export function make(options: ClientOptions) { requestOptions, ), }, + browserRequests: { + listRequests: (input?: BrowserRequestsListRequestsInput, requestOptions?: RequestOptions) => + request( + { + method: "GET", + path: `/api/browser/request`, + query: { location: input?.["location"] }, + successStatus: 200, + declaredStatuses: [401, 400], + empty: false, + }, + requestOptions, + ), + list: (input: BrowserRequestsListInput, requestOptions?: RequestOptions) => + request<{ readonly data: BrowserRequestsListOutput }>( + { + method: "GET", + path: `/api/session/${encodeURIComponent(input.sessionID)}/browser`, + successStatus: 200, + declaredStatuses: [404, 400, 401], + empty: false, + }, + requestOptions, + ).then((value) => value.data), + reply: (input: BrowserRequestsReplyInput, requestOptions?: RequestOptions) => + request( + { + method: "POST", + path: `/api/session/${encodeURIComponent(input.sessionID)}/browser/${encodeURIComponent(input.requestID)}/reply`, + body: { value: input["value"] }, + successStatus: 204, + declaredStatuses: [404, 400, 401], + empty: true, + }, + requestOptions, + ), + refuse: (input: BrowserRequestsRefuseInput, requestOptions?: RequestOptions) => + request( + { + method: "POST", + path: `/api/session/${encodeURIComponent(input.sessionID)}/browser/${encodeURIComponent(input.requestID)}/refuse`, + body: { reason: input["reason"] }, + successStatus: 204, + declaredStatuses: [404, 400, 401], + empty: true, + }, + requestOptions, + ), + }, references: { list: (input?: ReferencesListInput, requestOptions?: RequestOptions) => request( diff --git a/packages/client/src/generated/types.ts b/packages/client/src/generated/types.ts index 7cc6071d18..09fcd1fc85 100644 --- a/packages/client/src/generated/types.ts +++ b/packages/client/src/generated/types.ts @@ -94,6 +94,14 @@ export type QuestionNotFoundError = { export const isQuestionNotFoundError = (value: unknown): value is QuestionNotFoundError => typeof value === "object" && value !== null && "_tag" in value && value["_tag"] === "QuestionNotFoundError" +export type BrowserRequestNotFoundError = { + readonly _tag: "BrowserRequestNotFoundError" + readonly requestID: string + readonly message: string +} +export const isBrowserRequestNotFoundError = (value: unknown): value is BrowserRequestNotFoundError => + typeof value === "object" && value !== null && "_tag" in value && value["_tag"] === "BrowserRequestNotFoundError" + export type ProjectCopyError = { readonly name: "ProjectCopyError" readonly data: { readonly message: string; readonly forceRequired?: boolean | undefined } @@ -2823,6 +2831,83 @@ export type QuestionsRejectInput = { export type QuestionsRejectOutput = void +export type BrowserRequestsListRequestsInput = { + readonly location?: { + readonly location?: { readonly directory?: string | undefined; readonly workspace?: string | undefined } | undefined + }["location"] +} + +export type BrowserRequestsListRequestsOutput = { + readonly location: { + readonly directory: string + readonly workspaceID?: string + readonly project: { readonly id: string; readonly directory: string } + } + readonly data: ReadonlyArray<{ + readonly id: string + readonly sessionID: string + readonly command: + | { readonly action: "page.url" } + | { readonly action: "page.text" } + | { + readonly action: "page.query" + readonly selector: string + readonly limit?: number | "Infinity" | "-Infinity" | "NaN" + } + | { readonly action: "page.click"; readonly selector: string } + | { readonly action: "page.type"; readonly selector: string; readonly text: string; readonly submit?: boolean } + | { readonly action: "page.navigate"; readonly url: string } + }> +} + +export type BrowserRequestsListInput = { readonly sessionID: { readonly sessionID: string }["sessionID"] } + +export type BrowserRequestsListOutput = { + readonly data: ReadonlyArray<{ + readonly id: string + readonly sessionID: string + readonly command: + | { readonly action: "page.url" } + | { readonly action: "page.text" } + | { + readonly action: "page.query" + readonly selector: string + readonly limit?: number | "Infinity" | "-Infinity" | "NaN" + } + | { readonly action: "page.click"; readonly selector: string } + | { readonly action: "page.type"; readonly selector: string; readonly text: string; readonly submit?: boolean } + | { readonly action: "page.navigate"; readonly url: string } + }> +}["data"] + +export type BrowserRequestsReplyInput = { + readonly sessionID: { readonly sessionID: string; readonly requestID: string }["sessionID"] + readonly requestID: { readonly sessionID: string; readonly requestID: string }["requestID"] + readonly value: { + readonly value: + | { readonly type: "string"; readonly value: string } + | { + readonly type: "nodes" + readonly value: ReadonlyArray<{ + readonly selector: string + readonly text: string + readonly attributes: { readonly [x: string]: string } + }> + } + | { readonly type: "void" } + }["value"] +} + +export type BrowserRequestsReplyOutput = void + +export type BrowserRequestsRefuseInput = { + readonly sessionID: { readonly sessionID: string; readonly requestID: string }["sessionID"] + readonly requestID: { readonly sessionID: string; readonly requestID: string }["requestID"] + readonly reason: { readonly reason: string }["reason"] +} + +export type BrowserRequestsRefuseOutput = void + export type ReferencesListInput = { readonly location?: { readonly location?: { readonly directory?: string | undefined; readonly workspace?: string | undefined } | undefined diff --git a/packages/core/src/browser-request.ts b/packages/core/src/browser-request.ts new file mode 100644 index 0000000000..1193b2324a --- /dev/null +++ b/packages/core/src/browser-request.ts @@ -0,0 +1,218 @@ +export * as BrowserRequestV1 from "./browser-request" + +import { makeLocationNode } from "./effect/app-node" +import { Context, Deferred, Duration, Effect, Layer, Schema } from "effect" +import { BrowserRequest } from "@redrob-code/schema/browser-request" +import { EventV2 } from "./event" +import { SessionSchema } from "./session/schema" + +export const ID = BrowserRequest.ID +export type ID = typeof ID.Type + +export const Command = BrowserRequest.Command +export type Command = typeof Command.Type + +export const Request = BrowserRequest.Request +export type Request = typeof Request.Type + +export const Value = BrowserRequest.Value +export type Value = typeof Value.Type + +export const Node = BrowserRequest.Node +export type Node = typeof Node.Type + +export const Event = BrowserRequest.Event + +/** + * The deadline a pending browser request waits for a client before giving up. + * + * It exists because nothing tells the engine whether a client is listening: the event + * stream is a publish/subscribe fan-out with no subscriber count, so "no browser attached" + * and "the browser is thinking" look identical from here. Waiting forever is the one + * behaviour that is certainly wrong — a skill calling `page.text()` with no browser would + * hang its session rather than fail. + * + * Long enough that a real click and its navigation settle; short enough that an unattended + * engine answers in seconds. + */ +export const DEADLINE = Duration.seconds(20) + +/** + * No client settled the request. Carries the action by name, because which call went + * unanswered is the whole diagnostic: `page.navigate` timing out on a slow site and + * `page.url` timing out with nothing attached are different problems. + */ +export class UnansweredError extends Schema.TaggedErrorClass()("BrowserRequestV1.UnansweredError", { + action: Schema.String, +}) { + override get message() { + return `no browser client answered ${this.action} within ${Duration.toSeconds(DEADLINE)}s; the browser must be running with the Redrob extension attached to this engine` + } +} + +/** The client took the request and said it could not do it. Its sentence is carried through. */ +export class RefusedError extends Schema.TaggedErrorClass()("BrowserRequestV1.RefusedError", { + action: Schema.String, + reason: Schema.String, +}) { + override get message() { + return `the browser refused ${this.action}: ${this.reason}` + } +} + +/** + * The client answered with a value the action cannot produce. + * + * Separate from `RefusedError` on purpose. A refusal is the browser reporting about the + * page; this is the client disagreeing with the protocol, and collapsing the two would let + * a wrong-shaped answer read as a fact about the page. + */ +export class MalformedError extends Schema.TaggedErrorClass()("BrowserRequestV1.MalformedError", { + action: Schema.String, + expected: Schema.String, + received: Schema.String, +}) { + override get message() { + return `the browser answered ${this.action} with a ${this.received} value where ${this.expected} was required` + } +} + +export class NotFoundError extends Schema.TaggedErrorClass()("BrowserRequestV1.NotFoundError", { + requestID: ID, +}) {} + +export type AskError = UnansweredError | RefusedError + +export interface AskInput { + readonly sessionID: SessionSchema.ID + readonly command: Command +} + +export interface ReplyInput { + readonly requestID: ID + readonly value: Value +} + +export interface RefuseInput { + readonly requestID: ID + readonly reason: string +} + +export interface Interface { + /** Publishes one browser action and waits for a client to settle it. */ + readonly ask: (input: AskInput) => Effect.Effect + readonly reply: (input: ReplyInput) => Effect.Effect + readonly refuse: (input: RefuseInput) => Effect.Effect + readonly list: () => Effect.Effect> +} + +export class Service extends Context.Service()("@redrob/v1/BrowserRequest") {} + +interface Pending { + readonly request: Request + readonly deferred: Deferred.Deferred +} + +/** + * Location-owned pending browser requests, materialized once per embedded Location so a + * reply cannot settle another Location's request — the same ownership rule `QuestionV2` + * documents and for the same reason. + */ +const layer = Layer.effect( + Service, + Effect.gen(function* () { + const events = yield* EventV2.Service + const pending = new Map() + + yield* Effect.addFinalizer(() => + Effect.forEach( + pending.values(), + (item) => + Deferred.fail( + item.deferred, + new RefusedError({ action: item.request.command.action, reason: "the engine shut down" }), + ), + { discard: true }, + ).pipe( + Effect.ensuring( + Effect.sync(() => { + pending.clear() + }), + ), + ), + ) + + const ask = Effect.fn("BrowserRequestV1.ask")((input: AskInput) => + Effect.uninterruptibleMask((restore) => + Effect.gen(function* () { + const id = ID.ascending() + const deferred = yield* Deferred.make() + const request: Request = { id, sessionID: input.sessionID, command: input.command } + pending.set(id, { request, deferred }) + return yield* events.publish(Event.Asked, request).pipe( + Effect.andThen( + restore(Deferred.await(deferred)).pipe( + // `timeoutOrElse` with a failing fallback rather than a plain `timeout`: + // the caller needs to be told that nobody answered, by action name, not + // handed a generic timeout exception or an empty success. + Effect.timeoutOrElse({ + duration: DEADLINE, + orElse: () => Effect.fail(new UnansweredError({ action: input.command.action })), + }), + ), + ), + Effect.ensuring( + Effect.sync(() => { + pending.delete(id) + }), + ), + ) + }), + ), + ) + + const reply = Effect.fn("BrowserRequestV1.reply")((input: ReplyInput) => + Effect.uninterruptible( + Effect.gen(function* () { + const existing = pending.get(input.requestID) + if (!existing) return yield* new NotFoundError({ requestID: input.requestID }) + yield* events.publish(Event.Answered, { + sessionID: existing.request.sessionID, + requestID: existing.request.id, + }) + yield* Deferred.succeed(existing.deferred, input.value) + pending.delete(input.requestID) + }), + ), + ) + + const refuse = Effect.fn("BrowserRequestV1.refuse")((input: RefuseInput) => + Effect.uninterruptible( + Effect.gen(function* () { + const existing = pending.get(input.requestID) + if (!existing) return yield* new NotFoundError({ requestID: input.requestID }) + yield* events.publish(Event.Refused, { + sessionID: existing.request.sessionID, + requestID: existing.request.id, + reason: input.reason, + }) + yield* Deferred.fail( + existing.deferred, + new RefusedError({ action: existing.request.command.action, reason: input.reason }), + ) + pending.delete(input.requestID) + }), + ), + ) + + const list = Effect.fn("BrowserRequestV1.list")(function* () { + return Array.from(pending.values(), (item) => item.request) + }) + + return Service.of({ ask, reply, refuse, list }) + }), +) + +export const locationLayer = layer + +export const node = makeLocationNode({ service: Service, layer, deps: () => [EventV2.node] }) diff --git a/packages/core/src/location-services.ts b/packages/core/src/location-services.ts index 7da67673c3..89b4dd6219 100644 --- a/packages/core/src/location-services.ts +++ b/packages/core/src/location-services.ts @@ -22,6 +22,7 @@ import { Policy } from "./policy" import { ProjectCopy } from "./project/copy" import { Pty } from "./pty" import { QuestionV2 } from "./question" +import { BrowserRequestV1 } from "./browser-request" import { Reference } from "./reference" import { ReferenceGuidance } from "./reference/guidance" import * as SessionRunnerLLM from "./session/runner/llm" @@ -71,6 +72,7 @@ export const locationServices = LayerNode.group([ ReferenceGuidance.node, SessionTodo.node, QuestionV2.node, + BrowserRequestV1.node, ReadToolFileSystem.node, BuiltInTools.node, SessionRunnerModel.node, diff --git a/packages/core/src/plugin/skill.ts b/packages/core/src/plugin/skill.ts index fcf4a78e5d..6d23dabcf7 100644 --- a/packages/core/src/plugin/skill.ts +++ b/packages/core/src/plugin/skill.ts @@ -8,6 +8,7 @@ import { AbsolutePath } from "../schema" import { SkillV2 } from "../skill" import customizeRedrobContent from "./skill/customize-redrob.md" with { type: "text" } import documentToolchainContent from "./skill/document-toolchain.md" with { type: "text" } +import pageControlContent from "./skill/page-control.md" with { type: "text" } import skillWriterContent from "./skill/skill-writer.md" with { type: "text" } export const CustomizeRedrobContent = customizeRedrobContent @@ -18,6 +19,9 @@ export const DocumentToolchainContent = documentToolchainContent /** K-5's skill document. Exported so a test can round-trip the template it prescribes. */ export const SkillWriterContent = skillWriterContent +/** PA-9's skill document. Exported so a test can assert it against the real `page` surface. */ +export const PageControlContent = pageControlContent + /** * The skills the engine ships with. Exported as data rather than built inline in the plugin * effect so a test can decode every entry through `SkillV2.Info` — `Info.make` does not run @@ -49,6 +53,28 @@ export const BuiltinSkills: ReadonlyArray = [ }), content: DocumentToolchainContent, }), + // PA-9. `page` resolves through the browser channel now, so this document describes calls + // that actually run. The keywords are what a request to read or drive the page contains; + // no url globs, because arming on a URL would put it in every session on any page. + SkillV2.Info.make({ + name: "page-control", + description: + "Use when the task is about the page the user is looking at: reading its text, finding nodes by selector, filling a field, clicking, or navigating. Covers the six `page` calls in code mode, what a refusal means, and the round-trip cost. Do not use for fetching a URL the user is not on, which is a plain HTTP request.", + location: AbsolutePath.make("/builtin/page-control.md"), + autoInject: SkillV2.AutoInject.make({ + keywords: [ + "this page", + "current page", + "on screen", + "the tab", + "click the", + "fill in", + "selector", + "scrape", + ], + }), + content: PageControlContent, + }), // K-5. No `autoInject`: writing a skill is something a person asks for by name, and a skill // that armed on the word "skill" would be in the prompt of every session that mentions one. SkillV2.Info.make({ diff --git a/packages/core/src/plugin/skill/page-control.md b/packages/core/src/plugin/skill/page-control.md new file mode 100644 index 0000000000..4db636c702 --- /dev/null +++ b/packages/core/src/plugin/skill/page-control.md @@ -0,0 +1,91 @@ +# Reading and driving the page in code mode + +Use this when the task is about the page the user is looking at: read it, find things in +it, fill a field, click, or follow a link. In code mode you write a program and the `page` +global is in scope. + +## What exists + +Six calls. There is no seventh — if you need something else, say so rather than inventing +a call that will not resolve. + +```ts +await page.url() // string — the current URL +await page.text() // string — the visible text +await page.query({ selector, limit }) // PageNode[] — in document order +await page.click({ selector }) // void +await page.type({ selector, text, submit }) // void +await page.navigate({ url }) // string — the URL actually landed on +``` + +A `PageNode` is `{ selector, text, attributes }`. It is a snapshot, not a handle: you +cannot hold one and act on it later. Act by selector. + +## Where the page is + +Not in this process. The engine runs beside the browser and cannot reach into it, so each +call is a request the browser answers. Three consequences, all of which you will meet: + +1. **Every call can refuse.** When no browser is attached, the call fails with + `DomainUnavailableError` and the message names the action. That is the honest answer, not + a bug to work around. Tell the user the browser is not attached. +2. **A refusal is also the answer for a tab the user has not exposed**, and for the + browser's own pages (`chrome://`, the extension's pages). You cannot read those, and + nothing you write will change that. +3. **Each call is a round trip.** Three queries cost three round trips. Prefer one + `page.text()` over ten `page.query()` calls when you only need to read, and one + `page.query()` with a `limit` over a loop. + +## Reading before acting + +`page.text()` is the cheapest way to see what is on screen, and it is usually enough to +answer a question about the page. Reach for `page.query()` when you need structure — the +href of each link, the value of an attribute, which of several forms is which. + +```ts +const links = await page.query({ selector: "a[href]", limit: 50 }) +const external = links.filter((node) => !node.attributes.href?.startsWith("/")) +``` + +`query` returns `[]` when the selector matched nothing. That is a fact about the page, and +it is different from a refusal, which is a fact about the session. Do not treat an empty +array as "no browser" — check the error path for that. + +## Filling a field + +`page.type` targets the first node matching the selector and types into it. `submit: true` +submits the form afterwards, which is one round trip instead of a type and a click. + +```ts +await page.type({ selector: "input[name=q]", text: "redrob", submit: true }) +``` + +Type into the field, not into its wrapper: a selector that matches a `div` around the input +has nothing to receive text. Pick the `input`, `textarea` or `[contenteditable]` itself, +and confirm with `page.query` first when the markup is unfamiliar. + +## Navigating + +`page.navigate` returns the URL actually landed on, which is frequently not the one you +asked for — a redirect, a login wall, a trailing-slash normalisation. Read the return value +before assuming you are where you aimed. + +```ts +const landed = await page.navigate({ url: "https://example.com/report" }) +if (!landed.includes("/report")) { + // A login wall or a redirect. Say so; do not keep clicking. +} +``` + +## What not to do + +Do not poll. There is no wait call, and a loop of `page.text()` until something changes +is a round trip per iteration against a page that may never change. Read once, act, read +once more. + +Do not scrape a credential. The page the user is on may be signed in; reading a token out +of it and putting it in your answer or a file is an exfiltration, not a task step. + +Do not guess at a selector you have not seen. One `page.query({ selector: "form", limit: 5 +})` costs one round trip and tells you the shape; a wrong selector costs a refusal and a +confused user. diff --git a/packages/core/test/browser-request.test.ts b/packages/core/test/browser-request.test.ts new file mode 100644 index 0000000000..87518a9cbc --- /dev/null +++ b/packages/core/test/browser-request.test.ts @@ -0,0 +1,143 @@ +import { describe, expect } from "bun:test" +import { Cause, Deferred, Duration, Effect, Exit, Fiber } from "effect" +import * as TestClock from "effect/testing/TestClock" +import { LayerNode } from "@redrob-code/core/effect/layer-node" +import { AppNodeBuilder } from "@redrob-code/core/effect/app-node-builder" +import { EventV2 } from "@redrob-code/core/event" +import { BrowserRequestV1 } from "@redrob-code/core/browser-request" +import { SessionV2 } from "@redrob-code/core/session" +import { testEffect } from "./lib/effect" + +const browser = AppNodeBuilder.build(LayerNode.group([EventV2.node, BrowserRequestV1.node])) +const it = testEffect(browser) + +const sessionID = SessionV2.ID.make("ses_browser_test") +const command: BrowserRequestV1.Command = { action: "page.text" } + +/** Forks an ask and returns once its `asked` event has been observed on the stream. */ +const waitForAsk = Effect.fn("BrowserRequestV1Test.waitForAsk")(function* ( + service: BrowserRequestV1.Interface, + input: BrowserRequestV1.AskInput, +) { + const events = yield* EventV2.Service + const asked = yield* Deferred.make() + const unsubscribe = yield* events.listen((event) => + event.type === BrowserRequestV1.Event.Asked.type + ? Deferred.succeed(asked, event.data as BrowserRequestV1.Request).pipe(Effect.asVoid) + : Effect.void, + ) + yield* Effect.addFinalizer(() => unsubscribe) + const fiber = yield* service.ask(input).pipe(Effect.forkScoped) + return { fiber, request: yield* Deferred.await(asked) } +}) + +describe("BrowserRequestV1", () => { + it.effect("publishes the action on the event stream and settles on a reply", () => + Effect.gen(function* () { + const service = yield* BrowserRequestV1.Service + const events = yield* EventV2.Service + const published: EventV2.Payload[] = [] + const unsubscribe = yield* events.listen((event) => + Effect.sync(() => { + if (event.type.startsWith("browser.request.")) published.push(event) + }), + ) + yield* Effect.addFinalizer(() => unsubscribe) + const { fiber, request } = yield* waitForAsk(service, { sessionID, command }) + + expect(request.id).toMatch(/^brq_/) + // The command travels whole: a client that only got the action name could not act. + expect(request.command).toEqual(command) + expect(yield* service.list()).toEqual([request]) + + yield* service.reply({ requestID: request.id, value: { type: "string", value: "page body" } }) + + expect(yield* Fiber.join(fiber)).toEqual({ type: "string", value: "page body" }) + expect(yield* service.list()).toEqual([]) + expect(published.map((event) => event.type)).toEqual([ + BrowserRequestV1.Event.Asked.type, + BrowserRequestV1.Event.Answered.type, + ]) + }), + ) + + it.effect("a refusal fails the ask with the client's own reason", () => + Effect.gen(function* () { + const service = yield* BrowserRequestV1.Service + const { fiber, request } = yield* waitForAsk(service, { + sessionID, + command: { action: "page.click", selector: "#buy" }, + }) + + yield* service.refuse({ requestID: request.id, reason: "that tab is not shared with the agent" }) + + const exit = yield* Fiber.await(fiber) + expect(Exit.isFailure(exit)).toBe(true) + const error = Exit.isFailure(exit) ? Cause.squash(exit.cause) : undefined + expect(error).toBeInstanceOf(BrowserRequestV1.RefusedError) + expect((error as BrowserRequestV1.RefusedError).message).toBe( + "the browser refused page.click: that tab is not shared with the agent", + ) + // Settled either way: a refused request must not keep occupying the pending map. + expect(yield* service.list()).toEqual([]) + }), + ) + + it.effect("settling an unknown id is refused rather than ignored", () => + Effect.gen(function* () { + const service = yield* BrowserRequestV1.Service + const unknown = BrowserRequestV1.ID.make("brq_unknown") + + const replied = yield* service + .reply({ requestID: unknown, value: { type: "void" } }) + .pipe(Effect.exit) + const refused = yield* service.refuse({ requestID: unknown, reason: "whatever" }).pipe(Effect.exit) + + expect(Exit.isFailure(replied)).toBe(true) + expect(Exit.isFailure(refused)).toBe(true) + }), + ) + + /** + * The deadline is the reason this service can be used by a skill at all. + * + * Nothing tells the engine whether a client is subscribed — the event stream has no + * subscriber count — so "no browser attached" is indistinguishable from "the browser is + * slow". Waiting forever would hang the session of anyone who calls `page.text()` with no + * browser running, which is the single outcome that is certainly wrong. + * + * Driven by the TestClock, so this asserts the real `DEADLINE` constant without the suite + * paying twenty seconds of wall clock for it. + */ + it.effect("an unanswered request fails on the deadline, naming the action", () => + Effect.gen(function* () { + const service = yield* BrowserRequestV1.Service + const fiber = yield* service.ask({ sessionID, command }).pipe(Effect.forkScoped) + + yield* TestClock.adjust(Duration.sum(BrowserRequestV1.DEADLINE, Duration.seconds(1))) + + const exit = yield* Fiber.await(fiber) + expect(Exit.isFailure(exit)).toBe(true) + const error = Exit.isFailure(exit) ? Cause.squash(exit.cause) : undefined + expect(error).toBeInstanceOf(BrowserRequestV1.UnansweredError) + expect((error as BrowserRequestV1.UnansweredError).action).toBe("page.text") + expect((error as BrowserRequestV1.UnansweredError).message).toContain("no browser client answered page.text") + // And it is no longer pending: a request nobody answered must not leak. + expect(yield* service.list()).toEqual([]) + }), + ) + + it.effect("a request answered before the deadline does NOT fail", () => + Effect.gen(function* () { + // The other half of the deadline test. A timeout that fires regardless would pass the + // test above and break every real call. + const service = yield* BrowserRequestV1.Service + const { fiber, request } = yield* waitForAsk(service, { sessionID, command }) + + yield* service.reply({ requestID: request.id, value: { type: "void" } }) + yield* TestClock.adjust(Duration.sum(BrowserRequestV1.DEADLINE, Duration.seconds(1))) + + expect(yield* Fiber.join(fiber)).toEqual({ type: "void" }) + }), + ) +}) diff --git a/packages/core/test/skill/skill-writer.test.ts b/packages/core/test/skill/skill-writer.test.ts index 65c97f024e..fdd0272ef9 100644 --- a/packages/core/test/skill/skill-writer.test.ts +++ b/packages/core/test/skill/skill-writer.test.ts @@ -140,6 +140,7 @@ describe("K-5 the skill document itself", () => { expect(SkillPlugin.BuiltinSkills.map((skill) => skill.name)).toStrictEqual([ "customize-redrob", "documents", + "page-control", "skill-writer", ]) for (const skill of SkillPlugin.BuiltinSkills) { diff --git a/packages/protocol/src/api.ts b/packages/protocol/src/api.ts index 9280a31083..43b1e267de 100644 --- a/packages/protocol/src/api.ts +++ b/packages/protocol/src/api.ts @@ -15,6 +15,7 @@ import type { Definition } from "@redrob-code/schema/event" import { AgentGroup } from "./groups/agent" import { HealthGroup } from "./groups/health" import { PtyGroup } from "./groups/pty" +import { makeBrowserRequestGroup } from "./groups/browser-request" import { makeQuestionGroup } from "./groups/question" import { ReferenceGroup } from "./groups/reference" import { Authorization } from "./middleware/authorization" @@ -54,6 +55,7 @@ const makeApiFromGroup = < .add(eventGroup) .add(PtyGroup.middleware(locationMiddleware)) .add(makeQuestionGroup(locationMiddleware, sessionLocationMiddleware)) + .add(makeBrowserRequestGroup(locationMiddleware, sessionLocationMiddleware)) .add(ReferenceGroup.middleware(locationMiddleware)) .add(ProjectCopyGroup.middleware(locationMiddleware)) // The OpenAI-compatible inference route. It takes locationMiddleware like the diff --git a/packages/protocol/src/errors.ts b/packages/protocol/src/errors.ts index 3b1eced63a..8ea710a022 100644 --- a/packages/protocol/src/errors.ts +++ b/packages/protocol/src/errors.ts @@ -95,6 +95,23 @@ export class QuestionNotFoundError extends Schema.TaggedErrorClass()( + "BrowserRequestNotFoundError", + { + requestID: Schema.String, + message: Schema.String, + }, + { httpApiStatus: 404 }, +) {} + export class ForbiddenError extends Schema.TaggedErrorClass()( "ForbiddenError", { message: Schema.String }, diff --git a/packages/protocol/src/groups/browser-request.ts b/packages/protocol/src/groups/browser-request.ts new file mode 100644 index 0000000000..37e0f3559f --- /dev/null +++ b/packages/protocol/src/groups/browser-request.ts @@ -0,0 +1,102 @@ +import { BrowserRequest } from "@redrob-code/schema/browser-request" +import { Location } from "@redrob-code/schema/location" +import { Session } from "@redrob-code/schema/session" +import { Context, Schema } from "effect" +import { HttpApiEndpoint, HttpApiGroup, HttpApiMiddleware, HttpApiSchema, OpenApi } from "effect/unstable/httpapi" +import { BrowserRequestNotFoundError, SessionNotFoundError } from "../errors" +import { LocationQuery, locationQueryOpenApi } from "./location" + +/** + * PA-10: the client half of the browser channel. + * + * Three routes and no listener. The client already holds the engine's event stream, so it + * learns about a request there and settles it here — `reply` with a value, `refuse` with a + * sentence. The engine never connects to the browser, which is the property that keeps a + * local page or another local process from driving it. + */ +export const makeBrowserRequestGroup = < + LocationId extends HttpApiMiddleware.AnyId, + LocationService, + SessionLocationId extends HttpApiMiddleware.AnyId, + SessionLocationService, +>( + locationMiddleware: Context.Key, + sessionLocationMiddleware: Context.Key, +) => + HttpApiGroup.make("server.browser") + .add( + HttpApiEndpoint.get("browser.request.list", "/api/browser/request", { + query: LocationQuery, + success: Location.response(Schema.Array(BrowserRequest.Request)), + }) + .annotateMerge(locationQueryOpenApi) + .annotateMerge( + OpenApi.annotations({ + identifier: "v1.browser.request.list", + summary: "List pending browser requests", + description: + "Retrieve browser actions awaiting a client for a location. A client that reconnects uses this to recover requests published while it was away.", + }), + ), + ) + .annotateMerge( + OpenApi.annotations({ title: "browser requests", description: "Browser actions awaiting a client." }), + ) + // Group middleware applies only to endpoints already added; the session endpoints below + // carry session placement instead. + .middleware(locationMiddleware) + .add( + HttpApiEndpoint.get("session.browser.list", "/api/session/:sessionID/browser", { + params: { sessionID: Session.ID }, + success: Schema.Struct({ data: Schema.Array(BrowserRequest.Request) }), + error: SessionNotFoundError, + }) + .middleware(sessionLocationMiddleware) + .annotateMerge( + OpenApi.annotations({ + identifier: "v1.session.browser.list", + summary: "List session browser requests", + description: "Retrieve browser actions awaiting a client, owned by a session.", + }), + ), + ) + .add( + HttpApiEndpoint.post("session.browser.reply", "/api/session/:sessionID/browser/:requestID/reply", { + params: { sessionID: Session.ID, requestID: BrowserRequest.ID }, + payload: BrowserRequest.Reply, + success: HttpApiSchema.NoContent, + error: [SessionNotFoundError, BrowserRequestNotFoundError], + }) + .middleware(sessionLocationMiddleware) + .annotateMerge( + OpenApi.annotations({ + identifier: "v1.session.browser.reply", + summary: "Answer a pending browser request", + description: + "Settle a browser action with the value it produced. The value is typed per action, so answering with the wrong shape is reported to the engine's caller rather than read as a fact about the page.", + }), + ), + ) + .add( + HttpApiEndpoint.post("session.browser.refuse", "/api/session/:sessionID/browser/:requestID/refuse", { + params: { sessionID: Session.ID, requestID: BrowserRequest.ID }, + payload: BrowserRequest.Refusal, + success: HttpApiSchema.NoContent, + error: [SessionNotFoundError, BrowserRequestNotFoundError], + }) + .middleware(sessionLocationMiddleware) + .annotateMerge( + OpenApi.annotations({ + identifier: "v1.session.browser.refuse", + summary: "Refuse a pending browser request", + description: + "Report that the action could not be performed, with the reason. Refusing is faster and more honest than letting the request expire on the engine's deadline.", + }), + ), + ) + .annotateMerge( + OpenApi.annotations({ + title: "session browser requests", + description: "Session-owned browser action routes.", + }), + ) diff --git a/packages/redrob/src/session/prompt.ts b/packages/redrob/src/session/prompt.ts index b9d6c23ee7..2eb8313f96 100644 --- a/packages/redrob/src/session/prompt.ts +++ b/packages/redrob/src/session/prompt.ts @@ -1292,11 +1292,33 @@ const layer = Layer.effect( sys.mcp(agent, session.permission), MessageV2.toModelMessagesEffect(msgs, model), ]) + /* + * K-2's arming, finally called. + * + * The prompt text is every text part of the user's last message joined, not + * just the first: a message with an attachment between two sentences would + * otherwise arm on half of what the user said. + * + * The URL is deliberately NOT fetched. Arming by URL glob needs the active + * tab, and the only way to get it is a browser request -- a round trip on + * every single message, which with no extension attached spends the request + * deadline before the model sees anything. A twenty second pause before each + * reply is a worse product than URL arming is a better one. Keywords arm + * today; the URL half arms once a session carries its tab URL as state the + * engine already holds, rather than as a question it has to ask. + */ + const armedSkills = yield* sys.armedSkills(agent, { + prompt: (lastUserMsg?.parts ?? []) + .filter((part) => part.type === "text") + .map((part) => (part.type === "text" ? part.text : "")) + .join("\n"), + }) const system = [ ...env, ...instructions, ...(mcpInstructions ? [mcpInstructions] : []), ...(skills ? [skills] : []), + ...(armedSkills ? [armedSkills] : []), ANSWER_OPTIONS_PROMPT, ] const format = lastUser.format ?? { type: "text" as const } diff --git a/packages/redrob/src/session/system.ts b/packages/redrob/src/session/system.ts index bde67ebc91..f1e022937c 100644 --- a/packages/redrob/src/session/system.ts +++ b/packages/redrob/src/session/system.ts @@ -17,6 +17,8 @@ import type { Provider } from "@/provider/provider" import type { Agent } from "@/agent/agent" import { Permission } from "@/permission" import { Skill } from "@/skill" +import { SkillArming } from "@redrob-code/core/skill/arming" +import { escapeHtml } from "@/util/html" import { AbsolutePath } from "@redrob-code/core/schema" import { Location } from "@redrob-code/core/location" import { LocationServiceMap, locationServiceMapLayer } from "@redrob-code/core/location-services" @@ -51,6 +53,19 @@ export function provider(model: Provider.Model) { export interface Interface { readonly environment: (model: Provider.Model) => Effect.Effect readonly skills: (agent: Agent.Info) => Effect.Effect + /** + * The bodies of the skills this prompt armed, as their own block. + * + * Separate from `skills` because the two make different claims. `skills` is the + * discovery list -- names and descriptions, for the model to choose from with the skill + * tool. This is the content of the ones that armed themselves on what the user just + * typed, or on the page they are looking at, which the model did not ask for and must + * therefore be told it is reading. + */ + readonly armedSkills: ( + agent: Agent.Info, + input: { readonly prompt: string; readonly url?: string | undefined }, + ) => Effect.Effect readonly mcp: (agent: Agent.Info, permission?: PermissionV1.Ruleset) => Effect.Effect } @@ -116,6 +131,58 @@ const layer = Layer.effect( ].join("\n") }), + /** + * Arm the skills whose own `autoInject` block matches, and hand over their bodies. + * + * K-2 landed the matcher as a pure function and nothing called it, so every skill + * that declared keywords and URL globs behaved exactly like one that declared none. + * This is the call. + * + * Two properties are deliberate and both are about not drowning the prompt. A skill + * with no `autoInject` never arms, so the shipped `skill-writer` stays + * ask-for-it-by-name. And the armed bodies are labelled as armed, with the patterns + * that armed them: a model handed a document toolchain it never requested should be + * able to see why, and so should anyone reading the transcript. + */ + armedSkills: Effect.fn("SystemPrompt.armedSkills")(function* ( + agent: Agent.Info, + input: { readonly prompt: string; readonly url?: string | undefined }, + ) { + if (Permission.disabled(["skill"], agent.permission).has("skill")) return + + const list = yield* skill.available(agent) + const armed = SkillArming.arm({ skills: list, prompt: input.prompt, url: input.url }) + if (armed.length === 0) return + + const byName = new Map(list.map((entry) => [entry.name, entry] as const)) + const sections: string[] = [] + for (const entry of armed) { + const info = byName.get(entry.name) + // `arm` only ever returns names it was given, so a miss here is impossible rather + // than unlikely -- but a skill with no content would silently contribute an empty + // section, which reads as a skill that said nothing. + if (info === undefined || info.content.trim().length === 0) continue + const why = [ + ...entry.keywords.map((keyword) => `keyword ${keyword}`), + ...entry.urls.map((glob) => `url ${glob}`), + ].join(", ") + // Escaped with the repo's own helper: a keyword or URL glob is author-supplied + // text from a skill file, and the first version of this used `JSON.stringify`, + // which put a raw `"` inside the attribute and broke the tag. + sections.push(` `, info.content, ` `) + } + if (sections.length === 0) return + + return [ + "", + "These skills armed themselves on this request -- their own autoInject block matched", + "what the user typed, or the page they are on. You did not load them with the skill", + "tool; they are here already, so do not load them again.", + ...sections, + "", + ].join("\n") + }), + mcp: Effect.fn("SystemPrompt.mcp")(function* (agent: Agent.Info, permission?: PermissionV1.Ruleset) { const ruleset = Permission.merge(agent.permission, permission ?? []) const instructions = (yield* mcp.instructions()).filter( diff --git a/packages/redrob/src/skill/index.ts b/packages/redrob/src/skill/index.ts index 53a7cf4368..191f81b9f0 100644 --- a/packages/redrob/src/skill/index.ts +++ b/packages/redrob/src/skill/index.ts @@ -1,12 +1,13 @@ import { LayerNode } from "@redrob-code/core/effect/layer-node" import path from "path" -import { Effect, Layer, Context, Schema } from "effect" +import { Effect, Layer, Context, Option, Schema } from "effect" import { NamedError } from "@redrob-code/core/util/error" import type { Agent } from "@/agent/agent" import { EventV2Bridge } from "@/event-v2-bridge" import { InstanceState } from "@/effect/instance-state" import { Global } from "@redrob-code/core/global" import { SkillPlugin } from "@redrob-code/core/plugin/skill" +import { SkillV2 } from "@redrob-code/core/skill" import { Permission } from "@/permission" import { FSUtil } from "@redrob-code/core/fs-util" import { Config } from "@/config/config" @@ -39,6 +40,15 @@ export const Info = Schema.Struct({ description: Schema.optional(Schema.String), location: Schema.String, content: Schema.String, + /** + * The skill's own auto-arming block, when it declares one. + * + * Carried here because this is the skill surface the system prompt reads. K-2 landed the + * matcher and the frontmatter key, but this `Info` dropped the field, so arming had no + * data to work on no matter who called it -- every skill looked like a skill that + * declared nothing. Same schema as the loader's, not a second copy. + */ + autoInject: Schema.optional(SkillV2.AutoInject), }) export type Info = Schema.Schema.Type @@ -50,7 +60,14 @@ const Issue = Schema.StructWithRest( [Schema.Record(Schema.String, Schema.Unknown)], ) -function isSkillFrontmatter(data: unknown): data is { name: string; description?: string } { +/** + * Does this frontmatter block carry what a skill needs? + * + * `autoInject` is checked but NOT required: a malformed block costs only the arming + * behaviour, and the skill still loads and stays explicitly selectable. Refusing the whole + * file would lose a working skill over an optional key. + */ +function isSkillFrontmatter(data: unknown): data is { name: string; description?: string; autoInject?: unknown } { return ( isRecord(data) && typeof data.name === "string" && @@ -58,6 +75,17 @@ function isSkillFrontmatter(data: unknown): data is { name: string; description? ) } +/** + * Decode an `autoInject` block, keeping the skill when it is malformed. + * + * Returns undefined for anything the schema rejects, which makes the skill behave exactly + * like one that declared no block at all -- the documented failure mode. + */ +function autoInjectOf(data: { autoInject?: unknown }): SkillV2.AutoInject | undefined { + if (data.autoInject === undefined) return undefined + return Schema.decodeUnknownOption(SkillV2.AutoInject)(data.autoInject).pipe(Option.getOrUndefined) +} + export class InvalidError extends Schema.TaggedErrorClass()("SkillInvalidError", { path: Schema.String, message: Schema.optional(Schema.String), @@ -136,6 +164,7 @@ const add = Effect.fnUntraced(function* (state: State, match: string, events: Ev description: md.data.description, location: match, content: md.content, + autoInject: autoInjectOf(md.data), } }) diff --git a/packages/redrob/src/tool/code-mode.ts b/packages/redrob/src/tool/code-mode.ts index c922818e19..57eea30cfa 100644 --- a/packages/redrob/src/tool/code-mode.ts +++ b/packages/redrob/src/tool/code-mode.ts @@ -8,7 +8,15 @@ import { Agent } from "@/agent/agent" import { Session } from "@/session/session" import { Permission } from "@/permission" import { Plugin } from "@/plugin" -import { DOMAIN_GLOBALS, domainTools, sessionChannel, unavailableChannel, unavailablePage } from "./domain" +import { + DOMAIN_GLOBALS, + bridgedPage, + domainTools, + sessionChannel, + unavailableChannel, + unavailablePage, +} from "./domain" +import { BrowserRequestV1 } from "@redrob-code/core/browser-request" import * as Documents from "./document" import { InstanceRef } from "@/effect/instance-ref" import { assertExternalDirectoryEffect } from "./external-directory" @@ -202,6 +210,10 @@ export const CodeModeTool = Tool.define( const agents = yield* Agent.Service const sessions = yield* Session.Service const plugin = yield* Plugin.Service + // Resolved here with the other location services, not inside `execute`: the tool + // contract requires `execute` to need nothing from the context, so pulling the service + // per call would put it back in the effect's requirements and fail to typecheck. + const browser = yield* BrowserRequestV1.Service const init: Tool.DefWithoutID = { description: DESCRIPTION, @@ -224,9 +236,12 @@ export const CodeModeTool = Tool.define( const calls: CallEntry[] = [] const attachments: Attachment[] = [] // K-1 domain objects for this execution. `channel` posts into the session that owns - // the run; `page` has no working implementation in the engine process yet and - // refuses by name rather than returning a plausible value. - const page = unavailablePage() + // the run; `page` (PA-10) resolves through the browser channel — each call is + // published as a pending request on the event stream and settled by the attached + // browser client. With no client attached the request expires on the service's + // deadline and the call refuses by name, which is the same outcome the earlier + // `unavailablePage` gave and for a reason the message states. + const page = bridgedPage({ browser, sessionID: ctx.sessionID }) const channel = sessionChannel({ sessions, sessionID: ctx.sessionID, messageID: ctx.messageID }) // K-3 document objects. Unlike `page`, these DO work in the engine process: a write // lands a real file. The guard is the same external-directory check the `write` tool diff --git a/packages/redrob/src/tool/domain.ts b/packages/redrob/src/tool/domain.ts index 409b72e96e..f698a90d51 100644 --- a/packages/redrob/src/tool/domain.ts +++ b/packages/redrob/src/tool/domain.ts @@ -12,11 +12,15 @@ * namespace of tools plus the list of names to bind as globals, and never learns what * `page` or `channel` mean (see packages/codemode/AGENTS.md). * - * `page` has no implementation that can act today: nothing in the engine process controls - * a browser page. Its single implementation is therefore `unavailablePage`, which THROWS a - * message naming what is missing rather than returning a plausible value, so a skill - * written against the interface fails loudly instead of silently reading an empty string. + * `page` is resolved through the browser channel (PA-10): each call becomes a pending + * request published on the event stream the client already subscribes to, and the attached + * browser settles it. `bridgedPage` is that implementation. Where no channel exists — the + * catalog preview, a harness with no browser — the implementation is `unavailablePage`, + * which THROWS a message naming what is missing rather than returning a plausible value, so + * a skill written against the interface fails loudly instead of silently reading an empty + * string. */ +import { BrowserRequestV1 } from "@redrob-code/core/browser-request" import { SessionV1 } from "@redrob-code/core/v1/session" import { Effect, Schema } from "effect" import { Tool as SandboxTool, toolError } from "@redrob-code/codemode" @@ -79,14 +83,15 @@ export interface Channel { } /** - * The single `Page` implementation available in the engine process: none of it works. + * The `Page` for a caller with no browser channel: every method refuses by name. * - * Kept deliberately rather than omitted, so the interface, the tool schemas, the generated - * instructions, and the skills written against them all exist and are exercised before the - * browser side lands. `missing` names the capability, not the symptom. + * Still the right implementation for the catalog preview and for any harness that binds no + * browser, so the interface, the tool schemas and the generated instructions all exist and + * are exercised whether or not a browser is attached. `missing` names the capability, not + * the symptom. */ export const unavailablePage = ( - missing = "the engine process has no browser-page control surface; the browser must expose page control to the engine first", + missing = "no browser client is attached to this engine; the page object needs the browser extension connected to this engine's event stream", ): Page => { const refuse = () => Effect.fail(new DomainUnavailableError({ object: "page", missing })) as Effect.Effect @@ -100,6 +105,73 @@ export const unavailablePage = ( } } +/** + * The `Page` that actually works: every call becomes a pending browser request the + * attached client settles. + * + * This is PA-10. The engine still controls nothing itself — it publishes the action on the + * event stream it already serves and waits for an answer, so no socket is opened and + * nothing local can reach the browser through the engine. + * + * Every failure mode is mapped to `DomainUnavailableError` carrying the service's own + * sentence, because the `Page` interface promises exactly that error and a skill reads the + * sentence. The three causes stay distinguishable in the text: nobody answered, the browser + * refused, or the client answered with a shape the action cannot produce. + */ +export const bridgedPage = (input: { + readonly browser: BrowserRequestV1.Interface + readonly sessionID: SessionID +}): Page => { + const unavailable = (missing: string) => new DomainUnavailableError({ object: "page", missing }) + + const ask = (command: BrowserRequestV1.Command) => + input.browser + .ask({ sessionID: input.sessionID, command }) + .pipe(Effect.mapError((error) => unavailable(error.message))) + + /** + * Reads the value a settled request carried, insisting on the shape the action promised. + * + * The insistence is the point. A client answering `page.query` with a string must not + * become an empty node list: "nothing matched the selector" and "the client answered + * wrongly" are different claims and only one of them is about the page. + */ + const expect = + (expected: BrowserRequestV1.Value["type"], read: (value: BrowserRequestV1.Value) => A | undefined) => + (command: BrowserRequestV1.Command) => + ask(command).pipe( + Effect.flatMap((value) => { + const taken = read(value) + if (taken === undefined) { + return Effect.fail( + unavailable( + new BrowserRequestV1.MalformedError({ action: command.action, expected, received: value.type }) + .message, + ), + ) + } + return Effect.succeed(taken) + }), + ) + + const expectString = expect("string", (value) => (value.type === "string" ? value.value : undefined)) + const expectNodes = expect>("nodes", (value) => + value.type === "nodes" ? value.value : undefined, + ) + // `void` reads as `null` rather than `undefined`, since `undefined` is this helper's own + // "wrong shape" signal and would turn every successful click into a malformed answer. + const expectVoid = expect("void", (value) => (value.type === "void" ? null : undefined)) + + return { + url: () => expectString({ action: "page.url" }), + text: () => expectString({ action: "page.text" }), + query: (query) => expectNodes({ action: "page.query", ...query }), + click: (click) => expectVoid({ action: "page.click", ...click }).pipe(Effect.asVoid), + type: (type) => expectVoid({ action: "page.type", ...type }).pipe(Effect.asVoid), + navigate: (navigate) => expectString({ action: "page.navigate", ...navigate }), + } +} + /** * The single `Channel` implementation: posts into the session the program is running in, * by appending a text part to the assistant message that owns the execution. Every surface diff --git a/packages/redrob/src/tool/registry.ts b/packages/redrob/src/tool/registry.ts index 51b1f2eb12..e5c3bde3c2 100644 --- a/packages/redrob/src/tool/registry.ts +++ b/packages/redrob/src/tool/registry.ts @@ -52,6 +52,7 @@ import { RuntimeFlags } from "@/effect/runtime-flags" import { ProviderV2 } from "@redrob-code/core/provider" import { ModelV2 } from "@redrob-code/core/model" import { MCP } from "@/mcp" +import { BrowserRequestV1 } from "@redrob-code/core/browser-request" import { PermissionV1 } from "@redrob-code/core/v1/permission" import { McpCatalog } from "@/mcp/catalog" @@ -447,6 +448,9 @@ export const node = LayerNode.make({ Truncate.node, RuntimeFlags.node, MCP.node, + // PA-10: the code-mode tool resolves `page` through this service, so the registry that + // builds that tool depends on it. + BrowserRequestV1.node, Database.node, Ripgrep.node, ], diff --git a/packages/redrob/test/event-manifest.test.ts b/packages/redrob/test/event-manifest.test.ts index 8c152a0910..743c6a4bcf 100644 --- a/packages/redrob/test/event-manifest.test.ts +++ b/packages/redrob/test/event-manifest.test.ts @@ -9,12 +9,18 @@ describe("public event manifest", () => { expect(EventManifest.Definitions).toBe(SchemaEventManifest.Definitions) expect(EventManifest.Latest).toBe(SchemaEventManifest.Latest) expect(EventManifest.Durable).toBe(SchemaEventManifest.Durable) - expect(EventManifest.Latest.size).toBe(88) + expect(EventManifest.Latest.size).toBe(91) expect(EventManifest.Latest.get("session.next.step.ended")).toBe(SessionEvent.Step.Ended) expect(EventManifest.Latest.get("todo.updated")).toBe(Todo.Event.Updated) expect(EventManifest.Latest.has("ide.installed")).toBe(false) expect(EventManifest.Latest.has("server.connected")).toBe(true) expect(EventManifest.Latest.has("global.disposed")).toBe(true) + // PA-10's three, named rather than merely counted. The size above is a tripwire for an + // accidental addition, and a bare bump would have satisfied it without saying what + // arrived -- so the thing that moved the count asserts itself here. + expect(EventManifest.Latest.has("browser.request.asked")).toBe(true) + expect(EventManifest.Latest.has("browser.request.answered")).toBe(true) + expect(EventManifest.Latest.has("browser.request.refused")).toBe(true) }) test("contains only the current step settlement versions", () => { diff --git a/packages/redrob/test/server/httpapi-exercise/index.ts b/packages/redrob/test/server/httpapi-exercise/index.ts index e8cc795e47..3c4a84f0ad 100644 --- a/packages/redrob/test/server/httpapi-exercise/index.ts +++ b/packages/redrob/test/server/httpapi-exercise/index.ts @@ -919,6 +919,47 @@ const scenarios: Scenario[] = [ headers: ctx.headers(), })) .json(404, object, "status"), + // PA-10's four, mirroring the V2 question scenarios above because the browser request + // service is deliberately the same shape: a global list, a per-session list, and the two + // settlement routes. The settlements name a request id that does not exist, so each + // asserts the 404 path rather than needing a live browser on the other end. + http.protected.get("/api/browser/request", "v2.browser.request.list").json(200, (body) => { + object(body) + object(body.location) + array(body.data) + }), + http.protected + .get("/api/session/{sessionID}/browser", "v2.session.browser.list") + .seeded((ctx) => ctx.session({ title: "Browser request list owner" })) + .at((ctx) => ({ + path: route("/api/session/{sessionID}/browser", { sessionID: ctx.state.id }), + headers: ctx.headers(), + })) + .json(200, data(array)), + http.protected + .post("/api/session/{sessionID}/browser/{requestID}/reply", "v2.session.browser.reply") + .seeded((ctx) => ctx.session({ title: "Browser request reply owner" })) + .at((ctx) => ({ + path: route("/api/session/{sessionID}/browser/{requestID}/reply", { + sessionID: ctx.state.id, + requestID: "brq_httpapi_missing", + }), + headers: ctx.headers(), + body: { value: { type: "string", value: "" } }, + })) + .json(404, object, "status"), + http.protected + .post("/api/session/{sessionID}/browser/{requestID}/refuse", "v2.session.browser.refuse") + .seeded((ctx) => ctx.session({ title: "Browser request refuse owner" })) + .at((ctx) => ({ + path: route("/api/session/{sessionID}/browser/{requestID}/refuse", { + sessionID: ctx.state.id, + requestID: "brq_httpapi_missing", + }), + headers: ctx.headers(), + body: { reason: "no page executor" }, + })) + .json(404, object, "status"), http.protected.get("/api/permission/saved", "v2.permission.saved.list").json(200, (body) => { object(body) array(body.data) diff --git a/packages/redrob/test/session/system.test.ts b/packages/redrob/test/session/system.test.ts index 2ad97be86a..3a112bc3e0 100644 --- a/packages/redrob/test/session/system.test.ts +++ b/packages/redrob/test/session/system.test.ts @@ -35,6 +35,32 @@ const skills: Skill.Info[] = [ location: "/tmp/manual-skill/SKILL.md", content: "# manual-skill", }, + // Two armable skills, because one proves too little: with a single candidate a wiring bug + // that armed EVERY skill and one that armed the right skill look identical. + { + name: "docs-skill", + description: "Docs skill.", + location: "/tmp/docs-skill/SKILL.md", + content: "# docs-skill\nHow to edit a document.", + autoInject: { keywords: ["spreadsheet", "pptx"], url: ["docs.google.com/**"] }, + }, + { + name: "deploy-skill", + description: "Deploy skill.", + location: "/tmp/deploy-skill/SKILL.md", + content: "# deploy-skill\nHow to roll back.", + autoInject: { keywords: ["rollback"] }, + }, + // A keyword carrying the characters that break an attribute. Skill files are author + // supplied, so this is reachable, and the first version of the armed-by attribute put the + // raw quote straight into the tag. + { + name: "quoted-skill", + description: "Quoted skill.", + location: "/tmp/quoted-skill/SKILL.md", + content: "# quoted-skill\nBody of the quoted skill.", + autoInject: { keywords: ['say "hello" '] }, + }, ] const build: Agent.Info = { @@ -159,8 +185,53 @@ describe("session.system", () => { }), ) - it.effect("MCP output includes connected server instructions", () => + it.effect("a keyword in the prompt arms that skill's body and no other", () => + Effect.gen(function* () { + const prompt = yield* SystemPrompt.Service + const output = yield* prompt.armedSkills(build, { prompt: "export this as a spreadsheet please" }) + const armed = output ?? (yield* Effect.fail(new NamedError.Unknown({ message: "nothing armed" }))) + + // The BODY, not the description: an armed skill is one the model is already reading. + expect(armed).toContain("How to edit a document.") + expect(armed).toContain('armed-by="keyword spreadsheet"') + // The other armable skill did not match, and must not ride along. + expect(armed).not.toContain("How to roll back.") + // A skill with no autoInject block never arms, however the prompt reads. + expect(armed).not.toContain("manual-skill") + }), + ) + + it.effect("a prompt matching nothing arms nothing at all", () => Effect.gen(function* () { + const prompt = yield* SystemPrompt.Service + // Mentions skills and documents in the abstract, matching no declared keyword. + expect(yield* prompt.armedSkills(build, { prompt: "what skills do you have?" })).toBeUndefined() + }), + ) + + it.effect("arming is denied with the skill permission, like the discovery list", () => + Effect.gen(function* () { + const prompt = yield* SystemPrompt.Service + const denied: Agent.Info = { ...build, permission: Permission.fromConfig({ skill: "deny" }) } + expect(yield* prompt.armedSkills(denied, { prompt: "export this as a spreadsheet" })).toBeUndefined() + }), + ) + + it.effect("a pattern with markup characters is escaped, not emitted raw", () => + Effect.gen(function* () { + const prompt = yield* SystemPrompt.Service + const output = yield* prompt.armedSkills(build, { prompt: 'please say "hello" ' }) + const armed = output ?? (yield* Effect.fail(new NamedError.Unknown({ message: "nothing armed" }))) + + expect(armed).toContain("Body of the quoted skill.") + expect(armed).toContain("armed-by=\"keyword say "hello" <now>\"") + // The tag must close where it is supposed to: one `">` on the opening line. + const opening = armed.split("\n").find((line) => line.includes("quoted-skill")) ?? "" + expect(opening.endsWith('">')).toBe(true) + }), + ) + + it.effect("MCP output includes connected server instructions", () => Effect.gen(function* () { const prompt = yield* SystemPrompt.Service const output = yield* prompt.mcp(build) diff --git a/packages/redrob/test/skill/skill.test.ts b/packages/redrob/test/skill/skill.test.ts index 352af217bd..baa7f08c5a 100644 --- a/packages/redrob/test/skill/skill.test.ts +++ b/packages/redrob/test/skill/skill.test.ts @@ -2,6 +2,7 @@ import { describe, expect } from "bun:test" import { LayerNode } from "@redrob-code/core/effect/layer-node" import { Effect, Layer } from "effect" import { Skill } from "../../src/skill" +import { SkillArming } from "@redrob-code/core/skill/arming" import { Discovery } from "../../src/skill/discovery" import { RuntimeFlags } from "../../src/effect/runtime-flags" import { EventV2Bridge } from "../../src/event-v2-bridge" @@ -320,6 +321,97 @@ description: A skill in the .claude/skills directory. ), ) + it.live("carries a disk skill's autoInject block through to the arming matcher", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + yield* Effect.promise(() => + Bun.write( + path.join(dir, ".redrob", "skill", "armed-skill", "SKILL.md"), + `--- +name: armed-skill +description: A skill that declares its own arming. +autoInject: + keywords: + - rollback + - "incident report" + url: + - status.example.com/** +--- + +# Armed Skill + +Instructions here. +`, + ), + ) + yield* Effect.promise(() => + Bun.write( + path.join(dir, ".redrob", "skill", "plain-skill", "SKILL.md"), + `--- +name: plain-skill +description: A skill with no arming block. +--- + +# Plain Skill +`, + ), + ) + + const skill = yield* Skill.Service + const list = (yield* skill.all()).filter((s) => s.location !== "") + const armable = list.find((x) => x.name === "armed-skill")! + const plain = list.find((x) => x.name === "plain-skill")! + + // The loader used to drop this field entirely, which left K-2's matcher with + // nothing to match on: every skill looked like `plain-skill`. + expect(armable.autoInject?.keywords).toEqual(["rollback", "incident report"]) + expect(armable.autoInject?.url).toEqual(["status.example.com/**"]) + expect(plain.autoInject).toBeUndefined() + + // End to end through the real matcher, on the real loaded values. + expect(SkillArming.armedNames({ skills: list, prompt: "time to ROLLBACK" })).toEqual(["armed-skill"]) + expect( + SkillArming.armedNames({ skills: list, prompt: "nothing here", url: "status.example.com/incidents/4" }), + ).toEqual(["armed-skill"]) + expect(SkillArming.armedNames({ skills: list, prompt: "nothing here" })).toEqual([]) + }), + { git: true }, + ), + ) + + it.live("keeps a skill whose autoInject block is malformed, minus the arming", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + yield* Effect.promise(() => + Bun.write( + path.join(dir, ".redrob", "skill", "broken-skill", "SKILL.md"), + `--- +name: broken-skill +description: Arming block in a shape the schema rejects. +autoInject: "rollback" +--- + +# Broken Skill +`, + ), + ) + + const skill = yield* Skill.Service + const item = (yield* skill.all()).find((x) => x.name === "broken-skill") + + // The documented failure mode: the skill still loads and stays selectable by + // name, and only the arming behaviour is lost. Dropping the whole file would + // lose a working skill over an optional key. + expect(item).toBeDefined() + expect(item!.content).toContain("Broken Skill") + expect(item!.autoInject).toBeUndefined() + }), + { git: true }, + ), + ) + it.effect("exposes tagged expected skill failure classes", () => Effect.sync(() => { const invalid = new Skill.InvalidError({ path: "/tmp/SKILL.md", message: "Invalid skill frontmatter" }) diff --git a/packages/redrob/test/tool/browser-channel.ts b/packages/redrob/test/tool/browser-channel.ts new file mode 100644 index 0000000000..efa2145ce0 --- /dev/null +++ b/packages/redrob/test/tool/browser-channel.ts @@ -0,0 +1,27 @@ +import { BrowserRequestV1 } from "@redrob-code/core/browser-request" +import { Effect, Layer } from "effect" + +/** + * PA-10 test double for the browser channel. + * + * Shared because four suites build the real code-mode tool, and that tool now resolves + * `page` through this service. Without it they fail to construct rather than failing an + * assertion, which is a worse signal. + * + * `ask` fails immediately instead of sleeping to the service's real deadline. The deadline + * is tested where it lives (`packages/core/test/browser-request.test.ts`); paying 20 seconds + * for it in every suite that merely needs the tool to exist would buy nothing. + */ +export function unansweredBrowser(): BrowserRequestV1.Interface { + return { + ask: (input) => Effect.fail(new BrowserRequestV1.UnansweredError({ action: input.command.action })), + reply: () => Effect.void, + refuse: () => Effect.void, + list: () => Effect.succeed([]), + } +} + +/** The layer form, for a harness that just needs the service present. */ +export function unansweredBrowserLayer() { + return Layer.mock(BrowserRequestV1.Service, unansweredBrowser()) +} diff --git a/packages/redrob/test/tool/code-mode-integration.test.ts b/packages/redrob/test/tool/code-mode-integration.test.ts index 671acd8962..154911d627 100644 --- a/packages/redrob/test/tool/code-mode-integration.test.ts +++ b/packages/redrob/test/tool/code-mode-integration.test.ts @@ -18,6 +18,7 @@ import { type Tool as MCPToolDef, } from "@modelcontextprotocol/sdk/types.js" import { Cause, Effect, Exit, Layer } from "effect" +import { unansweredBrowserLayer } from "./browser-channel" const PNG = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==" @@ -152,6 +153,7 @@ async function buildTool() { tools: () => Effect.succeed(mcpTools), clients: () => Effect.succeed({ [SERVER]: {} as any }), }), + unansweredBrowserLayer(), ) return { tool: await Effect.runPromise(CodeModeTool.pipe(Effect.flatMap(Tool.init), Effect.provide(layer))), diff --git a/packages/redrob/test/tool/code-mode.test.ts b/packages/redrob/test/tool/code-mode.test.ts index fde63971d7..2fe90a6cd6 100644 --- a/packages/redrob/test/tool/code-mode.test.ts +++ b/packages/redrob/test/tool/code-mode.test.ts @@ -11,6 +11,7 @@ import { Tool } from "@/tool/tool" import * as Truncate from "@/tool/truncate" import { MessageID, SessionID } from "@/session/schema" import { Cause, Effect, Exit, Layer, Schema } from "effect" +import { unansweredBrowserLayer } from "./browser-channel" const ctx: Tool.Context = { sessionID: SessionID.make("ses_code-mode"), @@ -60,6 +61,7 @@ function harness(input: { tools: () => Effect.succeed(input.mcpTools), clients: () => Effect.succeed(Object.fromEntries(input.servers.map((name) => [name, {} as any]))), }), + unansweredBrowserLayer(), ) } diff --git a/packages/redrob/test/tool/document.test.ts b/packages/redrob/test/tool/document.test.ts index 246e394113..4b32c3129d 100644 --- a/packages/redrob/test/tool/document.test.ts +++ b/packages/redrob/test/tool/document.test.ts @@ -3,6 +3,7 @@ import * as fs from "node:fs/promises" import * as os from "node:os" import * as path from "node:path" import { Cause, Effect, Exit, Layer } from "effect" +import { unansweredBrowserLayer } from "./browser-channel" import { Agent } from "@/agent/agent" import { MCP } from "@/mcp" import { Plugin } from "@/plugin" @@ -95,6 +96,7 @@ const runProgram = (code: string, directory: string) => updatePart: (part: unknown) => Effect.succeed(part as never), } as never), Layer.mock(MCP.Service, { tools: () => Effect.succeed({}), clients: () => Effect.succeed({}) }), + unansweredBrowserLayer(), ), ), ), diff --git a/packages/redrob/test/tool/domain.test.ts b/packages/redrob/test/tool/domain.test.ts index 9315ac2864..a2dc9c3c57 100644 --- a/packages/redrob/test/tool/domain.test.ts +++ b/packages/redrob/test/tool/domain.test.ts @@ -14,10 +14,12 @@ import { pageTools, sessionChannel, unavailableChannel, + bridgedPage, unavailablePage, } from "@/tool/domain" import { MessageID, PartID, SessionID } from "@/session/schema" import type { SessionV1 } from "@redrob-code/core/v1/session" +import { BrowserRequestV1 } from "@redrob-code/core/browser-request" import { Cause, Effect, Exit, Layer } from "effect" const sessionID = SessionID.make("ses_domain") @@ -48,7 +50,25 @@ function recordingSessions() { return { parts, sessions } } -function harness(sessions: Record) { +/** + * A browser channel that never gets a client. The default for the harness, because that is + * the state of a session with no browser attached, and it is the state the PA-10 refusal + * messages are written for. + * + * It fails immediately rather than sleeping to the service's real deadline: the deadline + * itself is the service's own test, and a 20s sleep in every page test here would buy + * nothing. + */ +function unansweredBrowser(): BrowserRequestV1.Interface { + return { + ask: (input) => Effect.fail(new BrowserRequestV1.UnansweredError({ action: input.command.action })), + reply: () => Effect.void, + refuse: () => Effect.void, + list: () => Effect.succeed([]), + } +} + +function harness(sessions: Record, browser: BrowserRequestV1.Interface = unansweredBrowser()) { return Layer.mergeAll( Layer.mock(Plugin.Service, { trigger: ((_name, _input, output) => Effect.succeed(output)) as Plugin.Interface["trigger"], @@ -59,27 +79,28 @@ function harness(sessions: Record) { Layer.mock(Agent.Service, { get: () => Effect.succeed({ name: "build", permission: [] } as any) }), Layer.mock(Session.Service, sessions as any), Layer.mock(MCP.Service, { tools: () => Effect.succeed({}), clients: () => Effect.succeed({}) }), + Layer.mock(BrowserRequestV1.Service, browser), ) } /** Runs one Code Mode program through the real `execute` tool. */ -function run(code: string, sessions: Record) { +function run(code: string, sessions: Record, browser?: BrowserRequestV1.Interface) { return Effect.runPromise( CodeModeTool.pipe( Effect.flatMap(Tool.init), Effect.flatMap((def) => def.execute({ code }, ctx)), - Effect.provide(harness(sessions)), + Effect.provide(harness(sessions, browser)), ), ) } /** Program failures die at the tool boundary; recover the defect for message assertions. */ -async function failureOf(code: string, sessions: Record) { +async function failureOf(code: string, sessions: Record, browser?: BrowserRequestV1.Interface) { const exit = await Effect.runPromise( CodeModeTool.pipe( Effect.flatMap(Tool.init), Effect.flatMap((def) => def.execute({ code }, ctx)), - Effect.provide(harness(sessions)), + Effect.provide(harness(sessions, browser)), Effect.exit, ), ) @@ -138,7 +159,7 @@ describe("K-1 typed domain objects", () => { }) }) - describe("page is unavailable rather than faked", () => { + describe("page with no browser attached refuses rather than faking", () => { test("every method refuses, naming the missing capability", async () => { const page = unavailablePage() const calls = [ @@ -154,7 +175,7 @@ describe("K-1 typed domain objects", () => { expect(Exit.isFailure(exit)).toBe(true) const error = Exit.isFailure(exit) ? Cause.squash(exit.cause) : undefined expect(error).toBeInstanceOf(DomainUnavailableError) - expect((error as DomainUnavailableError).message).toContain("no browser-page control surface") + expect((error as DomainUnavailableError).message).toContain("no browser client is attached") } }) @@ -162,7 +183,8 @@ describe("K-1 typed domain objects", () => { const { sessions } = recordingSessions() const message = await failureOf("return await page.text({})", sessions) expect(message).toContain("`page` domain object is not available") - expect(message).toContain("no browser-page control surface") + // The bridge's reason, not the no-channel one: a channel exists, nobody answered on it. + expect(message).toContain("no browser client answered page.text") }) test("the refusal carries the named object", () => { @@ -183,3 +205,95 @@ describe("K-1 typed domain objects", () => { expect(PartID.ascending()).toStartWith("prt_") }) }) + +/** + * PA-10. These are the tests that say the channel carries real work, rather than that it + * refuses politely: a program reads what the attached client answered, and a client that + * answers with the wrong shape is reported as a protocol error instead of being read as a + * fact about the page. + */ +describe("PA-10 page over the browser channel", () => { + /** A client that answers every action, recording what it was asked. */ + function answeringBrowser(value: BrowserRequestV1.Value) { + const asked: BrowserRequestV1.Command[] = [] + const browser: BrowserRequestV1.Interface = { + ask: (input) => + Effect.sync(() => { + asked.push(input.command) + return value + }), + reply: () => Effect.void, + refuse: () => Effect.void, + list: () => Effect.succeed([]), + } + return { asked, browser } + } + + test("page.text returns what the client answered", async () => { + const { sessions } = recordingSessions() + const { asked, browser } = answeringBrowser({ type: "string", value: "the page said this" }) + const result = await run("return await page.text({})", sessions, browser) + expect(result.output).toContain("the page said this") + expect(asked.map((command) => command.action)).toStrictEqual(["page.text"]) + }) + + test("the selector and the submit flag reach the client, not just the action name", async () => { + const { sessions } = recordingSessions() + const { asked, browser } = answeringBrowser({ type: "void" }) + await run('return await page.type({ selector: "#q", text: "hello", submit: true })', sessions, browser) + expect(asked).toStrictEqual([{ action: "page.type", selector: "#q", text: "hello", submit: true }]) + }) + + test("page.query returns the client's nodes", async () => { + const { sessions } = recordingSessions() + const { browser } = answeringBrowser({ + type: "nodes", + value: [{ selector: "h1", text: "Title", attributes: { id: "top" } }], + }) + const result = await run('return (await page.query({ selector: "h1" }))[0].text', sessions, browser) + expect(result.output).toContain("Title") + }) + + test("a wrong-shaped answer is a protocol error, NOT an empty result", async () => { + const { sessions } = recordingSessions() + // The client answers `page.query` with a string. Reading that as zero nodes would tell + // the model "nothing matched the selector", which is a claim about the page that nobody + // made. + const { browser } = answeringBrowser({ type: "string", value: "h1" }) + const message = await failureOf('return await page.query({ selector: "h1" })', sessions, browser) + expect(message).toContain("answered page.query with a string value where nodes was required") + }) + + test("a refusal from the client carries its own sentence through", async () => { + const { sessions } = recordingSessions() + const browser: BrowserRequestV1.Interface = { + ask: () => + Effect.fail(new BrowserRequestV1.RefusedError({ action: "page.click", reason: "that tab is not shared" })), + reply: () => Effect.void, + refuse: () => Effect.void, + list: () => Effect.succeed([]), + } + const message = await failureOf('return await page.click({ selector: "h1" })', sessions, browser) + expect(message).toContain("the browser refused page.click: that tab is not shared") + }) + + test("bridgedPage asks with the session that owns the run", async () => { + const asked: BrowserRequestV1.AskInput[] = [] + const page = bridgedPage({ + sessionID, + browser: { + ask: (input) => + Effect.sync(() => { + asked.push(input) + return { type: "string", value: "https://example.invalid/" } as const + }), + reply: () => Effect.void, + refuse: () => Effect.void, + list: () => Effect.succeed([]), + }, + }) + const url = await Effect.runPromise(page.url()) + expect(url).toBe("https://example.invalid/") + expect(asked.map((input) => input.sessionID)).toStrictEqual([sessionID]) + }) +}) diff --git a/packages/redrob/test/tool/page-skill.test.ts b/packages/redrob/test/tool/page-skill.test.ts new file mode 100644 index 0000000000..200c9436c1 --- /dev/null +++ b/packages/redrob/test/tool/page-skill.test.ts @@ -0,0 +1,65 @@ +/** + * PA-9's page-control skill, held to the `page` surface it describes. + * + * Checked BOTH ways, for the same reason K-3's document is: a skill that names a call which + * does not exist produces an agent that calls it and fails, and a call the document never + * mentions is a capability the agent will not reach for. The real set comes from the bound + * tool namespace, not from a list retyped here -- a list retyped here would agree with the + * document while both disagreed with the code. + */ +import { describe, expect, test } from "bun:test" +import { Schema } from "effect" +import { SkillPlugin } from "@redrob-code/core/plugin/skill" +import { SkillV2 } from "@redrob-code/core/skill" +import { domainTools, unavailableChannel, unavailablePage } from "@/tool/domain" + +const DOC = SkillPlugin.PageControlContent + +/** Every `page.` the document mentions, deduplicated. */ +const mentioned = new Set([...DOC.matchAll(/\bpage\.([A-Za-z]+)\b/g)].map((match) => `page.${match[1]}`)) + +/** Every `page.` the engine actually binds. */ +const real = new Set( + Object.keys( + domainTools({ page: unavailablePage("test"), channel: unavailableChannel("test") }).page ?? {}, + ).map((method) => `page.${method}`), +) + +describe("page-control skill", () => { + test("names every page call that exists", () => { + expect(real.size).toBeGreaterThan(0) + for (const call of real) expect(mentioned).toContain(call) + }) + + test("names no page call that does not exist", () => { + for (const call of mentioned) expect(real).toContain(call) + }) + + test("the count it claims is the count there is", () => { + // The document opens with "Six calls. There is no seventh". If the surface grows, that + // sentence becomes a lie and this is where it is caught. + expect(real.size).toBe(6) + expect(DOC).toContain("Six calls") + }) + + test("it is shipped, and armed by keyword rather than by URL", () => { + const info = SkillPlugin.BuiltinSkills.find((skill) => skill.name === "page-control") + expect(info).toBeDefined() + expect(info?.content).toBe(DOC) + // Decoded through the real schema, the way the loader does: `Info.make` does not run the + // decoder, so a field in a shape the schema rejects would otherwise reach a session. + const decoded = Schema.decodeUnknownSync(SkillV2.Info)(JSON.parse(JSON.stringify(info))) + expect(decoded.name).toBe("page-control") + expect(decoded.autoInject?.keywords?.length ?? 0).toBeGreaterThan(0) + // No URL globs on purpose: a page skill armed by URL would be in the prompt of every + // session on any page, which is every session with a browser attached. + expect(info?.autoInject?.url ?? []).toEqual([]) + }) + + test("it says a refusal is about the session and an empty result about the page", () => { + // The distinction the engine's tagged Value union exists to preserve. A document that + // blurred it would teach the agent to read "nothing matched" as "no browser". + expect(DOC).toContain("DomainUnavailableError") + expect(DOC).toContain("different from a refusal") + }) +}) diff --git a/packages/schema/src/browser-request.ts b/packages/schema/src/browser-request.ts new file mode 100644 index 0000000000..25123132fc --- /dev/null +++ b/packages/schema/src/browser-request.ts @@ -0,0 +1,122 @@ +export * as BrowserRequest from "./browser-request" + +import { Schema } from "effect" +import { define, inventory } from "./event" +import { ascending } from "./identifier" +import { optional, statics } from "./schema" +import { SessionID } from "./session-id" + +/** + * PA-10: one browser action the engine wants performed, and the answer coming back. + * + * The engine process cannot reach a browser page. The direction that DOES exist is the + * client subscribing to `GET /api/event`, so a browser action is modelled as a pending + * request published on that stream and settled by an HTTP reply — the same shape + * `QuestionV2` already uses for asking a person something. + * + * Nothing here opens a socket, and nothing here is a listener the engine owns. That is the + * point: a loopback endpoint that can drive a browser is reachable by every other local + * process, and a web page can reach it too (DNS rebinding, missing Host/Origin checks). + * The client dials the engine, never the reverse. + */ +export const ID = Schema.String.check(Schema.isStartsWith("brq")).pipe( + Schema.brand("BrowserRequest.ID"), + statics((schema) => { + const create = () => schema.make("brq_" + ascending()) + return { + create, + ascending: (id?: string) => (id === undefined ? create() : schema.make(id)), + } + }), +) +export type ID = typeof ID.Type + +/** A node a page query returned. Mirrors `PageNode` in the engine's domain module. */ +export const Node = Schema.Struct({ + selector: Schema.String, + text: Schema.String, + attributes: Schema.Record(Schema.String, Schema.String), +}).annotate({ identifier: "BrowserRequest.Node" }) +export interface Node extends Schema.Schema.Type {} + +const Url = Schema.Struct({ action: Schema.Literal("page.url") }) +const Text = Schema.Struct({ action: Schema.Literal("page.text") }) +const Query = Schema.Struct({ + action: Schema.Literal("page.query"), + selector: Schema.String, + limit: Schema.Number.pipe(optional), +}) +const Click = Schema.Struct({ action: Schema.Literal("page.click"), selector: Schema.String }) +const Type = Schema.Struct({ + action: Schema.Literal("page.type"), + selector: Schema.String, + text: Schema.String, + submit: Schema.Boolean.pipe(optional), +}) +const Navigate = Schema.Struct({ action: Schema.Literal("page.navigate"), url: Schema.String }) + +/** + * What the client is being asked to do, discriminated by `action`. + * + * The action names are the `page` interface's method names with their object prefixed, so a + * refusal, a log line and a skill's own source all name the same call. + */ +export const Command = Schema.Union([Url, Text, Query, Click, Type, Navigate]) + .pipe(Schema.toTaggedUnion("action")) + .annotate({ identifier: "BrowserRequest.Command" }) +export type Command = typeof Command.Type + +/** The action names, for a client that wants to check it handles all of them. */ +export const ACTIONS = ["page.url", "page.text", "page.query", "page.click", "page.type", "page.navigate"] as const +export type Action = (typeof ACTIONS)[number] + +export const Request = Schema.Struct({ + id: ID, + sessionID: SessionID, + command: Command, +}).annotate({ identifier: "BrowserRequest.Request" }) +export interface Request extends Schema.Schema.Type {} + +/** + * The value a settled request carries. + * + * Deliberately a tagged union rather than one permissive `unknown`: the engine knows which + * shape each action must produce, so a client answering `page.query` with a string is a + * protocol error the engine can name, not an empty node list the model reads as "nothing + * matched". The two are different claims. + */ +const StringValue = Schema.Struct({ type: Schema.Literal("string"), value: Schema.String }) +const NodesValue = Schema.Struct({ type: Schema.Literal("nodes"), value: Schema.Array(Node) }) +const VoidValue = Schema.Struct({ type: Schema.Literal("void") }) + +export const Value = Schema.Union([StringValue, NodesValue, VoidValue]) + .pipe(Schema.toTaggedUnion("type")) + .annotate({ identifier: "BrowserRequest.Value" }) +export type Value = typeof Value.Type + +export const Reply = Schema.Struct({ + value: Value, +}).annotate({ identifier: "BrowserRequest.Reply" }) +export interface Reply extends Schema.Schema.Type {} + +/** + * Why the client could not do it: the tab is gone, the selector matched nothing, the user + * has not exposed this tab. A refusal is a normal outcome, not an exception, so it travels + * as its own route rather than as a missing reply that would stall until the deadline. + */ +export const Refusal = Schema.Struct({ + reason: Schema.String.annotate({ description: "Why the action could not be performed, in one sentence." }), +}).annotate({ identifier: "BrowserRequest.Refusal" }) +export interface Refusal extends Schema.Schema.Type {} + +const Asked = define({ type: "browser.request.asked", schema: Request.fields }) +const Answered = define({ + type: "browser.request.answered", + schema: { sessionID: SessionID, requestID: ID }, +}) +const Refused = define({ + type: "browser.request.refused", + schema: { sessionID: SessionID, requestID: ID, reason: Schema.String }, +}) + +export const Event = { Asked, Answered, Refused, Definitions: inventory(Asked, Answered, Refused) } diff --git a/packages/schema/src/event-manifest.ts b/packages/schema/src/event-manifest.ts index b681362e83..128bec718d 100644 --- a/packages/schema/src/event-manifest.ts +++ b/packages/schema/src/event-manifest.ts @@ -1,5 +1,6 @@ export * as EventManifest from "./event-manifest" +import { BrowserRequest } from "./browser-request" import { Catalog } from "./catalog" import { Durable } from "./durable-event-manifest" import { Event } from "./event" @@ -52,6 +53,7 @@ const featureDefinitions = Event.inventory( ...FileSystemWatcher.Event.Definitions, ...Pty.Event.Definitions, ...Question.Event.Definitions, + ...BrowserRequest.Event.Definitions, ) export const ServerDefinitions = Event.inventory( diff --git a/packages/server/src/handlers.ts b/packages/server/src/handlers.ts index ab62df08e9..5a1dc10f79 100644 --- a/packages/server/src/handlers.ts +++ b/packages/server/src/handlers.ts @@ -12,6 +12,7 @@ import { EventHandler } from "./handlers/event" import { AgentHandler } from "./handlers/agent" import { HealthHandler } from "./handlers/health" import { PtyHandler } from "./handlers/pty" +import { BrowserRequestHandler } from "./handlers/browser-request" import { QuestionHandler } from "./handlers/question" import { ReferenceHandler } from "./handlers/reference" import { LocationHandler } from "./handlers/location" @@ -38,6 +39,7 @@ export const handlers = Layer.mergeAll( EventHandler, PtyHandler, QuestionHandler, + BrowserRequestHandler, ReferenceHandler, ProjectCopyHandler, ChatCompletionHandler, diff --git a/packages/server/src/handlers/browser-request.ts b/packages/server/src/handlers/browser-request.ts new file mode 100644 index 0000000000..0591d4db13 --- /dev/null +++ b/packages/server/src/handlers/browser-request.ts @@ -0,0 +1,71 @@ +import { BrowserRequestV1 } from "@redrob-code/core/browser-request" +import { Effect } from "effect" +import { HttpApiBuilder, HttpApiSchema } from "effect/unstable/httpapi" +import { Api } from "../api" +import { BrowserRequestNotFoundError } from "@redrob-code/protocol/errors" +import { response } from "../location" + +/** + * PA-10 server half. Mirrors the question handler, including its ownership check: a reply + * is accepted only for a request that belongs to the session in the path, so one session's + * client cannot settle another's pending browser action. + */ +function missingRequest(id: BrowserRequestV1.ID) { + return new BrowserRequestNotFoundError({ + requestID: id, + // Says the likely cause, because the expired case is normal rather than a client bug. + message: `Browser request not found: ${id}. It was answered already, or it expired on the engine's deadline.`, + }) +} + +export const BrowserRequestHandler = HttpApiBuilder.group(Api, "server.browser", (handlers) => + Effect.gen(function* () { + const withOwnedRequest = Effect.fnUntraced(function* ( + sessionID: BrowserRequestV1.Request["sessionID"], + requestID: BrowserRequestV1.ID, + use: (browser: BrowserRequestV1.Interface) => Effect.Effect, + ) { + const browser = yield* BrowserRequestV1.Service + const request = (yield* browser.list()).find((request) => request.id === requestID) + if (!request || request.sessionID !== sessionID) return yield* missingRequest(requestID) + return yield* use(browser) + }) + + return handlers + .handle( + "browser.request.list", + Effect.fn(function* () { + return yield* response((yield* BrowserRequestV1.Service).list()) + }), + ) + .handle( + "session.browser.list", + Effect.fn(function* (ctx) { + const requests = yield* (yield* BrowserRequestV1.Service).list() + return { data: requests.filter((request) => request.sessionID === ctx.params.sessionID) } + }), + ) + .handle( + "session.browser.reply", + Effect.fn(function* (ctx) { + yield* withOwnedRequest(ctx.params.sessionID, ctx.params.requestID, (browser) => + browser + .reply({ requestID: ctx.params.requestID, value: ctx.payload.value }) + .pipe(Effect.catchTag("BrowserRequestV1.NotFoundError", () => missingRequest(ctx.params.requestID))), + ) + return HttpApiSchema.NoContent.make() + }), + ) + .handle( + "session.browser.refuse", + Effect.fn(function* (ctx) { + yield* withOwnedRequest(ctx.params.sessionID, ctx.params.requestID, (browser) => + browser + .refuse({ requestID: ctx.params.requestID, reason: ctx.payload.reason }) + .pipe(Effect.catchTag("BrowserRequestV1.NotFoundError", () => missingRequest(ctx.params.requestID))), + ) + return HttpApiSchema.NoContent.make() + }), + ) + }), +)