Skip to content

Commit b4cf6c9

Browse files
committed
Merge remote-tracking branch 'origin/main' into gate393
# Conflicts: # CHANGELOG.md # server/src/components/sandboxed-routes.ts
2 parents 364f4a8 + d97641e commit b4cf6c9

21 files changed

Lines changed: 538 additions & 42 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,54 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
88

99
## Unreleased
1010

11-
### A sandboxed component cannot be saved with a blank title
12-
13-
`POST /api/sandboxed` accepted a title of spaces. The slug has a pattern of its own and refuses one,
14-
but the title is a separate field with no check, so the component reached the grid with nothing to
15-
read: an administrator could not tell it from the next one, and a Bot granted it was told the
16-
component is called " ". The title is now trimmed, and one that is only whitespace is refused the
17-
way a missing one always was. The two endpoints of the same shape, `POST /api/servers/custom` and
18-
`POST /api/skills`, already did this.
11+
### Wiping a computer is recorded even if clearing its stored state fails
12+
13+
Resetting a computer destroys the profile first and wrote the audit row last, after two Postgres
14+
deletes. A connection reset, a failover or a statement timeout in either delete threw before the row
15+
was written, so the most destructive button in the product could leave a wiped computer -- every
16+
login gone, no undo -- with nothing on the trail to say who wiped it or when. The row is now written
17+
as soon as the profile is gone, which is the point after which nothing can be put back. A failure in
18+
either delete is still reported to the caller.
19+
### Pressing Stop is recorded as a stop, not as a computer that is not running
20+
21+
The computer transport answered a Stop correctly only when it arrived before the request left. The
22+
caller's signal is handed to `fetch` precisely so a Stop can also land mid-action, and a fetch
23+
aborted that way rejects with an `AbortError`, which fell through to the message for a computer that
24+
cannot be reached. The person was told their own click had failed because the assistant's computer
25+
was not running, and the gateway wrote that sentence into the action's audit row as its failure --
26+
so a deliberate stop read back as an outage. A genuinely unreachable computer and a timeout still
27+
say what they said.
28+
### A component the server refused is no longer drawn anyway
29+
30+
A sandboxed component asks the server at call time whether the Bot may still use it, and a refusal
31+
is recorded so the drawing can be replaced with a card saying so. The renderer looked that refusal
32+
up under `props.toolCall.id`, which is the shape a tool HANDLER is given; a renderer's props carry
33+
the id flat, as `toolCallId`. The lookup key was therefore always undefined, the refusal was never
34+
found, and the component rendered as though it had been allowed -- so revoking a component from a
35+
Bot did not take effect on screen until the five-second grant poll caught up, and a failed decision
36+
request showed nothing at all.
37+
### A component whose name has a stray space is the same component
38+
39+
The catalogue announcement asked whether each component's `name`, `title`, `kind` and `description`
40+
were more than whitespace, and then published the untrimmed strings. A `name` is a component's
41+
identity -- it is what `syncCatalogue` compares against what is already published, what `decide` and
42+
`listForAgent` look up, and what a grant names -- so a build shipping `" weatherPanel "` added a
43+
second catalogue row beside `weatherPanel`: published, ungranted by anybody, and impossible to hold
44+
a Bot back from under the name people use. The four fields are now stored as the strings the guard
45+
approved.
46+
### A password with a `%` in it says so, instead of failing as `URI error`
47+
48+
`DATABASE_URL` is taken apart before it reaches Bun, and each part is percent-decoded. A part
49+
holding a `%` that starts no escape -- `postgres://openbot:100%pure@host:5432/openbot`, which a
50+
generated password produces often enough -- is a string `new URL` accepts and `decodeURIComponent`
51+
rejects, so the server stopped with `URIError: URI error` and named neither the variable nor the
52+
part. It now refuses with the same kind of sentence as every other malformed address: which part is
53+
wrong, and that a literal `%` must be written `%25`.
54+
55+
A correctly encoded password is unaffected.
56+
### An empty Bot `PORT` is unset, so NaN never reaches Bun.serve
57+
58+
`PORT=` on `agent-bot` and `agent-langgraph` used to parse as `NaN` (`??` does not treat empty as absent) and `Bun.serve` bound an ephemeral port while compose still published 4200/4201. A prefix typo (`42o0`) started on 42. Empty now means the shipped default; anything that is not a whole port number refuses to start.
1959

2060
### The server connects to Postgres on Windows, and `localhost` is no longer a coin toss
2161

‎agent-bot/src/index.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { EventEncoder } from "@ag-ui/encoder";
33
import { serve } from "bun";
44
import OpenAI from "openai";
55
import { hasManagedAgentToken } from "../../shared/agent-authorisation";
6+
import { listenPort } from "../../shared/listen-port";
67
import { toProviderMessages } from "./history";
78

89
/**
@@ -16,7 +17,12 @@ import { toProviderMessages } from "./history";
1617
* running where its effects are visible to the person watching.
1718
*/
1819

19-
const PORT = Number.parseInt(process.env.PORT ?? "4200", 10);
20+
const resolvedPort = listenPort(process.env.PORT, 4200);
21+
if (!resolvedPort.ok) {
22+
console.error(resolvedPort.reason);
23+
process.exit(1);
24+
}
25+
const PORT = resolvedPort.port;
2026
const MANAGED_AGENT_TOKEN = process.env.MANAGED_AGENT_TOKEN?.trim();
2127
if (!MANAGED_AGENT_TOKEN) {
2228
console.error(

‎agent-langgraph/src/index.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import { ChatOpenAI } from "@langchain/openai";
1313
import { serve } from "bun";
1414
import { hasManagedAgentToken } from "../../shared/agent-authorisation";
15+
import { listenPort } from "../../shared/listen-port";
1516
import { toLangChainMessages } from "./history";
1617
import { readReasoningEffort } from "./model-options";
1718
import { streamRun } from "./stream";
@@ -36,7 +37,12 @@ import { streamRun } from "./stream";
3637
* The graph provides model orchestration without changing that contract.
3738
*/
3839

39-
const PORT = Number.parseInt(process.env.PORT ?? "4201", 10);
40+
const resolvedPort = listenPort(process.env.PORT, 4201);
41+
if (!resolvedPort.ok) {
42+
console.error(resolvedPort.reason);
43+
process.exit(1);
44+
}
45+
const PORT = resolvedPort.port;
4046
const MANAGED_AGENT_TOKEN = process.env.MANAGED_AGENT_TOKEN?.trim();
4147
if (!MANAGED_AGENT_TOKEN) {
4248
console.error(

‎app/src/components/computer/live-screen.tsx‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,30 @@ export function LiveScreen({ computerId, driving, onProblem }: Props) {
6464
};
6565

6666
socket.onmessage = async (event) => {
67-
let message: {
67+
let parsed: unknown;
68+
try {
69+
parsed = JSON.parse(String(event.data));
70+
} catch {
71+
return;
72+
}
73+
// A frame that is not an object (`null`, a number, a string, an array) has
74+
// no `type` to read: reaching for it throws a `TypeError` inside this
75+
// handler and stops the live view from drawing further frames. Drop it.
76+
if (
77+
typeof parsed !== "object" ||
78+
parsed === null ||
79+
Array.isArray(parsed)
80+
) {
81+
return;
82+
}
83+
const message = parsed as {
6884
type: string;
6985
data?: string;
7086
width?: number;
7187
height?: number;
7288
error?: string;
7389
};
74-
try {
75-
message = JSON.parse(String(event.data));
76-
} catch {
77-
return;
78-
}
90+
if (typeof message.type !== "string") return;
7991
if (message.type === "error") {
8092
onProblem?.(message.error ?? "The screen could not be shown.");
8193
return;

‎app/src/lib/copilot/sandboxed-tools.tsx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,20 @@ function SandboxedTool({
7575
const isHeld = description !== undefined;
7676

7777
const render = useCallback(
78+
// `toolCallId`, not `toolCall.id`: a renderer's props carry the id flat,
79+
// which is the shape gallery-tools and computer-tools already read and
80+
// the one the library's own DefaultToolCallRenderer destructures. The
81+
// handler below is the other shape, `context.toolCall.id`, and reading
82+
// that one here meant the key was always undefined -- so a refusal the
83+
// server had just recorded was never found, and the component the server
84+
// refused was drawn instead of the card saying it was not allowed.
7885
(props: {
7986
args?: Record<string, unknown>;
8087
status?: string;
81-
toolCall?: { id?: string };
88+
toolCallId?: string;
8289
}) => {
83-
const refusal = props.toolCall?.id
84-
? refusals.get(props.toolCall.id)
90+
const refusal = props.toolCallId
91+
? refusals.get(props.toolCallId)
8592
: undefined;
8693
if (refusal) {
8794
return <RefusedCard reason={refusal} title={component.name} />;

‎app/tests/skill-creator-slug.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, test } from "bun:test";
22
import { readFileSync } from "node:fs";
3+
import { fileURLToPath } from "node:url";
34
import { parse } from "yaml";
45
import { SKILL_CREATOR_SLUG } from "@/lib/skills/proposal";
56

@@ -15,7 +16,9 @@ import { SKILL_CREATOR_SLUG } from "@/lib/skills/proposal";
1516
* Read from the file rather than from a fixture, because the file is what ships.
1617
*/
1718

18-
const packageDir = new URL("../../examples/fintech", import.meta.url).pathname;
19+
const packageDir = fileURLToPath(
20+
new URL("../../examples/fintech", import.meta.url),
21+
);
1922

2023
const shipped = parse(readFileSync(`${packageDir}/skills.yaml`, "utf8")) as {
2124
skills: { slug: string; title: string; instructions: string }[];

‎server/src/components/routes.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,19 @@ export function createComponentRoutes(
105105
) {
106106
return [];
107107
}
108-
return [{ name, title, kind, description }];
108+
// Trimmed, because that is the string the guard above just approved. A component's `name` is
109+
// its identity -- `syncCatalogue` compares it against what is already published, `decide` and
110+
// `listForAgent` look it up by it, and a grant names it -- so publishing " weatherPanel "
111+
// adds a second component beside `weatherPanel` that nobody has granted and no Bot can be
112+
// held back from by the name people use.
113+
return [
114+
{
115+
name: name.trim(),
116+
title: title.trim(),
117+
kind: kind.trim(),
118+
description: description.trim(),
119+
},
120+
];
109121
});
110122

111123
const { added } = await store.syncCatalogue(valid);

‎server/src/components/sandboxed-routes.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,18 @@ export function createSandboxedRoutes(
6363
sampleArguments?: Record<string, unknown>;
6464
} | null;
6565

66-
if (!body?.slug || !body.title?.trim()) {
66+
if (
67+
typeof body?.slug !== "string" ||
68+
!body.slug.trim() ||
69+
typeof body?.title !== "string" ||
70+
!body.title.trim()
71+
) {
6772
return context.json({ error: "A name and a title are required." }, 400);
6873
}
6974

7075
try {
7176
const component = await store.save({
72-
slug: body.slug,
77+
slug: body.slug.trim(),
7378
title: body.title.trim(),
7479
description: body.description ?? "",
7580
html: body.html ?? "",

‎server/src/computer/client.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,18 @@ export function createComputerTransport(
155155
: AbortSignal.timeout(timeoutMs),
156156
});
157157
} catch (error) {
158+
/*
159+
* The caller's own abort is answered first, and says what the check above the fetch says.
160+
*
161+
* The signal is handed to fetch precisely so a Stop can land mid-flight, and a fetch aborted
162+
* that way rejects with an AbortError, which is neither a TimeoutError nor a computer that is
163+
* not running. Both of the other answers are statements about the infrastructure, and this
164+
* message is not only read by the model: the gateway writes it into the action's audit row as
165+
* `failure`, so a person pressing Stop was recorded as an outage.
166+
*/
167+
if (caller?.aborted) {
168+
throw new ComputerUnavailableError("The action was stopped.");
169+
}
158170
throw new ComputerUnavailableError(
159171
error instanceof Error && error.name === "TimeoutError"
160172
? "The assistant's computer did not respond in time."

‎server/src/computer/gateway.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,22 @@ export function createComputerGateway(
713713
*/
714714
async resetComputer(botId: string, actor: ActionActor) {
715715
const result = await provider.reset(botId);
716+
/*
717+
* The row goes in HERE, before the two deletes below, because this line is the point of no
718+
* return: the profile is already gone and nothing after it can put the logins back.
719+
*
720+
* Both clears are Postgres deletes, and a connection reset, a failover or a statement timeout
721+
* in either used to throw before the row was written -- leaving a computer wiped with nothing
722+
* on the trail to say who wiped it, which is the one outcome the note above rules out. The
723+
* failure still propagates, so the caller is told the clears did not finish.
724+
*/
725+
await writeControlEvent(auditStore, "computer.reset", {
726+
botId,
727+
actor,
728+
reason: result.cleared
729+
? "the computer and its saved state were deleted"
730+
: "no saved state was present to delete",
731+
});
716732
// The refs the last snapshot handed out describe a page that no longer exists, and a fresh
717733
// computer counts generations from one again, so the row has to go with the profile.
718734
await snapshots.clear(botId);
@@ -725,13 +741,6 @@ export function createComputerGateway(
725741
* anything a person would recognise as private.
726742
*/
727743
await pageFrames?.clear(botId);
728-
await writeControlEvent(auditStore, "computer.reset", {
729-
botId,
730-
actor,
731-
reason: result.cleared
732-
? "the computer and its saved state were deleted"
733-
: "no saved state was present to delete",
734-
});
735744
return result;
736745
},
737746

0 commit comments

Comments
 (0)