Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions agent-image/entrypoint.nix
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ let
dontFixup = true;
outputHashMode = "recursive";
outputHashAlgo = "sha256";
outputHash = "sha256-GrsX8VmqoHIgvq/OZaOZN8HwrjI9PWcU6nYwI7I0uMo=";
outputHash = "sha256-JbgM44AwH7/b3Y/2T44+eBXwyvMi8owXToGVspEeCk4=";
};

# The package's own source. A BARE path here (`${../packages/…}`) would copy
Expand Down Expand Up @@ -188,10 +188,10 @@ let

mkdir -p $out
# `omp-legacy-pi-modules` is an OPTIONAL dynamic import inside the SDK's
# legacy-compat shim (pi-coding-agent legacy-pi-compat.ts:50), guarded at
# runtime and absent from our dependency closure. Left external so the
# bundler does not fail resolving a module the code already tolerates
# missing.
# legacy-compat shim (pi-coding-agent
# src/extensibility/plugins/legacy-pi-compat.ts:751), guarded at runtime and
# absent from our dependency closure. Left external so the bundler does not
# fail resolving a module the code already tolerates missing.
#
# `--compile` emits a STANDALONE executable (bun runtime + the whole
# resolved graph baked in), not an interpreted `cli.js`. This is what lets
Expand Down
350 changes: 81 additions & 269 deletions bun.lock

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions packages/compass-agent/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
"@connectrpc/connect-node": "^2.1.0",
"@effect/opentelemetry": "catalog:",
"@effect/platform": "catalog:",
"@oh-my-pi/pi-agent-core": "^16.4.8",
"@oh-my-pi/pi-ai": "^16.4.8",
"@oh-my-pi/pi-coding-agent": "^16.4.8",
"@oh-my-pi/omptype": "^18.0.11",
"@oh-my-pi/pi-agent-core": "^18.0.11",
"@oh-my-pi/pi-ai": "^18.0.11",
"@oh-my-pi/pi-coding-agent": "^18.0.11",
"@opentelemetry/api": "catalog:",
"@opentelemetry/exporter-metrics-otlp-proto": "catalog:",
"@opentelemetry/exporter-trace-otlp-proto": "catalog:",
Expand All @@ -29,7 +30,6 @@
"@opentelemetry/sdk-trace-node": "catalog:",
"@opentelemetry/sdk-trace-web": "catalog:",
"@opentelemetry/semantic-conventions": "catalog:",
"arktype": "2.2.3",
"effect": "catalog:"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/compass-agent/src/board.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
// result out, and the captured request asserted verbatim. Mirrors forge.test.ts.

import { describe, expect, test } from "bun:test";
import { ArkErrors, type Type } from "@oh-my-pi/omptype/ark";
import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core";
import { ArkErrors, type Type } from "arktype";
import {
BoardBroker,
type BoardTransport,
Expand Down
25 changes: 17 additions & 8 deletions packages/compass-agent/src/board.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@
// the RelayBoardCall payload on the Runner->Server leg (DL-049), one wire shape
// for both hops. See packages/compass-agent/AGENTS.md for the package contract.

// The schema builder rides the SDK's own schema stack via its `/ark` compat
// facade — see the comms.ts note; one schema implementation in the graph, so
// there is no two-copy mismatch to catch.
import { type } from "@oh-my-pi/omptype/ark";
import type { AgentTool } from "@oh-my-pi/pi-agent-core";
// `arktype` is pinned exact in package.json to whatever the SDK resolves — see
// the comms.ts note on this pin; a mismatch resolves two @ark/schema copies and
// `tsc` catches it.
import { type } from "arktype";
import {
type BoardCallRequest,
BoardCallRequestSchema,
Expand Down Expand Up @@ -73,13 +73,22 @@ export class BoardBroker {
}

// The required-non-blank string idiom (comms/lifecycle/forge precedent): the
// `.narrow` predicate is enforced at runtime but has no JSON Schema form
// (`toJsonSchema` drops it), so the model sees a bare string and learns the rule
// only from the description — hence the description repeats it.
// `.narrow` predicate is enforced at runtime but has no JSON Schema form (the
// harness degrades the node to its unconstrained base), so the model sees a
// bare string and learns the rule only from the description — hence the
// description repeats it. Appended here rather than hand-written into each
// caller's text so no call site can forget it: under omptype a `.describe()`
// SHADOWS the narrow's `ctx.mustBe(...)` reason in the rejection message, so if
// the rule is missing from the description it reaches the model through no
// channel at all.
const nonBlank = (description: string) =>
type("string")
.narrow((s, ctx) => s.trim().length > 0 || ctx.mustBe("non-blank"))
.describe(description);
.describe(
description.includes("must not be blank")
? description
: `${description} (must not be blank)`,
);

// The eight real board states, as the model-facing tokens the tool accepts.
// ISSUE_STATE_UNSPECIFIED is deliberately absent: the closed enum rejects it at
Expand Down
113 changes: 98 additions & 15 deletions packages/compass-agent/src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1624,16 +1624,26 @@ describe("main", () => {
});

// A compaction round-trip: a fixture body whose file contains a superseded
// compaction loads through the SDK's own elision — proving the T5
// reconstruction needs no compaction awareness beyond T4's supersession.
test("a superseded-compaction fixture loads via the SDK's own elision", async () => {
// compaction loads intact — proving the T5 reconstruction needs no compaction
// awareness beyond T4's supersession.
//
// The SDK moved WHERE supersession is applied. It used to elide the superseded
// summary at session LOAD (`elideSupersededCompactionEntries`, gone in 18.x);
// it now keeps stored entries verbatim and elides only when assembling the
// model-facing context (`buildSessionContext`, session-context.ts:174 ->
// `active ? entry.summary : SUPERSEDED_COMPACTION_SUMMARY` at :377). So the
// loaded entries carry BOTH real summaries, and this asserts the property that
// actually matters to compass either way: the round-trip preserves the whole
// compaction chain, so reconstruction never has to reason about supersession.
test("a superseded-compaction fixture round-trips with its chain intact", async () => {
const session = fakeSession();
const cwd = process.cwd();
const sessionDir = SessionManager.getDefaultSessionDir(cwd);
mkdirSync(sessionDir, { recursive: true });
const resumeFile = join(sessionDir, "20260101-000000_compacted.jsonl");
// Two compactions on the active branch; the earlier one is superseded and
// the SDK's elideSupersededCompactionEntries collapses its summary on load.
// Two compactions on the active branch; the earlier one is superseded. Both
// summaries are stored verbatim — supersession is applied downstream at
// context assembly, not at load.
writeFileSync(
resumeFile,
sessionFixture([
Expand All @@ -1659,13 +1669,12 @@ describe("main", () => {
fakeCarrier(emptyLog(), { control: emptyControlStream }),
},
);
// The session loaded (post-compaction entry present) and the superseded
// compaction's summary was elided by the SDK loader.
// The session loaded with its full compaction chain intact — both summaries
// present, in order. (Supersession is applied downstream at context
// assembly, not here; compass never reads the elided form.)
const summaries = compactionSummariesOf(entriesAtCreate);
expect(summaries).toHaveLength(2);
expect(summaries[0]).toBe(
"[Superseded compaction summary elided during session load]",
);
expect(summaries[0]).toBe("first compaction summary");
expect(summaries[1]).toBe("second compaction summary");
expect(textsOf(entriesAtCreate)).toContain("after compaction");
});
Expand Down Expand Up @@ -2393,7 +2402,16 @@ interface SeenConfig {
skills?: unknown[];
additionalExtensionPaths?: string[];
disableExtensionDiscovery?: boolean;
customTools?: unknown[];
// Typed to the fields these tests actually read (name/loadMode plus the
// callable surface a stamp must preserve), so the compiler checks the access
// instead of an inline assertion fabricating the shape.
customTools?: {
name?: unknown;
loadMode?: unknown;
execute?: unknown;
renderCall?: unknown;
renderResult?: unknown;
}[];
enableMCP?: boolean;
autoApprove?: boolean;
customSystemPrompt?: string;
Expand Down Expand Up @@ -2439,7 +2457,33 @@ describe("main wires the mounted agent-config into createAgentSession", () => {
);

const session = fakeSession();
const mcpTools = [{ name: "db.query" }];
// A CLASS instance with `#private` state, not an object literal. The real
// `mcp.tools` are SDK class instances: `MCPTool` (pi-coding-agent
// src/mcp/tool-bridge.ts:492) keeps `execute`/`renderCall`/`renderResult`
// on the PROTOTYPE, and its sibling `DeferredMCPTool` (:604) also holds
// ECMAScript `#private` fields its `execute` reads. Both properties matter
// to the fixture: a plain literal spreads losslessly (so it cannot see a
// `{ ...tool }` stamp shear the methods off), and a fixture without
// `#private` state cannot see an `Object.create` clone re-home `this` —
// that break passes a `typeof execute === "function"` check and only
// surfaces when the method is actually CALLED, which is why the assertion
// below invokes it.
class FakeMcpTool {
readonly name = "db.query";
readonly mcpServerName = "db";
readonly mcpToolName = "query";
readonly #server = "db";
async execute() {
return { content: [], server: this.#server };
}
renderCall() {
return "db.query";
}
renderResult() {
return "db.query result";
}
}
const mcpTools = [new FakeMcpTool()];
let connectedWith: Record<string, unknown> | undefined;
const seen: SeenConfig[] = [];
await main(
Expand Down Expand Up @@ -2488,7 +2532,41 @@ describe("main wires the mounted agent-config into createAgentSession", () => {
// comms/lifecycle tools (merged in main), so this is a containment check,
// not identity — the dedicated native-tools test below pins those.
expect(connectedWith).toEqual({ db: { command: "db-mcp" } });
expect(opts.customTools).toEqual(expect.arrayContaining(mcpTools));
// Asserted by PRESENTATION, not bare membership. main stamps the whole
// merged array `loadMode: "essential"`, and that stamp is the load-bearing
// part for a mounted-MCP tool: without it the SDK's adapter boundary
// defaults it to `"discoverable"`, which registers the tool but keeps it
// out of the model's top-level callable schema — and the `xd://` transport
// does not recover it in this headless session shape. A membership-only
// check passes while the tool is silently unreachable, which is the exact
// failure the RIG-1741/CD-3 mount contract exists to prevent.
for (const tool of mcpTools) {
const stamped = opts.customTools?.find((t) => t.name === tool.name);
expect(stamped).toEqual(
expect.objectContaining({
name: tool.name,
loadMode: "essential",
}),
);
// The stamp must preserve the tool's callable surface AND its identity.
// All three of `execute`/`renderCall`/`renderResult` are prototype
// methods, so a `{ ...tool }` stamp drops them outright; an
// `Object.create` clone keeps them but re-homes `this`, so a
// `#private` read throws only when the method is INVOKED. Presence
// checks pass in that second case, so the contract is asserted by
// actually calling `execute` — a tool the model can see and cannot
// call is worse than one it never sees.
expect(typeof stamped?.renderCall).toBe("function");
expect(typeof stamped?.renderResult).toBe("function");
const execute = stamped?.execute;
if (typeof execute !== "function") {
throw new Error("the stamp dropped the tool's execute method");
}
await expect(execute.call(stamped)).resolves.toEqual({
content: [],
server: "db",
});
}
expect(opts.enableMCP).toBe(false);
});

Expand Down Expand Up @@ -2894,9 +2972,14 @@ describe("main wires the mounted agent-config into createAgentSession", () => {
// (2) rules survived (the custom template's <rules> list).
expect(rendered).toContain("MP1-RULE-SENTINEL");
expect(rendered).toContain("<rules>");
// (2) the project footer survived as its own block (environment + cwd).
// (2) the project footer survived as its own block. The SDK restructured
// this block: `project-prompt.md` renders `PROJECT` + `<workstation>` (the
// environment list), while the cwd line moved out to its own
// `date-cwd-reminder` block (session/date-cwd-reminder.ts) that this render
// path does not include. Assert the footer's own markers, not the relocated
// cwd text.
expect(rendered).toContain("PROJECT");
expect(rendered).toContain("current working directory");
expect(rendered).toContain("<workstation>");
// The read tool stayed in the set (the gate's precondition).
expect(systemPrompt.join("\n")).toContain("read");
});
Expand Down
57 changes: 56 additions & 1 deletion packages/compass-agent/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import type { Stats } from "node:fs";
import { lstat, mkdir, readlink, rm, symlink } from "node:fs/promises";
import { join } from "node:path";
import type { ToolLoadMode } from "@oh-my-pi/pi-agent-core";
import type { ApiKey, Model } from "@oh-my-pi/pi-ai";
import {
type AgentSession,
Expand Down Expand Up @@ -382,6 +383,25 @@ async function connectMountedMcp(
};
}

/**
* Mark every custom tool `essential` so it lands in the model's top-level
* callable schema, and return the same array.
*
* Assigns in place BY DESIGN — see the `customTools` seam in `main` for why
* copying is unsafe here (SDK tools are class instances with prototype methods
* and `#private` state, so a spread or clone yields a tool the model can see
* and cannot call). `loadMode` is a mutable field on `CustomTool`, so this
* preserves object identity.
*/
function stampEssential<T extends { loadMode?: ToolLoadMode }>(
tools: T[],
): T[] {
for (const tool of tools) {
tool.loadMode = "essential";
}
return tools;
}

/**
* The user-level agent dir the SDK's native discovery anchors on inside the
* container: `$HOME/.omp/agent` (`getAgentDir()` default, dirs.ts). The
Expand Down Expand Up @@ -906,7 +926,42 @@ export async function main(
// (RIG-1741 gap-1, constructed above): all reach the session as natives via
// the same customTools→state.tools→#withNatives path, so the container
// agent can spawn peers and post to channels.
customTools: [...mcp.tools, ...nativeTools],
//
// `loadMode: "essential"` is REQUIRED, not decorative, and it is stamped
// on the WHOLE merged array — natives AND mounted-MCP tools. SDK 18.x
// added progressive tool disclosure: at an adapter boundary an omitted
// `loadMode` defaults to `"discoverable"`
// (`defaultLoadModeForToolName`, pi-coding-agent
// src/tools/essential-tools.ts:43-45), which registers the tool but keeps
// it OUT of the model's top-level callable schema. `MCPManager.getTools()`
// never sets `loadMode`, so stamping only the natives would silently
// demote every mounted-MCP tool — and the `xd://` device transport does
// NOT recover them here, because it is gated on a top-level `write` tool
// this headless session does not request (`xdevEnabled`, pi-coding-agent
// src/tools/index.ts:772). A demoted MCP tool would therefore be neither
// top-level callable NOR xd://-reachable: registered and unreachable,
// which is exactly the silent-no-surface failure the RIG-1741/CD-3 mount
// contract exists to prevent. Stamped once here, at the single
// registration seam, rather than in the four native factories.
//
// Stamped IN PLACE, never by copying. `mcp.tools` are SDK class instances
// (`MCPTool`, pi-coding-agent src/mcp/tool-bridge.ts:492) whose
// `execute`/`renderCall`/`renderResult` live on the PROTOTYPE, and its
// sibling `DeferredMCPTool` (:604) additionally holds ECMAScript
// `#private` state. Neither a spread nor an `Object.create` clone can
// carry that: a spread drops the prototype methods outright, and a clone
// keeps the methods but re-homes `this`, so a `#private` read throws
// `Cannot access invalid private field` at the model's FIRST call — a
// tool the model can see and cannot use, which is worse than a demoted
// one. `loadMode` is a mutable field on `CustomTool`
// (extensibility/custom-tools/types.ts:224), so assigning it preserves
// object identity: the methods, the private state, and the connection
// rebinding `MCPTool.execute` performs on reconnect all keep working,
// and the session cannot drift onto a stale copy. Compass owns this
// manager exclusively (built above; `getTools()` feeds `customTools` and
// nothing else), so there is no second consumer to isolate from the
// assignment.
customTools: stampEssential([...mcp.tools, ...nativeTools]),
enableMCP: false,
// Headless approval policy (RIG-1741, design compass-agent-comms-tools
// §"the container runs headless with write-approval tools auto-executing"):
Expand Down
Loading
Loading