Skip to content

Commit 233b388

Browse files
committed
fix(appkit): apply review feedback to agents plugin
PR #304 agentic review applied (P1 + cheap P2): - Approval gate now honours the modern `effect` field (write/update/ destructive), matching the documented contract on ToolAnnotations. Previously a tool authored with `effect: "destructive"` and no legacy `destructive: true` boolean bypassed the gate. - Sub-agent tool calls share the parent's RunState, so the per-run tool-call budget and the destructive-tool approval gate apply to nested sub-agent calls — not only the top-level adapter. - /invocations enforces `maxConcurrentStreamsPerUser`. Without this a client could bypass the cap by switching from /chat to /invocations. - /cancel uses a Zod schema instead of a raw `as` cast, matching the validation pattern of the sibling routes. - agents.reload() builds the registry into a fresh Map and only swaps on success. A malformed markdown file no longer wipes the live registry and breaks in-flight requests. - consumeAdapterStream and normalizeToolResult are wired into the four call sites that previously inlined the same accumulation / serialization logic (top-level _streamAgent, runSubAgent, dispatchToolCall, runAgent). - load-agents.ts uses fs.promises so reload() does not block the event loop. - Pin js-yaml to 4.1.1 (drop the caret), matching the rest of appkit's pinned deps. Tests cover the approval-gate `effect` matrix (destructive/write/update trigger; read/undefined skip), the deny path, the shared budget across top-level + sub-agent dispatches, and the /invocations rate-limit gate. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
1 parent a20ab5e commit 233b388

8 files changed

Lines changed: 647 additions & 217 deletions

File tree

packages/appkit/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676
"dotenv": "16.6.1",
7777
"express": "4.22.0",
7878
"get-port": "7.2.0",
79-
"js-yaml": "^4.1.1",
79+
"js-yaml": "4.1.1",
8080
"obug": "2.1.1",
8181
"pg": "8.18.0",
8282
"picocolors": "1.1.1",

packages/appkit/src/core/agent/load-agents.ts

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import fs from "node:fs";
1+
import type { Dirent } from "node:fs";
2+
import fs from "node:fs/promises";
23
import path from "node:path";
34
import yaml from "js-yaml";
45
import type { AgentAdapter } from "shared";
@@ -103,7 +104,7 @@ export async function loadAgentFromFile(
103104
filePath: string,
104105
ctx: LoadContext,
105106
): Promise<AgentDefinition> {
106-
const raw = fs.readFileSync(filePath, "utf-8");
107+
const raw = await fs.readFile(filePath, "utf-8");
107108
const name = agentIdFromMarkdownPath(filePath);
108109
const { data } = parseFrontmatter(raw, filePath);
109110
if (Array.isArray(data?.agents) && data.agents.length > 0) {
@@ -138,11 +139,15 @@ export async function loadAgentsFromDir(
138139
dir: string,
139140
ctx: LoadContext,
140141
): Promise<LoadResult> {
141-
if (!fs.existsSync(dir)) {
142-
return { defs: {}, defaultAgent: null };
142+
let entries: Dirent[];
143+
try {
144+
entries = await fs.readdir(dir, { withFileTypes: true });
145+
} catch (err) {
146+
if ((err as NodeJS.ErrnoException).code === "ENOENT") {
147+
return { defs: {}, defaultAgent: null };
148+
}
149+
throw err;
143150
}
144-
145-
const entries = fs.readdirSync(dir, { withFileTypes: true });
146151
const orphanMd = entries
147152
.filter((e) => e.isFile() && e.name.endsWith(".md"))
148153
.map((e) => e.name)
@@ -174,12 +179,17 @@ export async function loadAgentsFromDir(
174179
// Pass 1: build every agent's definition; collect sub-agent refs.
175180
for (const id of agentIds) {
176181
const agentPath = path.join(dir, id, "agent.md");
177-
if (!fs.existsSync(agentPath)) {
178-
throw new Error(
179-
`Agents subdirectory '${path.join(dir, id)}' must contain agent.md.`,
180-
);
182+
let raw: string;
183+
try {
184+
raw = await fs.readFile(agentPath, "utf-8");
185+
} catch (err) {
186+
if ((err as NodeJS.ErrnoException).code === "ENOENT") {
187+
throw new Error(
188+
`Agents subdirectory '${path.join(dir, id)}' must contain agent.md.`,
189+
);
190+
}
191+
throw err;
181192
}
182-
const raw = fs.readFileSync(agentPath, "utf-8");
183193
defs[id] = buildDefinition(id, raw, agentPath, ctx);
184194
const { data } = parseFrontmatter(raw, agentPath);
185195
if (data?.agents !== undefined) {

packages/appkit/src/core/agent/run-agent.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type {
55
AgentToolDefinition,
66
Message,
77
} from "shared";
8+
import { consumeAdapterStream } from "./consume-adapter-stream";
89
import {
910
type FunctionTool,
1011
functionToolToDefinition,
@@ -78,7 +79,6 @@ export async function runAgent(
7879
};
7980

8081
const events: AgentEvent[] = [];
81-
let text = "";
8282

8383
const stream = adapter.run(
8484
{
@@ -90,15 +90,15 @@ export async function runAgent(
9090
{ executeTool, signal },
9191
);
9292

93-
for await (const event of stream) {
94-
if (signal?.aborted) break;
95-
events.push(event);
96-
if (event.type === "message_delta") {
97-
text += event.content;
98-
} else if (event.type === "message") {
99-
text = event.content;
100-
}
101-
}
93+
// Shared accumulation rule (deltas append, `message` replaces). The
94+
// `events` array is filled via the `onEvent` side effect so callers that
95+
// inspect the raw stream still get the full record.
96+
const text = await consumeAdapterStream(stream, {
97+
signal,
98+
onEvent: (event) => {
99+
events.push(event);
100+
},
101+
});
102102

103103
return { text, events };
104104
}

0 commit comments

Comments
 (0)