Skip to content

Commit 2af6c8f

Browse files
committed
Merge remote-tracking branch 'origin/main' into gate577
# Conflicts: # CHANGELOG.md
2 parents b37710d + 2eddad8 commit 2af6c8f

19 files changed

Lines changed: 4376 additions & 54 deletions

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ jobs:
351351
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
352352
with:
353353
persist-credentials: false
354-
- uses: docker/setup-buildx-action@594f3bf4285d9ea8dc53c9a0c9c4092420091003 # v4.4.0
354+
- uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1
355355
# Loaded rather than pushed: this runs on pull requests, including from forks, and it proves
356356
# the image builds without granting anything the ability to publish one.
357357
- uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0
@@ -462,7 +462,7 @@ jobs:
462462
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
463463
with:
464464
persist-credentials: false
465-
- uses: docker/setup-buildx-action@594f3bf4285d9ea8dc53c9a0c9c4092420091003 # v4.4.0
465+
- uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1
466466
# A Dockerfile the release does not publish is one a machine would build from source, so the
467467
# list and the tree have to agree. Checked here rather than trusted, because the failure is
468468
# silent: the release succeeds and a laptop somewhere compiles the app.

.github/workflows/publish-release.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ jobs:
132132
with:
133133
ref: ${{ github.sha }}
134134
persist-credentials: false
135-
- uses: docker/setup-buildx-action@594f3bf4285d9ea8dc53c9a0c9c4092420091003 # v4.4.0
135+
- uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1
136136
- uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0
137137
with:
138138
registry: ghcr.io
@@ -197,7 +197,7 @@ jobs:
197197
with:
198198
ref: ${{ github.sha }}
199199
persist-credentials: false
200-
- uses: docker/setup-buildx-action@594f3bf4285d9ea8dc53c9a0c9c4092420091003 # v4.4.0
200+
- uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1
201201
- uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0
202202
with:
203203
registry: ghcr.io
@@ -270,7 +270,7 @@ jobs:
270270
matrix:
271271
image: ${{ fromJSON(needs.verify.outputs.components) }}
272272
steps:
273-
- uses: docker/setup-buildx-action@594f3bf4285d9ea8dc53c9a0c9c4092420091003 # v4.4.0
273+
- uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1
274274
- uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0
275275
with:
276276
registry: ghcr.io

CHANGELOG.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,35 @@ or lead with a sentence. Every such answer read as no answer, so the Bot was off
1717
holds and the audit row said `unavailable`. The object is now read out of the answer, the way the
1818
router already reads its own. A bare JSON answer is read as before.
1919

20+
### A long message reaches the coworker it is for, and is recorded
21+
22+
A message over 10,000 characters, such as a pasted email thread or log, was refused by the router
23+
since it started capping the text it reads. The home composer carries on past a routing that fails,
24+
so the message went to the default coworker rather than the one it is for, and a coworker chosen with
25+
`@` or from the To: field started with no `channel.routed` row. The app now asks the router about the
26+
message's opening, and the whole message still goes to the coworker. Shorter messages route as
27+
before.
28+
29+
### Removing somebody is recorded even when retiring what they owned fails
30+
31+
Removing somebody denies their access and ends their sessions, then retires the credentials and
32+
brokered connections they had granted this deployment. When that second half failed — a vault or
33+
Composio not answering — the removal was already committed but nothing was written to the audit trail,
34+
and removing them a second time reported success without retrying it, leaving those connections
35+
standing. The removal is now recorded as soon as it takes effect, and removing somebody already
36+
removed finishes the retirement that failed.
37+
38+
### Removing a connector takes its grants with it
39+
40+
A grant naming a connector's tool outlived the connector. Removing an app revoked every credential
41+
and every brokered account and deleted the app itself, and left the grant rows behind, naming a
42+
server that no longer existed. Nothing showed them: the page that reports grants a connector no
43+
longer advertises reads them off the connector's own row, and there was none. Adding the same app
44+
back — which mints the same id, and so the same tool names — put every action it had back on every
45+
Bot that used to hold it, with nobody granting anything and no row in the trail saying a grant had
46+
been made. An app's grants are now removed in the same step as the app, the removal records which
47+
grants it released and from which Bots, and a migration drops the grants earlier removals left
48+
behind. Grants for other connectors, and skill grants, are untouched.
2049
### A vendor that broke no longer reads as a refusal to a Bot running its own loop
2150

2251
When a Bot that calls tools back from its own process, such as the LangGraph Bots, called a tool

app/src/lib/channels/route.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,37 @@ export type RoutingDecision = {
2121
viaMention: boolean;
2222
};
2323

24+
/**
25+
* The most of a message `POST /api/route` reads, and a message's opening cut to it.
26+
*
27+
* The route refuses anything longer with a 400, so the prompt it builds stays bounded. The composer
28+
* has no such limit, and both callers here carry on past a routing that failed: the home composer
29+
* sends the message to the default coworker instead, and a chosen coworker's conversation starts
30+
* without its `channel.routed` row. So a pasted email thread or log went to the wrong coworker, or
31+
* unrecorded, and nothing on screen said so. Who a message is for is plain from its opening, so the
32+
* opening is what is asked about; the whole message still goes to the coworker.
33+
*
34+
* Trimmed first, as the route trims, and cut one unit short when the cut would split a character, so
35+
* the router is not handed half of an emoji.
36+
*/
37+
const ROUTING_TEXT_LIMIT = 10_000;
38+
39+
function routingText(text: string): string {
40+
const trimmed = text.trim();
41+
if (trimmed.length <= ROUTING_TEXT_LIMIT) return trimmed;
42+
const opening = trimmed.slice(0, ROUTING_TEXT_LIMIT);
43+
const last = opening.charCodeAt(opening.length - 1);
44+
return last >= 0xd800 && last <= 0xdbff ? opening.slice(0, -1) : opening;
45+
}
46+
2447
export async function routeMessage(
2548
text: string,
2649
agentId?: string,
2750
): Promise<RoutingDecision> {
51+
const asked = routingText(text);
2852
const response = await client("/api/route", {
2953
method: "POST",
30-
body: agentId ? { text, agentId } : { text },
54+
body: agentId ? { text: asked, agentId } : { text: asked },
3155
fallback: "Could not choose a coworker.",
3256
});
3357
return (await response.json()) as RoutingDecision;
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { afterEach, expect, test } from "bun:test";
2+
import type { AgentProfileStore } from "../../server/src/agents/profile-store";
3+
import type { AuditStore } from "../../server/src/audit";
4+
import type { IntentRouter } from "../../server/src/routing/classify";
5+
import { createRoutingRoutes } from "../../server/src/routing/routes";
6+
import { routeMessage } from "../src/lib/channels/route";
7+
8+
/**
9+
* A long message still finds its coworker, and the trail still says how.
10+
*
11+
* `POST /api/route` refuses a message over 10,000 characters, so the model prompt it builds stays
12+
* bounded. The composer has no such limit: a pasted email thread or log is one message. Both callers
13+
* of `routeMessage` carry on past a failed routing on purpose, so the refusal said nothing on
14+
* screen. The home composer sent the message to the default coworker instead of the one it is for,
15+
* and a coworker the person chose was started without its `channel.routed` row.
16+
*
17+
* The route itself answers here, not a stub of it, so the cap these hold against is the server's own.
18+
*/
19+
20+
const originalFetch = globalThis.fetch;
21+
afterEach(() => {
22+
globalThis.fetch = originalFetch;
23+
});
24+
25+
const ROSTER = [
26+
{
27+
id: "general-assistant",
28+
name: "General Assistant",
29+
roleDescription: "everyday work",
30+
visibility: "public",
31+
},
32+
{
33+
id: "risk-analyst",
34+
name: "Risk Analyst",
35+
roleDescription: "regulatory and compliance questions",
36+
visibility: "public",
37+
},
38+
];
39+
40+
function serve() {
41+
/** What the router was asked to read, so the text it saw is an assertion. */
42+
const asked: string[] = [];
43+
const written: { eventType: string; payload: Record<string, unknown> }[] = [];
44+
45+
const asActor: Parameters<typeof createRoutingRoutes>[2] = async (
46+
context,
47+
next,
48+
) => {
49+
context.set("actor", {
50+
id: "u1",
51+
email: "person@openbot.test",
52+
role: "user",
53+
});
54+
await next();
55+
};
56+
const store = { list: async () => ROSTER } as unknown as AgentProfileStore;
57+
const router = {
58+
route: async (text: string) => {
59+
asked.push(text);
60+
return {
61+
agentId: "risk-analyst",
62+
name: "Risk Analyst",
63+
reason: "matches what it is for",
64+
fallback: false,
65+
undecided: null,
66+
};
67+
},
68+
} as unknown as IntentRouter;
69+
const auditStore = {
70+
insert: async (event: {
71+
eventType: string;
72+
payload: Record<string, unknown>;
73+
}) => {
74+
written.push(event);
75+
},
76+
} as unknown as AuditStore;
77+
78+
const routes = createRoutingRoutes(store, router, asActor, auditStore);
79+
globalThis.fetch = Object.assign(
80+
async (
81+
path: Parameters<typeof fetch>[0],
82+
init?: Parameters<typeof fetch>[1],
83+
) => {
84+
if (path !== "/api/route") throw new Error(`unexpected ${String(path)}`);
85+
return routes.request("http://openbot.test/", init);
86+
},
87+
{ preconnect: originalFetch.preconnect },
88+
);
89+
return { asked, written };
90+
}
91+
92+
test("a message longer than the route reads is routed to the coworker it is for, and recorded", async () => {
93+
const { asked, written } = serve();
94+
95+
const decision = await routeMessage(
96+
`Which of these clauses breach the policy?\n${"clause ".repeat(4_000)}`,
97+
);
98+
99+
expect(decision.agentId).toBe("risk-analyst");
100+
expect(asked).toHaveLength(1);
101+
expect(asked[0]?.startsWith("Which of these clauses")).toBe(true);
102+
expect(written.map((row) => row.eventType)).toEqual(["channel.routed"]);
103+
});
104+
105+
test("a long message to a coworker the person chose is recorded as their choice", async () => {
106+
const { asked, written } = serve();
107+
108+
const decision = await routeMessage("x".repeat(25_000), "risk-analyst");
109+
110+
expect(decision).toMatchObject({
111+
agentId: "risk-analyst",
112+
viaMention: true,
113+
});
114+
expect(asked).toEqual([]);
115+
expect(written).toHaveLength(1);
116+
expect(written[0]?.payload).toMatchObject({
117+
chosen: "risk-analyst",
118+
viaMention: true,
119+
});
120+
});
121+
122+
test("the opening is cut between characters, not through an emoji", async () => {
123+
const { asked } = serve();
124+
125+
// The emoji's two halves straddle the 10,000th unit.
126+
await routeMessage(`${"a".repeat(9_999)}😀${"b".repeat(50)}`);
127+
128+
expect(asked).toEqual(["a".repeat(9_999)]);
129+
});
130+
131+
test("a message that fits is sent as it was written", async () => {
132+
const { asked } = serve();
133+
134+
await routeMessage("Is this contract compliant? 😀");
135+
136+
expect(asked).toEqual(["Is this contract compliant? 😀"]);
137+
});

desktop/src-tauri/src/engine.rs

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,11 +380,13 @@ fn where_installers_put(engine: Engine) -> Option<PathBuf> {
380380
let places: Vec<PathBuf> = match engine {
381381
#[cfg(target_os = "windows")]
382382
Engine::Podman => {
383-
// The per-user MSI first: it is the one OpenBot runs. A machine-wide install left by
384-
// somebody else is still found by the second.
383+
// The MSI uses Programs\Podman per user and Program Files\Podman per machine.
384+
// Keep the legacy RedHat location for installations from the older EXE installer.
385385
[
386386
std::env::var_os("LOCALAPPDATA")
387387
.map(|local| PathBuf::from(local).join("Programs\\Podman\\podman.exe")),
388+
std::env::var_os("ProgramFiles")
389+
.map(|files| PathBuf::from(files).join("Podman\\podman.exe")),
388390
std::env::var_os("ProgramFiles")
389391
.map(|files| PathBuf::from(files).join("RedHat\\Podman\\podman.exe")),
390392
]
@@ -613,6 +615,48 @@ mod tests {
613615
assert_eq!(on_path("openbot-not-a-real-binary"), None);
614616
}
615617

618+
#[test]
619+
#[cfg(windows)]
620+
fn windows_installed_podman_runs_without_an_inherited_path_entry() {
621+
if crate::test_support::isolated_process(
622+
"engine::tests::windows_installed_podman_runs_without_an_inherited_path_entry",
623+
) {
624+
return;
625+
}
626+
let root = crate::test_support::temp_root("podman install locations");
627+
std::fs::create_dir_all(&root).unwrap();
628+
let source = root.join("engine.rs");
629+
let binary = root.join("fixture.exe");
630+
std::fs::write(&source, "fn main() { println!(\"1.44\"); }").unwrap();
631+
crate::test_support::compile_fixture(&source, &binary);
632+
let local = root.join("Local");
633+
let program_files = root.join("Program Files");
634+
std::env::set_var("LOCALAPPDATA", &local);
635+
std::env::set_var("ProgramFiles", &program_files);
636+
std::env::set_var("PATH", root.join("empty-path"));
637+
638+
for installation in [
639+
local.join("Programs/Podman/podman.exe"),
640+
program_files.join("Podman/podman.exe"),
641+
program_files.join("RedHat/Podman/podman.exe"),
642+
] {
643+
std::fs::create_dir_all(installation.parent().unwrap()).unwrap();
644+
std::fs::copy(&binary, &installation).unwrap();
645+
assert_eq!(program(Engine::Podman).as_ref(), Some(&installation));
646+
let address = Address::new(Engine::Podman, Some("openbot".into()));
647+
assert_eq!(address.parts().0, installation);
648+
assert!(address.responds(), "resolved engine should actually run");
649+
assert!(tool(Engine::Podman)
650+
.arg("--version")
651+
.output()
652+
.unwrap()
653+
.status
654+
.success());
655+
std::fs::remove_file(installation).unwrap();
656+
}
657+
std::fs::remove_dir_all(root).unwrap();
658+
}
659+
616660
/// The provider directory has to be in *front* of PATH: a broken `docker-compose` earlier on
617661
/// somebody's PATH would otherwise be the one Podman runs.
618662
#[test]

0 commit comments

Comments
 (0)