mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): keep active tool order when the same tools are set again (#11514)
The system prompt lists tool snippets and per-tool guidelines in the order of the active tools. When an extension called `pi.setActiveTools()` with the same tools in a different order (for example `[...pi.getActiveTools()].reverse()`), the "tools" and "rules" sections came out different. pi then recorded a "System Prompt Update: tools, rules" message even though the active tools had not changed. On providers without mid-conversation system messages, this also rewrites the prompt head and loses the cache. `_applyToolLoadout` in `agent-session.ts` now treats the requested names as a set: - tools that stay active keep their current order; - newly activated tools are appended at the end. Reordering the same set therefore leaves the prompt unchanged. Adding a tool only appends its bullet. The same rule covers tool lists changed by `before_agent_start` handlers, since they go through the same function. Testing: - Added a test in `test/system-prompt-updates.test.ts`: it reverses the active tools between two prompts and expects a single system message. It fails without the change (a second message patches `tools` and `rules`) and passes with it. - `npm run check` passes. - `./test.sh`: all coding-agent suites pass. The only failure is an unrelated Anthropic model metadata test in `packages/ai`. Fixes #11495 Requested by @badlogic Co-authored-by: triagent <triagent@localhost>
This commit is contained in:
parent
f6ee9ce9c1
commit
b9e3eef6c2
3 changed files with 24 additions and 1 deletions
|
|
@ -13,6 +13,7 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- Fixed a redundant system prompt update (tools, rules) when an extension set the same active tools in a different order, for example a reversed `pi.setActiveTools()` list. Tools that stay active now keep their order and newly activated tools are appended ([#11495](https://github.com/earendil-works/pi/issues/11495))
|
||||
- Fixed `pi install` and `pi remove` adding ever deeper nested entries to the npm `package-lock.json` when the agent directory is reached through a symlink, such as `~/.pi` linked into a dotfiles repo. pi now passes the resolved real path to npm ([#11358](https://github.com/earendil-works/pi/issues/11358))
|
||||
- MCP sign-in now rejects authorization endpoints and other OAuth metadata URLs that are not `http:` or `https:`. Before, a malicious MCP server could supply a `file:` or custom protocol URL, which pi passed to the platform URL handler.
|
||||
- Automatic `fd` and `rg` downloads no longer fetch x86-64 binaries on ARM32 and other architectures without a prebuilt release. pi now warns and recommends installing the tool with the system package manager ([#11209](https://github.com/earendil-works/pi/issues/11209))
|
||||
|
|
|
|||
|
|
@ -1591,9 +1591,14 @@ export class AgentSession {
|
|||
* the registered, non-hidden ones; they are declared to the model. Active tools with a
|
||||
* `prepareLoadout` hook can change the declared descriptions and hide declarations from
|
||||
* requests (see {@link _installHiddenDeclarationsProjection}).
|
||||
*
|
||||
* The active tools are a set: tools that stay active keep their current order and newly active
|
||||
* ones are appended, so reordering the same names does not change the prompt (#11495).
|
||||
*/
|
||||
private _applyToolLoadout(toolNames: string[]): AgentTool[] {
|
||||
const tools = [...new Set(toolNames)].flatMap((name) => {
|
||||
const requested = new Set(toolNames);
|
||||
const kept = this.getActiveToolNames().filter((name) => requested.has(name));
|
||||
const tools = [...new Set([...kept, ...requested])].flatMap((name) => {
|
||||
const tool = this._toolRegistry.get(name);
|
||||
return tool && this._getToolExposure(name) !== "hidden" && this._isActivatable(name) ? [tool] : [];
|
||||
});
|
||||
|
|
|
|||
|
|
@ -53,6 +53,23 @@ describe("system prompt updates", () => {
|
|||
}
|
||||
});
|
||||
|
||||
// #11495
|
||||
test("does not patch the prompt when the active tools are only reordered", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
harness.setResponses([fauxAssistantMessage("first"), fauxAssistantMessage("second")]);
|
||||
await harness.session.prompt("one");
|
||||
const before = harness.session.getActiveToolNames();
|
||||
harness.session.setActiveToolsByName([...before].reverse());
|
||||
expect(harness.session.getActiveToolNames()).toEqual(before);
|
||||
await harness.session.prompt("two");
|
||||
const systemMessages = harness.session.messages.filter((message) => message.role === "system");
|
||||
expect(systemMessages).toHaveLength(1);
|
||||
} finally {
|
||||
harness.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test("opens a transcript without a system message and declares the prompt on the first request", async () => {
|
||||
const tempDir = mkdtempSync(join(tmpdir(), "pi-system-prompt-migration-"));
|
||||
try {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue