Merge branch 'main' into pr-8772
This commit is contained in:
can1357
@@ -123,6 +123,7 @@ Core methods:
|
||||
- `setModel`, `getThinkingLevel`, `setThinkingLevel`
|
||||
- `getServiceTiers`, `setServiceTier`
|
||||
- `registerProvider`
|
||||
- `registerFileWriteFallback`, `registerFileDeleteFallback`
|
||||
- `events` (shared event bus)
|
||||
|
||||
`getServiceTiers()` returns a detached snapshot of the session's live per-family tier map. `setServiceTier(family, tier)` changes one family for subsequent requests; pass `undefined` to clear that session override. OpenAI accepts `auto`, `default`, `flex`, `scale`, or `priority`; Anthropic accepts `priority`; Google accepts `flex` or `priority`. Changes made while a response is streaming do not alter that in-flight request.
|
||||
@@ -370,6 +371,137 @@ pi.registerTool({
|
||||
|
||||
`tool_call`/`tool_result` intercept all tools once the registry is wrapped in `sdk.ts`, including built-ins and extension/custom tools. `ToolDefinition` also supports optional `hidden`, `defaultInactive`, `loadMode` (`"discoverable"` by default, or `"essential"`), `deferrable`, `approval` (`"exec"` by default), `strict`, `mcpServerName`, `mcpToolName`, `renderCall`, and `renderResult` fields.
|
||||
|
||||
### File write fallback (`registerFileWriteFallback`)
|
||||
|
||||
`write`, `edit` and `apply_patch` perform the real byte-write to an ordinary file
|
||||
path through one shared primitive
|
||||
(`file ? file.write(content) : Bun.write(dst, content)`). When that primitive fails
|
||||
with a permission error (`EPERM`/`EACCES`/`EROFS` — every other error, such as
|
||||
`EISDIR`, is unaffected), the coding agent consults handlers registered
|
||||
via `pi.registerFileWriteFallback` before giving up:
|
||||
|
||||
```ts
|
||||
import type { FileWriteFallbackHandler } from "@oh-my-pi/pi-coding-agent";
|
||||
|
||||
const writeThroughBroker: FileWriteFallbackHandler = async (req, ctx) => {
|
||||
// req: { dst: string; content: string; cause: unknown }
|
||||
const ok = await myPrivilegedWriter.write(req.dst, req.content);
|
||||
return ok;
|
||||
};
|
||||
|
||||
pi.registerFileWriteFallback(writeThroughBroker);
|
||||
```
|
||||
|
||||
Handlers run in registration order; the first one to resolve `true` counts as the
|
||||
bytes being durably on disk, and the native tool continues exactly as if its own
|
||||
write had succeeded — including recording its file snapshot under the real
|
||||
destination path, so a later hashline `edit` on that path keeps working. A
|
||||
throwing handler is logged and skipped in favor of the next one — per handler, so a
|
||||
later handler registered by the same extension still runs; if every handler
|
||||
returns `false` (or none are registered), the original error is rethrown
|
||||
unchanged. Intended for a host that embeds the agent inside a sandbox denying
|
||||
direct filesystem writes but exposing a privileged write channel.
|
||||
|
||||
`req.dst` is the **symlink-resolved** destination, not the path the tool was given.
|
||||
The kernel follows every component above the last, so `ws/link/file` under a
|
||||
`ws/link -> /elsewhere` link lands outside `ws` while still looking in-workspace, and
|
||||
a prefix allowlist in your handler would pass on that innocent-looking path. For a
|
||||
write the final component is followed too, so it is resolved as well; for a delete it
|
||||
is not, because `unlink` removes a link rather than what it points at (so a delete
|
||||
`req.dst` may itself name a link). Treat `req.dst` as authoritative and do not
|
||||
re-derive the target from anything else. When the real destination cannot be
|
||||
established — a dangling final link, or an ancestor this process may not resolve — no
|
||||
handler is consulted at all and the original error is rethrown, because there is no
|
||||
destination to hand a privileged writer.
|
||||
|
||||
Two details matter when the destination is outside what the host allows:
|
||||
|
||||
- **A missing parent directory.** `Bun.write` creates missing parents itself, and
|
||||
when that `mkdir` is the operation being denied it reports the subsequent
|
||||
`open()`'s `ENOENT` rather than the denial. The agent redoes the `mkdir`
|
||||
explicitly to recover the real errno, so this still reaches a handler — with
|
||||
`req.cause` set to the `mkdir` denial. In that case `req.dst`'s parent does not
|
||||
exist yet and the handler is responsible for creating it. An `ENOENT` with a
|
||||
genuinely creatable or invalid parent is not diverted. (`apply_patch` creates the
|
||||
parent as a separate step before writing; that `mkdir` tolerates a denial when a
|
||||
fallback is registered, so the write still reaches the handler.)
|
||||
- **A hashline `MV`.** `edit`'s move writes its destination directly rather than
|
||||
through the LSP writethrough. It is routed to the same handlers, and the source
|
||||
unlink goes to the delete seam below, so a move out of a directory you cannot
|
||||
write completes too.
|
||||
|
||||
This is deliberately not an interception of every write the agent can make. A
|
||||
permission error from these surfaces as it does today, with no handler consulted:
|
||||
|
||||
- `write` to an archive member (`foo.zip:entry`) or to a SQLite row. Neither is a
|
||||
byte-write to `dst`: an archive rewrite reads the whole archive, replaces one
|
||||
entry, writes a temp file and renames over the original, so what lands is a whole
|
||||
binary container rather than the string the tool was handed; a SQLite write is a
|
||||
row operation inside the database engine with no byte payload at all. Brokering
|
||||
either needs a different request shape than "these bytes belong at this path".
|
||||
- The ACP bridge's `writeTextFile`, which hands the write to a remote client.
|
||||
- The `lsp` tool's own writes: applying a workspace edit or code action, and the
|
||||
Biome formatter, which writes the buffer and then shells out to `biome format
|
||||
--write` — a subprocess write no in-process seam can reach.
|
||||
|
||||
### File delete fallback (`registerFileDeleteFallback`)
|
||||
|
||||
Removing a file is a different primitive from writing one, and it has its own seam:
|
||||
|
||||
```ts
|
||||
pi.registerFileDeleteFallback(async (req, ctx) => {
|
||||
// req: { dst; cause; confirmedFile; sessionId } — no `content`.
|
||||
return await myPrivilegedWriter.unlink(req.dst);
|
||||
});
|
||||
```
|
||||
|
||||
It covers `edit`'s `REM`, the source side of a hashline `MV`, and `apply_patch`'s
|
||||
delete op, and follows the same rules as the write seam: same permission codes, first
|
||||
`true` wins, a throwing handler is skipped, the original error is rethrown if none
|
||||
succeed, and nothing happens at all when no handler is registered. Two differences:
|
||||
|
||||
- **`ENOENT` is never diverted.** Nothing is created on the way to an unlink, so a
|
||||
missing file genuinely is missing — `REM` turns it into a not-found error.
|
||||
- **A handler must unlink, never remove recursively.** `unlink` on a directory reports
|
||||
`EPERM` on macOS, which is indistinguishable from a sandbox denial by error code
|
||||
alone, so the seam `lstat`s the target and refuses to divert a directory. But when
|
||||
the target's own metadata sits behind the same boundary that denied the unlink —
|
||||
the common sandbox case — that check cannot be resolved, and `req.dst` may then be a
|
||||
directory. `req.confirmedFile` is `true` only when the seam positively established
|
||||
the target is a plain regular file; a symlink reports `false` too, since unlinking a
|
||||
link is fine but resolving it acts on something else entirely. A privileged helper
|
||||
that recursively removes `req.dst`, or realpaths it first, would act far outside
|
||||
what a tool that only ever removes one file asked for.
|
||||
|
||||
**Registering for deletes is deliberately separate from registering for writes.** A
|
||||
write handler brokers `req.content` to `req.dst`; if a delete request reached it, the
|
||||
missing content invites brokering an empty write and *truncating* the file that was
|
||||
meant to be removed. A write-only handler therefore never sees a delete.
|
||||
|
||||
Two lifecycle constraints, which apply to both seams:
|
||||
|
||||
- **Register during extension load** (from the default factory), like other
|
||||
`register*` calls. Handlers are installed when `ExtensionRunner.initialize` runs;
|
||||
an extension that registered nothing by then is skipped entirely, so a first
|
||||
registration made later never takes effect. The `ctx` a handler receives is built
|
||||
per invocation, not captured at install time, so `ctx.cwd` and `ctx.hasUI` describe
|
||||
the session as it is when the mutation is denied — a workspace change (`/move`) is
|
||||
reflected in the next request rather than pinned to load time.
|
||||
- **The registries are process-wide.** A process can host several sessions (a subagent
|
||||
gets its own runner), so a handler may be consulted for a denied write or delete
|
||||
from any session in the process — not only the one whose extension registered it.
|
||||
This is deliberate: a subagent spawned with restricted tools loads no extensions of
|
||||
its own, and a host that registers once in its top-level session still expects its
|
||||
subagents' writes brokered. `req.sessionId` names the session that issued the
|
||||
mutation (`undefined` when it did not come from a tool call), and
|
||||
`ctx.sessionManager.getSessionId()` names the handler's own — compare them to make
|
||||
the decision per session. It matters most before prompting: `ctx.ui` belongs to the
|
||||
handler's session, not necessarily to the one being asked about. Handlers are
|
||||
removed on `session_shutdown`.
|
||||
|
||||
With nothing registered none of this engages: the primitive runs exactly as it did
|
||||
before and performs no extra syscalls.
|
||||
|
||||
## UI integration points
|
||||
|
||||
`ctx.ui` implements the `ExtensionUIContext` interface. Support differs by mode.
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
### Fixed
|
||||
|
||||
- Raised the GPT-5.6 Sol/Terra/Luna context window on the Codex transport (openai-codex) from 372K to 1M tokens: OpenAI enabled the 1M window for subscription Codex on 2026-08-16, but the Codex model registry still reports the stale 272,000, so discovery now floors these SKUs at 1,000,000 instead of trusting the reported value ([openai/codex#38917](https://github.com/openai/codex/issues/38917)).
|
||||
|
||||
|
||||
### Changed
|
||||
|
||||
|
||||
@@ -145,7 +145,7 @@ const CODEX_GPT_5_4_PRIORITY_BY_VARIANT: Partial<Record<OpenAIVariant, number>>
|
||||
nano: 2,
|
||||
};
|
||||
|
||||
const CODEX_GPT_5_6_372K_MODEL_IDS: Record<string, true> = {
|
||||
const CODEX_GPT_5_6_1M_MODEL_IDS: Record<string, true> = {
|
||||
"gpt-5.6-luna": true,
|
||||
"gpt-5.6-sol": true,
|
||||
"gpt-5.6-terra": true,
|
||||
@@ -536,12 +536,12 @@ function applyOpenAICatalogPolicy(model: ModelSpec<Api>, parsedModel: OpenAIMode
|
||||
model.contextWindow = 272000;
|
||||
}
|
||||
}
|
||||
// GPT-5.6 luna/sol/terra on the Codex transport: OpenAI's Codex model
|
||||
// registry declares context_window = max_context_window = 372000, but Codex
|
||||
// discovery omits `context_window` for these SKUs and falls back to
|
||||
// DEFAULT_CONTEXT_WINDOW (272000, src/discovery/codex.ts), which regressed
|
||||
// the bundled hard capacity (#5705). Pin the true 372K input window.
|
||||
if (model.api === "openai-codex-responses" && CODEX_GPT_5_6_372K_MODEL_IDS[model.id]) {
|
||||
model.contextWindow = 372000;
|
||||
// GPT-5.6 luna/sol/terra on the Codex transport: OpenAI enabled a 1M-token
|
||||
// window for subscription Codex (2026-08-16), but the Codex model registry
|
||||
// still reports the stale 272000 (openai/codex#38917), so floor the bundled
|
||||
// window at 1,000,000. Daybreak aliases are excluded — the registry actively
|
||||
// reports their true window.
|
||||
if (model.api === "openai-codex-responses" && CODEX_GPT_5_6_1M_MODEL_IDS[model.id]) {
|
||||
model.contextWindow = Math.max(model.contextWindow ?? 0, 1_000_000);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,13 +9,19 @@ const DEFAULT_MODEL_LIST_PATHS = ["/codex/models", "/models"] as const;
|
||||
const DEFAULT_CONTEXT_WINDOW = 272_000;
|
||||
const DEFAULT_MAX_TOKENS = 128_000;
|
||||
/**
|
||||
* GPT-5.6 luna/sol/terra hard context capacity. Codex discovery omits
|
||||
* `context_window` for these SKUs, so the generic {@link DEFAULT_CONTEXT_WINDOW}
|
||||
* (272000) would understate the real window — OpenAI's Codex model registry
|
||||
* declares context_window = max_context_window = 372000 (#5705). Used as the
|
||||
* fallback only when upstream reports no value.
|
||||
* Fallback for GPT-5.6-family SKUs when upstream omits `context_window`: the
|
||||
* generic {@link DEFAULT_CONTEXT_WINDOW} (272000) understates the registry's
|
||||
* former 372000 hard capacity (#5705).
|
||||
*/
|
||||
const GPT_5_6_CONTEXT_WINDOW = 372_000;
|
||||
/**
|
||||
* OpenAI enabled a 1M-token window for subscription Codex on GPT-5.6
|
||||
* luna/sol/terra (2026-08-16), but the Codex model registry still reports the
|
||||
* stale 272000 — so the reported value must be floored, not just defaulted
|
||||
* (openai/codex#38917; Codex CLI override `model_context_window = 1000000`).
|
||||
*/
|
||||
const GPT_5_6_1M_CONTEXT_WINDOW = 1_000_000;
|
||||
const CODEX_GPT_5_6_1M_SLUGS: ReadonlySet<string> = new Set(["gpt-5.6-luna", "gpt-5.6-sol", "gpt-5.6-terra"]);
|
||||
const CODEX_REMOTE_COMPACTION = {
|
||||
enabled: true,
|
||||
api: "openai-codex-responses",
|
||||
@@ -224,14 +230,18 @@ function normalizeCodexModelEntry(entry: unknown, baseUrl: string): NormalizedCo
|
||||
}
|
||||
|
||||
const name = toNonEmptyString(payload.display_name) ?? slug;
|
||||
// Codex discovery omits `context_window` for GPT-5.6 luna/sol/terra; the
|
||||
// generic 272000 fallback understates their real 372000 window (#5705).
|
||||
// Codex discovery historically omitted `context_window` for GPT-5.6-family
|
||||
// SKUs (#5705); luna/sol/terra additionally floor the reported value because
|
||||
// the registry still declares the pre-1M 272000 window.
|
||||
const parsed = parseKnownModel(slug);
|
||||
const fallbackContextWindow =
|
||||
parsed.family === "openai" && semverEqual(parsed.version, "5.6")
|
||||
? GPT_5_6_CONTEXT_WINDOW
|
||||
: DEFAULT_CONTEXT_WINDOW;
|
||||
const contextWindow = toPositiveInt(payload.context_window) ?? fallbackContextWindow;
|
||||
const reportedContextWindow = toPositiveInt(payload.context_window) ?? fallbackContextWindow;
|
||||
const contextWindow = CODEX_GPT_5_6_1M_SLUGS.has(slug)
|
||||
? Math.max(reportedContextWindow, GPT_5_6_1M_CONTEXT_WINDOW)
|
||||
: reportedContextWindow;
|
||||
const maxTokens = Math.min(DEFAULT_MAX_TOKENS, contextWindow);
|
||||
const reasoning = supportsReasoning(payload.default_reasoning_level, payload.supported_reasoning_levels);
|
||||
const input = normalizeInputModalities(payload.input_modalities);
|
||||
|
||||
+682
-208
File diff suppressed because it is too large
Load Diff
@@ -103,7 +103,7 @@ describe("Codex model discovery", () => {
|
||||
expect(legacy?.useResponsesLite).toBeUndefined();
|
||||
});
|
||||
|
||||
it("falls back to the 372K window for GPT-5.6 SKUs when upstream omits context_window (#5705)", async () => {
|
||||
it("floors GPT-5.6 luna/sol/terra at the 1M window when upstream omits context_window (#5705)", async () => {
|
||||
const fetchFn: typeof fetch = Object.assign(
|
||||
async () =>
|
||||
new Response(
|
||||
@@ -138,7 +138,7 @@ describe("Codex model discovery", () => {
|
||||
});
|
||||
|
||||
const sol = result?.models.find(model => model.id === "gpt-5.6-sol");
|
||||
expect(sol?.contextWindow).toBe(372_000);
|
||||
expect(sol?.contextWindow).toBe(1_000_000);
|
||||
const legacy = result?.models.find(model => model.id === "gpt-5.5");
|
||||
expect(legacy?.contextWindow).toBe(272_000);
|
||||
});
|
||||
@@ -195,7 +195,7 @@ describe("Codex model discovery", () => {
|
||||
expect(red.cost).toEqual({ input: 12.5, output: 75, cacheRead: 1.25, cacheWrite: 15.625 });
|
||||
});
|
||||
|
||||
it("honors context_window when upstream actively reports it for GPT-5.6 SKUs", async () => {
|
||||
it("floors stale reported windows for GPT-5.6 luna/sol/terra and honors reports above the floor", async () => {
|
||||
const fetchFn: typeof fetch = Object.assign(
|
||||
async () =>
|
||||
new Response(
|
||||
@@ -210,6 +210,15 @@ describe("Codex model discovery", () => {
|
||||
input_modalities: ["text", "image"],
|
||||
supported_in_api: true,
|
||||
},
|
||||
{
|
||||
slug: "gpt-5.6-terra",
|
||||
display_name: "GPT-5.6-Terra",
|
||||
context_window: 1_050_000,
|
||||
default_reasoning_level: "medium",
|
||||
supported_reasoning_levels: ["low", "medium", "high"],
|
||||
input_modalities: ["text", "image"],
|
||||
supported_in_api: true,
|
||||
},
|
||||
{
|
||||
slug: "gpt-5.5",
|
||||
display_name: "GPT-5.5",
|
||||
@@ -231,8 +240,13 @@ describe("Codex model discovery", () => {
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
// Registry still reports the pre-1M 272000 for sol; the floor must win.
|
||||
const sol = result?.models.find(model => model.id === "gpt-5.6-sol");
|
||||
expect(sol?.contextWindow).toBe(272_000);
|
||||
expect(sol?.contextWindow).toBe(1_000_000);
|
||||
// Reports above the floor are honored as-is.
|
||||
const terra = result?.models.find(model => model.id === "gpt-5.6-terra");
|
||||
expect(terra?.contextWindow).toBe(1_050_000);
|
||||
// Non-floored SKUs keep the actively reported value.
|
||||
const legacy = result?.models.find(model => model.id === "gpt-5.5");
|
||||
expect(legacy?.contextWindow).toBe(272_000);
|
||||
});
|
||||
|
||||
@@ -120,9 +120,9 @@ describe("generated model policies", () => {
|
||||
expect(models[4]?.cost.longContext).toBeUndefined();
|
||||
});
|
||||
|
||||
it("pins GPT-5.6 Codex-transport context window to the 372K hard capacity (#5705)", () => {
|
||||
it("floors GPT-5.6 Codex-transport context windows at 1M (openai/codex#38917)", () => {
|
||||
const models: ModelSpec<Api>[] = [
|
||||
// Codex discovery underreports these via DEFAULT_CONTEXT_WINDOW=272000.
|
||||
// Codex discovery/registry still reports the stale 272000 for these.
|
||||
createSpec({
|
||||
id: "gpt-5.6-luna",
|
||||
api: "openai-codex-responses",
|
||||
@@ -155,9 +155,9 @@ describe("generated model policies", () => {
|
||||
|
||||
applyGeneratedModelPolicies(models);
|
||||
|
||||
expect(models[0]?.contextWindow).toBe(372000);
|
||||
expect(models[1]?.contextWindow).toBe(372000);
|
||||
expect(models[2]?.contextWindow).toBe(372000);
|
||||
expect(models[0]?.contextWindow).toBe(1_000_000);
|
||||
expect(models[1]?.contextWindow).toBe(1_000_000);
|
||||
expect(models[2]?.contextWindow).toBe(1_000_000);
|
||||
expect(models[3]?.contextWindow).toBe(1050000);
|
||||
expect(models[4]?.contextWindow).toBe(272000);
|
||||
});
|
||||
|
||||
@@ -1,65 +0,0 @@
|
||||
/**
|
||||
* Repro for #887 — OpenCode Go: Minimax M2.7 (and Qwen3.5/3.6 Plus) return 404
|
||||
* because the resolver routes them to anthropic-messages /v1/messages while
|
||||
* the OpenCode Go gateway only serves them at /v1/chat/completions.
|
||||
*
|
||||
* stencil.so declares these ids with `provider.npm = "@ai-sdk/anthropic"`,
|
||||
* which by default would resolve to anthropic-messages on opencode-go. The
|
||||
* descriptor must override these specific ids to openai-completions so that
|
||||
* regenerated models.json keeps the correct routing.
|
||||
*/
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import {
|
||||
MODELS_DEV_PROVIDER_DESCRIPTORS,
|
||||
type ModelsDevModel,
|
||||
opencodeGoModelManagerOptions,
|
||||
} from "@oh-my-pi/pi-catalog/provider-models/openai-compat";
|
||||
|
||||
const OPENCODE_GO_BASE = "https://opencode.ai/zen/go/v1";
|
||||
|
||||
describe("opencode-go resolver routes 404-ing ids to openai-completions (issue #887)", () => {
|
||||
const descriptor = MODELS_DEV_PROVIDER_DESCRIPTORS.find(d => d.providerId === "opencode-go");
|
||||
|
||||
// Per upstream stencil.so (verified 2026-05-02 against
|
||||
// https://stencil.so/api.json["opencode-go"].models), these three ids carry
|
||||
// `provider.npm = "@ai-sdk/anthropic"`. The naive @ai-sdk/anthropic rule
|
||||
// would route them to /v1/messages on opencode.ai/zen/go which 404s.
|
||||
const npmAnthropic: ModelsDevModel = { provider: { npm: "@ai-sdk/anthropic" }, tool_call: true };
|
||||
|
||||
test.each([["minimax-m2.7"], ["qwen3.5-plus"], ["qwen3.6-plus"]])(
|
||||
"%s resolves to openai-completions on /v1/chat/completions",
|
||||
modelId => {
|
||||
const resolved = descriptor?.resolveApi?.(modelId, npmAnthropic);
|
||||
expect(resolved).toEqual({ api: "openai-completions", baseUrl: OPENCODE_GO_BASE });
|
||||
},
|
||||
);
|
||||
|
||||
test("minimax-m2.5 (control: works empirically) also resolves to openai-completions", () => {
|
||||
// stencil.so currently lists minimax-m2.5 without an explicit provider.npm,
|
||||
// so it falls through to the default openai-completions resolution.
|
||||
const m25: ModelsDevModel = { tool_call: true };
|
||||
const resolved = descriptor?.resolveApi?.("minimax-m2.5", m25);
|
||||
expect(resolved).toEqual({ api: "openai-completions", baseUrl: OPENCODE_GO_BASE });
|
||||
});
|
||||
|
||||
test("runtime /v1/models refresh preserves qwen3.7-max Anthropic transport", async () => {
|
||||
let requestedUrl = "";
|
||||
const fetchMock = (async (input: string | Request | URL): Promise<Response> => {
|
||||
requestedUrl = input instanceof Request ? input.url : String(input);
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
data: [{ id: "qwen3.7-max", name: "Qwen3.7 Max", context_length: 1000000 }],
|
||||
}),
|
||||
{ headers: { "content-type": "application/json" } },
|
||||
);
|
||||
}) as typeof fetch;
|
||||
|
||||
const options = opencodeGoModelManagerOptions({ apiKey: "opencode-test-key", fetch: fetchMock });
|
||||
const models = await options.fetchDynamicModels?.();
|
||||
const qwenMax = models?.find(model => model.id === "qwen3.7-max");
|
||||
|
||||
expect(requestedUrl).toBe("https://opencode.ai/zen/go/v1/models");
|
||||
expect(qwenMax?.api).toBe("anthropic-messages");
|
||||
expect(qwenMax?.baseUrl).toBe("https://opencode.ai/zen/go");
|
||||
});
|
||||
});
|
||||
@@ -6,6 +6,12 @@
|
||||
|
||||
- Updated the default model for XAI_API_KEY (xai) and SuperGrok OAuth (xai-oauth) to grok-4.6. Automatic model selection continues to prefer paid xai/grok-4.6 when only XAI_API_KEY is set, with xai-oauth/grok-4.6 still available explicitly.
|
||||
|
||||
### Added
|
||||
|
||||
- Added `ExtensionAPI.registerFileWriteFallback(handler)` and `ExtensionAPI.registerFileDeleteFallback(handler)`, letting an extension supply a fallback writer or deleter that is consulted when a native `write`, `edit`, or `apply_patch` byte-write or unlink is denied with a permission error (`EPERM`/`EACCES`/`EROFS`) — for hosts that embed the agent inside a sandbox that denies direct filesystem access but exposes a privileged channel. The brokered path is symlink-resolved so a handler's allowlist sees the real destination, a destination that cannot be resolved is not brokered at all, and `req.sessionId` names the session that issued the mutation so a handler sharing the process-wide registry can enforce policy per session. See [`docs/extensions.md`](../../docs/extensions.md).
|
||||
### Fixed
|
||||
|
||||
- Fixed `omp stats` and `/stats` dashboards being unreachable from container hosts by accepting an explicit `--host` bind address while preserving the `127.0.0.1` default.
|
||||
|
||||
## [17.3.5] - 2026-08-16
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import { truncateToWidth } from "@oh-my-pi/pi-tui/utils";
|
||||
import { APP_NAME, formatDuration, formatNumber, formatPercent } from "@oh-my-pi/pi-utils";
|
||||
import { formatDuration, formatNumber, formatPercent } from "@oh-my-pi/pi-utils";
|
||||
import chalk from "@oh-my-pi/pi-utils/chalk";
|
||||
import { openPath } from "../utils/open";
|
||||
|
||||
@@ -57,45 +57,11 @@ function shortenSessionFile(p: string): string {
|
||||
|
||||
export interface StatsCommandArgs {
|
||||
port: number;
|
||||
host: string;
|
||||
json: boolean;
|
||||
summary: boolean;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Argument Parser
|
||||
// =============================================================================
|
||||
|
||||
/**
|
||||
* Parse stats subcommand arguments.
|
||||
* Returns undefined if not a stats command.
|
||||
*/
|
||||
export function parseStatsArgs(args: string[]): StatsCommandArgs | undefined {
|
||||
if (args.length === 0 || args[0] !== "stats") {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const result: StatsCommandArgs = {
|
||||
port: 3847,
|
||||
json: false,
|
||||
summary: false,
|
||||
};
|
||||
|
||||
for (let i = 1; i < args.length; i++) {
|
||||
const arg = args[i];
|
||||
if (arg === "--json" || arg === "-j") {
|
||||
result.json = true;
|
||||
} else if (arg === "--summary" || arg === "-s") {
|
||||
result.summary = true;
|
||||
} else if ((arg === "--port" || arg === "-p") && i + 1 < args.length) {
|
||||
result.port = parseInt(args[++i], 10);
|
||||
} else if (arg.startsWith("--port=")) {
|
||||
result.port = parseInt(arg.split("=")[1], 10);
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
function formatCost(n: number): string {
|
||||
if (n < 0.01) return `$${n.toFixed(4)}`;
|
||||
if (n < 1) return `$${n.toFixed(3)}`;
|
||||
@@ -112,9 +78,8 @@ function normalizePremiumRequests(n: number): number {
|
||||
|
||||
export async function runStatsCommand(cmd: StatsCommandArgs): Promise<void> {
|
||||
// Lazy import to avoid loading stats module when not needed
|
||||
const { getDashboardStats, syncAllSessions, getTotalMessageCount, startServer, closeDb } = await import(
|
||||
"@oh-my-pi/omp-stats"
|
||||
);
|
||||
const { closeDb, formatStatsDashboardUrl, getDashboardStats, getTotalMessageCount, startServer, syncAllSessions } =
|
||||
await import("@oh-my-pi/omp-stats");
|
||||
|
||||
// Sync session files first
|
||||
const progress = createSyncProgressReporter();
|
||||
@@ -136,8 +101,8 @@ export async function runStatsCommand(cmd: StatsCommandArgs): Promise<void> {
|
||||
}
|
||||
|
||||
// Start the dashboard server
|
||||
const { hostname, port } = await startServer(cmd.port);
|
||||
const url = `http://${hostname}:${port}`;
|
||||
const { hostname, port } = await startServer(cmd.port, cmd.host);
|
||||
const url = formatStatsDashboardUrl(hostname, port);
|
||||
console.log(chalk.green(`Dashboard available at: ${url}`));
|
||||
|
||||
// Open browser
|
||||
@@ -197,34 +162,3 @@ async function printStatsSummary(): Promise<void> {
|
||||
|
||||
console.log("");
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Help
|
||||
// =============================================================================
|
||||
|
||||
export function printStatsHelp(): void {
|
||||
console.log(`${chalk.bold(`${APP_NAME} stats`)} - AI Usage Statistics Dashboard
|
||||
|
||||
${chalk.bold("Usage:")}
|
||||
${APP_NAME} stats [options]
|
||||
|
||||
${chalk.bold("Options:")}
|
||||
-p, --port <port> Port for the dashboard server (default: 3847)
|
||||
-j, --json Output stats as JSON and exit
|
||||
-s, --summary Print summary to console and exit
|
||||
-h, --help Show this help message
|
||||
|
||||
${chalk.bold("Examples:")}
|
||||
${APP_NAME} stats # Start dashboard server
|
||||
${APP_NAME} stats --json # Print stats as JSON
|
||||
${APP_NAME} stats --summary # Print summary to console
|
||||
${APP_NAME} stats --port 8080 # Start on custom port
|
||||
|
||||
${chalk.bold("Metrics:")}
|
||||
- Total requests and error rate
|
||||
- Token usage (input, output, cache)
|
||||
- Cost breakdown
|
||||
- Average duration and time to first token (TTFT)
|
||||
- Tokens per second throughput
|
||||
`);
|
||||
}
|
||||
|
||||
@@ -4,13 +4,15 @@
|
||||
|
||||
import { Command, Flags } from "@oh-my-pi/pi-utils/cli";
|
||||
import { statsHelp as commandHelp } from "../cli/command-help";
|
||||
import { runStatsCommand, type StatsCommandArgs } from "../cli/stats-cli";
|
||||
import { initTheme } from "../modes/theme/theme";
|
||||
import type { StatsCommandArgs } from "../cli/stats-cli";
|
||||
import * as statsCli from "../cli/stats-cli";
|
||||
import * as theme from "../modes/theme/theme";
|
||||
|
||||
export default class Stats extends Command {
|
||||
static description = commandHelp.description;
|
||||
static flags = {
|
||||
port: Flags.integer({ char: "p", description: "Port for the dashboard server", default: 3847 }),
|
||||
host: Flags.string({ description: "Host to bind", default: "127.0.0.1" }),
|
||||
json: Flags.boolean({ char: "j", description: "Output stats as JSON", default: false }),
|
||||
summary: Flags.boolean({ char: "s", description: "Print summary to console", default: false }),
|
||||
};
|
||||
@@ -20,11 +22,12 @@ export default class Stats extends Command {
|
||||
|
||||
const cmd: StatsCommandArgs = {
|
||||
port: flags.port,
|
||||
host: flags.host ?? "127.0.0.1",
|
||||
json: flags.json,
|
||||
summary: flags.summary,
|
||||
};
|
||||
|
||||
await initTheme();
|
||||
await runStatsCommand(cmd);
|
||||
await theme.initTheme();
|
||||
await statsCli.runStatsCommand(cmd);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -25,6 +25,7 @@ import { FileChangeType, notifyWorkspaceWatchedFiles } from "../../lsp/client";
|
||||
import type { ToolSession } from "../../tools";
|
||||
import { routeWriteThroughBridge } from "../../tools/acp-bridge";
|
||||
import { assertEditableFileContent } from "../../tools/auto-generated-guard";
|
||||
import { deleteFileWithFallback, writeFileWithFallback } from "../../tools/file-write-fallback";
|
||||
import { invalidateFsScanAfterWrite } from "../../tools/fs-cache-invalidation";
|
||||
import { isInternalUrlPath } from "../../tools/path-utils";
|
||||
import { enforcePlanModeWrite, resolvePlanPath, targetsLocalSandbox } from "../../tools/plan-mode-guard";
|
||||
@@ -153,7 +154,7 @@ export class HashlineFilesystem extends Filesystem {
|
||||
enforcePlanModeWrite(this.session, relativePath, { op: "delete" });
|
||||
const absolutePath = this.resolveAbsolute(relativePath);
|
||||
try {
|
||||
await fs.rm(absolutePath);
|
||||
await deleteFileWithFallback(absolutePath);
|
||||
} catch (error) {
|
||||
if (isEnoent(error)) throw new NotFoundError(relativePath, error);
|
||||
throw error;
|
||||
@@ -173,8 +174,13 @@ export class HashlineFilesystem extends Filesystem {
|
||||
const fromAbsolute = this.resolveAbsolute(fromRelative);
|
||||
const toAbsolute = this.resolveAbsolute(toRelative);
|
||||
if (content !== undefined) {
|
||||
await Bun.write(toAbsolute, content);
|
||||
await fs.rm(fromAbsolute);
|
||||
// The one `edit` write that does not pass through the writethrough, so it
|
||||
// routes to the fallback seam directly. `patcher.ts` always supplies
|
||||
// `content` for a hashline `MV`, making this the live branch. The source
|
||||
// unlink is a separate primitive with its own seam, so a move out of the
|
||||
// workspace and a move out of denied territory both complete.
|
||||
await writeFileWithFallback(toAbsolute, content);
|
||||
await deleteFileWithFallback(fromAbsolute);
|
||||
} else {
|
||||
await fs.rename(fromAbsolute, toAbsolute);
|
||||
}
|
||||
|
||||
@@ -20,6 +20,12 @@ import { FileChangeType, notifyWorkspaceWatchedFiles } from "../../lsp/client";
|
||||
import type { ToolSession } from "../../tools";
|
||||
import { routeWriteThroughBridge } from "../../tools/acp-bridge";
|
||||
import { assertEditableFile } from "../../tools/auto-generated-guard";
|
||||
import {
|
||||
deleteFileWithFallback,
|
||||
hasFileWriteFallback,
|
||||
isPermissionDeniedError,
|
||||
writeFileWithFallback,
|
||||
} from "../../tools/file-write-fallback";
|
||||
import {
|
||||
invalidateFsScanAfterDelete,
|
||||
invalidateFsScanAfterRename,
|
||||
@@ -111,6 +117,26 @@ export interface ApplyPatchOptions {
|
||||
// Default File System
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
/**
|
||||
* Create a patch target's parent directory, tolerating a permission denial when a
|
||||
* file-write fallback is registered.
|
||||
*
|
||||
* `apply_patch` mkdirs the parent before writing, so under a sandbox that denies
|
||||
* the out-of-tree path this throws before the write — and therefore before
|
||||
* {@link writeFileWithFallback} — is ever reached, leaving the fallback unable to
|
||||
* broker a `create` or a rename-move into a new directory. Swallowing only a
|
||||
* permission denial, and only with a handler installed, hands control to the write,
|
||||
* which reports the denial through the seam. Without a handler the error propagates
|
||||
* exactly as before.
|
||||
*/
|
||||
async function mkdirAllowingFallback(dir: string): Promise<void> {
|
||||
try {
|
||||
await fs.promises.mkdir(dir, { recursive: true });
|
||||
} catch (error) {
|
||||
if (!hasFileWriteFallback() || !isPermissionDeniedError(error)) throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/** Default filesystem implementation using Bun APIs */
|
||||
export const defaultFileSystem: FileSystem = {
|
||||
async exists(path: string): Promise<boolean> {
|
||||
@@ -123,13 +149,13 @@ export const defaultFileSystem: FileSystem = {
|
||||
return fs.promises.readFile(path);
|
||||
},
|
||||
async write(path: string, content: string): Promise<void> {
|
||||
await Bun.write(path, await serializeEditFileText(path, path, content));
|
||||
await writeFileWithFallback(path, await serializeEditFileText(path, path, content));
|
||||
},
|
||||
async delete(path: string): Promise<void> {
|
||||
await fs.promises.unlink(path);
|
||||
await deleteFileWithFallback(path);
|
||||
},
|
||||
async mkdir(path: string): Promise<void> {
|
||||
await fs.promises.mkdir(path, { recursive: true });
|
||||
await mkdirAllowingFallback(path);
|
||||
},
|
||||
};
|
||||
|
||||
@@ -1753,7 +1779,7 @@ class LspFileSystem implements FileSystem {
|
||||
}
|
||||
|
||||
async delete(path: string): Promise<void> {
|
||||
await this.#getFile(path).unlink();
|
||||
await deleteFileWithFallback(path, this.#getFile(path));
|
||||
if (this.session.enableLsp ?? true) {
|
||||
await notifyWorkspaceWatchedFiles(
|
||||
this.session.cwd,
|
||||
@@ -1764,7 +1790,7 @@ class LspFileSystem implements FileSystem {
|
||||
}
|
||||
|
||||
async mkdir(path: string): Promise<void> {
|
||||
await fs.promises.mkdir(path, { recursive: true });
|
||||
await mkdirAllowingFallback(path);
|
||||
}
|
||||
|
||||
getDiagnostics(): FileDiagnosticsResult | undefined {
|
||||
|
||||
@@ -28,6 +28,7 @@ import { execCommand } from "../../exec/exec";
|
||||
// Runtime self-reference: dereference this namespace only inside loader functions to keep the index.ts cycle safe.
|
||||
import * as PiCodingAgent from "../../index";
|
||||
import type { CustomMessagePayload } from "../../session/messages";
|
||||
import type { FileDeleteFallbackHandler, FileWriteFallbackHandler } from "../../tools/file-write-fallback";
|
||||
import { EventBus } from "../../utils/event-bus";
|
||||
import * as TypeBox from "../legacy-typebox";
|
||||
import { installLegacyPiSpecifierShim, loadLegacyPiModule } from "../plugins/legacy-pi-compat";
|
||||
@@ -183,6 +184,14 @@ class ConcreteExtensionAPI implements ExtensionAPI, IExtensionRuntime {
|
||||
for (const listener of this.extension.toolRegistrationListeners ?? []) listener(tool.name);
|
||||
}
|
||||
|
||||
registerFileWriteFallback(handler: FileWriteFallbackHandler): void {
|
||||
this.extension.fileWriteFallbackHandlers.push(handler);
|
||||
}
|
||||
|
||||
registerFileDeleteFallback(handler: FileDeleteFallbackHandler): void {
|
||||
this.extension.fileDeleteFallbackHandlers.push(handler);
|
||||
}
|
||||
|
||||
registerCommand(
|
||||
name: string,
|
||||
options: {
|
||||
@@ -320,6 +329,8 @@ function createExtension(extensionPath: string, resolvedPath: string): Extension
|
||||
tools: new Map(),
|
||||
toolRegistrationListeners: new Set(),
|
||||
assistantThinkingRenderers: [],
|
||||
fileWriteFallbackHandlers: [],
|
||||
fileDeleteFallbackHandlers: [],
|
||||
messageRenderers: new Map(),
|
||||
commands: new Map(),
|
||||
flags: new Map(),
|
||||
|
||||
@@ -19,6 +19,7 @@ import type { MemoryRuntimeContext } from "../../memory-backend";
|
||||
import { type Theme, theme } from "../../modes/theme/theme";
|
||||
import type { AsyncJobSnapshot } from "../../session/agent-session";
|
||||
import type { SessionManager } from "../../session/session-manager";
|
||||
import { addFileDeleteFallback, addFileWriteFallback } from "../../tools/file-write-fallback";
|
||||
import type { BranchHandler, NavigateTreeHandler, NewSessionHandler } from "../session-handler-types";
|
||||
import { ManagedTimers } from "./managed-timers";
|
||||
import { createExtensionModelQuery } from "./model-api";
|
||||
@@ -375,10 +376,12 @@ export type SwitchSessionHandler = (sessionPath: string) => Promise<{ cancelled:
|
||||
export type ShutdownHandler = () => void;
|
||||
|
||||
/**
|
||||
* Emit `session_shutdown` and clear timers owned by an extension runner.
|
||||
* Emit `session_shutdown`, dispose file-write-fallback registrations, and clear
|
||||
* timers owned by an extension runner.
|
||||
*
|
||||
* Returns whether any shutdown handlers were present. Timer cleanup runs even
|
||||
* when a handler fails so extension background work cannot outlive its host.
|
||||
* Returns whether any shutdown handlers were present. Fallback disposal and timer
|
||||
* cleanup run even when a handler fails so extension background work — and a
|
||||
* fallback bound to this session's context — cannot outlive its host.
|
||||
*/
|
||||
export async function emitSessionShutdownEvent(extensionRunner: ExtensionRunner | undefined): Promise<boolean> {
|
||||
if (!extensionRunner) return false;
|
||||
@@ -389,6 +392,7 @@ export async function emitSessionShutdownEvent(extensionRunner: ExtensionRunner
|
||||
});
|
||||
return true;
|
||||
} finally {
|
||||
extensionRunner.disposeFileFallbacks();
|
||||
extensionRunner.clearManagedTimers();
|
||||
}
|
||||
}
|
||||
@@ -486,6 +490,23 @@ export class ExtensionRunner {
|
||||
#managedTimers = new ManagedTimers((event, error, stack) =>
|
||||
this.emitError({ extensionPath: "<timer>", event, error, stack }),
|
||||
);
|
||||
/**
|
||||
* Disposers for the trampolines installed via {@link addFileWriteFallback} and
|
||||
* {@link addFileDeleteFallback} — one per extension per seam it registered for.
|
||||
* Installed during {@link initialize} (after the UI/runtime context is live, so
|
||||
* the bound handler sees a working `ctx.ui`) and drained by
|
||||
* {@link disposeFileFallbacks} on session shutdown so a handler from a
|
||||
* torn-down session can never fire for a later one sharing the same process.
|
||||
*
|
||||
* Each trampoline re-reads its extension's handler list at call time rather than
|
||||
* closing over a snapshot, matching how `ext.handlers` is re-read on every emit,
|
||||
* so an extension that already had a handler for that seam at `initialize` picks
|
||||
* up later additions to it. A seam the extension registered NOTHING for gets no
|
||||
* trampoline at all, which keeps the registry empty for a host with no fallbacks;
|
||||
* the cost is that a first registration for that seam after `initialize` never
|
||||
* takes effect, which is why the API documents load-time registration.
|
||||
*/
|
||||
#fileFallbackDisposers: Array<() => void> = [];
|
||||
/**
|
||||
* Dedup markers for `tool_call` emission, keyed `${toolCallId}:${toolName}`.
|
||||
* The agent loop emits `tool_call` at arg-prep time (before scheduling and
|
||||
@@ -609,6 +630,18 @@ export class ExtensionRunner {
|
||||
return this.sessionManager.getCwd();
|
||||
}
|
||||
|
||||
/**
|
||||
* Stable id of the session this runner serves. Read through `sessionManager`
|
||||
* for the same reason as {@link cwd}: it is this session's own, never a
|
||||
* process-global, so a subagent runner reports itself and not its parent.
|
||||
*
|
||||
* Used to attribute a denied file write or delete to the session that issued
|
||||
* it, since the fallback registry those handlers live in is process-wide.
|
||||
*/
|
||||
get sessionId(): string {
|
||||
return this.sessionManager.getSessionId();
|
||||
}
|
||||
|
||||
initialize(
|
||||
actions: ExtensionActions,
|
||||
contextActions: ExtensionContextActions,
|
||||
@@ -666,6 +699,75 @@ export class ExtensionRunner {
|
||||
this.#mode = mode;
|
||||
this.#initialized = true;
|
||||
|
||||
// Re-initialize (e.g. a mode switch rewiring UI/runtime actions) must not
|
||||
// accumulate duplicate global registrations — drop the prior generation before
|
||||
// installing this one's trampolines.
|
||||
this.disposeFileFallbacks();
|
||||
for (const ext of this.extensions) {
|
||||
// Nothing registered by this extension means no trampoline, so a host with
|
||||
// no fallback-registering extension leaves the seam genuinely empty and
|
||||
// `hasFileWriteFallback()`/`hasFileDeleteFallback()` false — the invariant
|
||||
// the whole feature rests on. Each seam is checked separately, so an
|
||||
// extension that only brokers writes never appears in the delete registry.
|
||||
if (ext.fileWriteFallbackHandlers.length === 0 && ext.fileDeleteFallbackHandlers.length === 0) continue;
|
||||
// One trampoline per extension per seam, not per handler: the list is walked
|
||||
// at mutation time so a handler this extension adds later still takes effect,
|
||||
// and `createContext()` takes no extension argument, so within one invocation
|
||||
// a single context is all any of this extension's handlers would have
|
||||
// received anyway.
|
||||
//
|
||||
// The context is built PER INVOCATION rather than captured here, matching
|
||||
// every other dispatch site. `createContext()` materializes `cwd` and
|
||||
// `hasUI` as values, so a trampoline holding one context for the life of the
|
||||
// session would keep handing handlers the workspace this runner initialized
|
||||
// in — wrong the moment `SessionManager.moveTo()` relocates the session
|
||||
// (`/move`), and a handler that scopes or prompts against `ctx.cwd` would
|
||||
// then allow the old workspace and deny the new one. A denied mutation is a
|
||||
// rare path, so the extra object costs nothing that matters.
|
||||
//
|
||||
// Isolation is per HANDLER, not per extension. The registry only sees one
|
||||
// trampoline per extension, so a throw escaping this loop would advance the
|
||||
// registry to the NEXT extension and skip every later handler this one
|
||||
// registered — breaking both the documented "a throwing handler is skipped"
|
||||
// contract and registration order for a backup-handler setup.
|
||||
if (ext.fileWriteFallbackHandlers.length > 0) {
|
||||
this.#fileFallbackDisposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
const ctx = this.createContext();
|
||||
for (const handler of ext.fileWriteFallbackHandlers) {
|
||||
try {
|
||||
if (await handler(req, ctx)) return true;
|
||||
} catch (error) {
|
||||
logger.warn("Extension file write fallback handler threw; trying next handler", {
|
||||
extension: ext.path,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}),
|
||||
);
|
||||
}
|
||||
if (ext.fileDeleteFallbackHandlers.length > 0) {
|
||||
this.#fileFallbackDisposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
const ctx = this.createContext();
|
||||
for (const handler of ext.fileDeleteFallbackHandlers) {
|
||||
try {
|
||||
if (await handler(req, ctx)) return true;
|
||||
} catch (error) {
|
||||
logger.warn("Extension file delete fallback handler threw; trying next handler", {
|
||||
extension: ext.path,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Drain events buffered by emitCredentialDisabled() before initialize ran. The
|
||||
// spread adds the `type` discriminator — `event` is the pi-ai shape (no `type`).
|
||||
// Deferred by one microtask so callers that register an onError listener
|
||||
@@ -1098,6 +1200,16 @@ export class ExtensionRunner {
|
||||
this.#managedTimers.clearAll();
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove every file write and delete fallback this runner installed into the
|
||||
* process-wide registries. Called on session shutdown (and before reinstalling
|
||||
* on a re-{@link initialize}) so a handler bound to a torn-down session's
|
||||
* context can never fire for another session sharing this process.
|
||||
*/
|
||||
disposeFileFallbacks(): void {
|
||||
for (const dispose of this.#fileFallbackDisposers.splice(0)) dispose();
|
||||
}
|
||||
|
||||
createCommandContext(): ExtensionCommandContext {
|
||||
return {
|
||||
...this.createContext(),
|
||||
|
||||
@@ -76,6 +76,7 @@ import type {
|
||||
WriteToolInput,
|
||||
} from "../../tools";
|
||||
import type { ApprovalMode } from "../../tools/approval";
|
||||
import type { FileDeleteFallbackHandler, FileWriteFallbackHandler } from "../../tools/file-write-fallback";
|
||||
import type { EventBus } from "../../utils/event-bus";
|
||||
import type {
|
||||
AgentEndEvent,
|
||||
@@ -1256,6 +1257,63 @@ export interface ExtensionAPI {
|
||||
/** Register a tool that the LLM can call. */
|
||||
registerTool<TParams extends TSchema = TSchema, TDetails = unknown>(tool: ToolDefinition<TParams, TDetails>): void;
|
||||
|
||||
/**
|
||||
* Register a fallback writer consulted when a native `write`/`edit` byte-write is
|
||||
* denied with a permission error (`EPERM`/`EACCES`/`EROFS`). Every other write
|
||||
* error is unaffected. Handlers run in registration order; the first one to
|
||||
* resolve `true` counts as the bytes being durably on disk, and the native tool
|
||||
* continues as if its own write had succeeded — including recording its file
|
||||
* snapshot under the real destination path, so a later hashline `edit` on that
|
||||
* path keeps working. Intended for a host embedding the agent inside a sandbox
|
||||
* that denies direct filesystem writes but exposes a privileged write channel.
|
||||
*
|
||||
* A denial that `Bun.write` masks as `ENOENT` — a write into a directory the host
|
||||
* may not create — also diverts here, with `req.dst`'s parent absent and the
|
||||
* handler responsible for creating it.
|
||||
*
|
||||
* `req.dst` is symlink-RESOLVED: the path the failed write itself acted on, not
|
||||
* the one the tool was given. A link anywhere in a lexical path redirects the
|
||||
* bytes while still passing a prefix allowlist, so treat `req.dst` as
|
||||
* authoritative. A destination that cannot be resolved is never brokered.
|
||||
*
|
||||
* Call this during extension load, like the other `register*` methods: handlers
|
||||
* are installed when the runner initializes, so an extension that has registered
|
||||
* none by then is skipped and a first registration made later never takes effect.
|
||||
*
|
||||
* The underlying registry is process-wide, so a handler may be consulted for a
|
||||
* denied write from any session in the process, not only its own.
|
||||
* `req.sessionId` names the session that issued the write and
|
||||
* `ctx.sessionManager.getSessionId()` names the handler's own; compare them
|
||||
* before prompting, because `ctx.ui` belongs to the latter. See
|
||||
* `docs/extensions.md`.
|
||||
*/
|
||||
registerFileWriteFallback(handler: FileWriteFallbackHandler): void;
|
||||
|
||||
/**
|
||||
* Register a fallback deleter consulted when a native `edit`/`apply_patch` unlink is
|
||||
* denied with a permission error (`EPERM`/`EACCES`/`EROFS`). Covers `edit`'s `REM`,
|
||||
* the source side of a hashline `MV`, and `apply_patch`'s delete op. Return `true`
|
||||
* once `dst` is gone from disk.
|
||||
*
|
||||
* A handler MUST remove `dst` with a plain unlink and MUST NOT fall back to a
|
||||
* recursive removal. `unlink` on a directory reports `EPERM` on Darwin, so the seam
|
||||
* checks the target before diverting — but when the target's own metadata is behind
|
||||
* the same boundary that denied the unlink, which is the common sandbox case, that
|
||||
* check cannot be resolved and `dst` may be a directory. `req.confirmedFile` says
|
||||
* which situation the handler is in.
|
||||
*
|
||||
* `req.dst` resolves every component ABOVE the last, for the same reason the
|
||||
* write seam resolves all of them; the last is left alone because `unlink`
|
||||
* removes a link rather than its target, so `req.dst` may name a link.
|
||||
*
|
||||
* Separate from {@link registerFileWriteFallback} on purpose. A write handler
|
||||
* brokers `req.content` to `req.dst`, so a delete request reaching it with no
|
||||
* content invites brokering an empty write and truncating the file instead of
|
||||
* removing it. Registering for deletes is therefore an explicit opt-in, and the
|
||||
* same load-time and process-wide notes above apply.
|
||||
*/
|
||||
registerFileDeleteFallback(handler: FileDeleteFallbackHandler): void;
|
||||
|
||||
// =========================================================================
|
||||
// Command, Shortcut, Flag Registration
|
||||
// =========================================================================
|
||||
@@ -1632,6 +1690,8 @@ export interface Extension {
|
||||
tools: Map<string, RegisteredTool<any, any>>;
|
||||
toolRegistrationListeners?: Set<ToolRegistrationListener>;
|
||||
assistantThinkingRenderers: AssistantThinkingRenderer[];
|
||||
fileWriteFallbackHandlers: FileWriteFallbackHandler[];
|
||||
fileDeleteFallbackHandlers: FileDeleteFallbackHandler[];
|
||||
messageRenderers: Map<string, MessageRenderer>;
|
||||
commands: Map<string, RegisteredCommand>;
|
||||
flags: Map<string, ExtensionFlag>;
|
||||
|
||||
@@ -14,6 +14,7 @@ import type { Settings } from "../../config/settings";
|
||||
import type { Theme } from "../../modes/theme/theme";
|
||||
import { type ApprovalMode, formatApprovalPrompt, resolveApproval, truncateForPrompt } from "../../tools/approval";
|
||||
import { defaultLoadModeForToolName } from "../../tools/essential-tools";
|
||||
import { withFileMutationSession } from "../../tools/file-write-fallback";
|
||||
import { normalizeToolEventInput, resolveToolEventInput } from "../tool-event-input";
|
||||
import { applyToolProxy } from "../tool-proxy";
|
||||
import type { ExtensionRunner } from "./runner";
|
||||
@@ -349,7 +350,15 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
|
||||
let executionError: Error | undefined;
|
||||
|
||||
try {
|
||||
result = await this.tool.execute(toolCallId, effectiveParams, signal, onUpdate, context);
|
||||
// A denied file write or delete inside this tool can be brokered to an
|
||||
// extension handler, and that registry is PROCESS-WIDE — so the session is
|
||||
// named here, the one place where every tool's execution and the runner
|
||||
// that owns the handlers are both in scope (`sdk.ts` wraps the whole tool
|
||||
// registry with this class whenever a runner exists). Inert with no
|
||||
// fallback registered: no scope is entered.
|
||||
result = await withFileMutationSession(this.runner.sessionId, () =>
|
||||
this.tool.execute(toolCallId, effectiveParams, signal, onUpdate, context),
|
||||
);
|
||||
} catch (err) {
|
||||
executionError = err instanceof Error ? err : new Error(String(err));
|
||||
result = {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import * as fs from "node:fs";
|
||||
import { isEnoent, logger, once, untilAborted } from "@oh-my-pi/pi-utils";
|
||||
import type { BunFile } from "bun";
|
||||
import { isPermissionDeniedError, writeFileWithFallback } from "../tools/file-write-fallback";
|
||||
import { FileChangeType, notifyWorkspaceWatchedFiles } from "./client";
|
||||
import { getServersForFile } from "./config";
|
||||
import {
|
||||
@@ -66,11 +67,7 @@ export async function writethroughNoop(
|
||||
_batch?: LspWritethroughBatchRequest,
|
||||
_getDeferred?: (dst: string) => WritethroughDeferredHandle | undefined,
|
||||
): Promise<FileDiagnosticsResult | undefined> {
|
||||
if (file) {
|
||||
await file.write(content);
|
||||
} else {
|
||||
await Bun.write(dst, content);
|
||||
}
|
||||
await writeFileWithFallback(dst, content, file);
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -78,6 +75,12 @@ interface PendingWritethrough {
|
||||
dst: string;
|
||||
file?: BunFile;
|
||||
changeType: FileChangeType;
|
||||
/**
|
||||
* The bytes this entry committed. The flush prefers a fresh read of `dst` so
|
||||
* post-processing sees whatever else in the batch touched the file, and falls
|
||||
* back to these when that read is denied.
|
||||
*/
|
||||
content: string;
|
||||
}
|
||||
|
||||
interface RunLspWritethroughOptions {
|
||||
@@ -288,7 +291,7 @@ async function runLspWritethrough(
|
||||
const contentAlreadyWritten = runOptions?.contentAlreadyWritten ?? false;
|
||||
|
||||
let finalContent = content;
|
||||
const writeContent = async (value: string) => (file ? file.write(value) : Bun.write(dst, value));
|
||||
const writeContent = async (value: string) => writeFileWithFallback(dst, value, file);
|
||||
const getWritePromise = once(() =>
|
||||
contentAlreadyWritten && finalContent === content ? Promise.resolve() : writeContent(finalContent),
|
||||
);
|
||||
@@ -458,9 +461,16 @@ async function flushWritethroughBatch(
|
||||
try {
|
||||
content = await fs.promises.readFile(entry.dst, "utf8");
|
||||
} catch (error) {
|
||||
if (!isEnoent(error)) throw error;
|
||||
bundle?.finalize(undefined);
|
||||
continue;
|
||||
if (isEnoent(error)) {
|
||||
bundle?.finalize(undefined);
|
||||
continue;
|
||||
}
|
||||
// A brokered write lands bytes this process may not be able to read
|
||||
// back: a sandbox that denies the write commonly denies the read too.
|
||||
// Failing here would fail a flush whose every write succeeded, so the
|
||||
// content this entry committed stands in for the unreadable file.
|
||||
if (!isPermissionDeniedError(error)) throw error;
|
||||
content = entry.content;
|
||||
}
|
||||
const deferredInner =
|
||||
bundle &&
|
||||
@@ -552,7 +562,7 @@ export function createLspWritethrough(cwd: string, options?: WritethroughOptions
|
||||
}
|
||||
|
||||
const state = getOrCreateWritethroughBatch(batch.id, resolvedOptions);
|
||||
state.entries.set(dst, { dst, file, changeType });
|
||||
state.entries.set(dst, { dst, file, changeType, content });
|
||||
if (!batch.flush) return undefined;
|
||||
|
||||
writethroughBatches.delete(batch.id);
|
||||
|
||||
@@ -336,7 +336,7 @@ export const BUILTIN_SESSION_SLASH_COMMANDS: ReadonlyArray<SlashCommandSpec> = [
|
||||
{
|
||||
name: "stats",
|
||||
description: "Launch the local stats dashboard",
|
||||
inlineHint: "[--port <port>]",
|
||||
inlineHint: "[--port <port>] [--host <host>]",
|
||||
allowArgs: true,
|
||||
handle: async (command, runtime) => {
|
||||
const parsed = parseStatsDashboardArgs(command.args);
|
||||
|
||||
@@ -11,6 +11,7 @@ interface StatsDashboardServer {
|
||||
|
||||
export interface StatsDashboardArgs {
|
||||
port: number;
|
||||
host: string;
|
||||
}
|
||||
|
||||
export interface StatsDashboardLaunchResult {
|
||||
@@ -20,7 +21,7 @@ export interface StatsDashboardLaunchResult {
|
||||
|
||||
let activeStatsServer: StatsDashboardServer | undefined;
|
||||
|
||||
const STATS_DASHBOARD_USAGE = "Usage: /stats [--port <port>]";
|
||||
const STATS_DASHBOARD_USAGE = "Usage: /stats [--port <port>] [--host <host>]";
|
||||
|
||||
function parsePort(value: string | undefined): number | string {
|
||||
if (!value) return `Missing port. ${STATS_DASHBOARD_USAGE}`;
|
||||
@@ -33,6 +34,7 @@ function parsePort(value: string | undefined): number | string {
|
||||
export function parseStatsDashboardArgs(args: string): StatsDashboardArgs | { error: string } {
|
||||
const tokens = args.split(/\s+/).filter(Boolean);
|
||||
let port = DEFAULT_STATS_DASHBOARD_PORT;
|
||||
let host = "127.0.0.1";
|
||||
|
||||
for (let i = 0; i < tokens.length; i++) {
|
||||
const token = tokens[i];
|
||||
@@ -48,28 +50,40 @@ export function parseStatsDashboardArgs(args: string): StatsDashboardArgs | { er
|
||||
port = parsed;
|
||||
continue;
|
||||
}
|
||||
if (token === "--host") {
|
||||
const value = tokens[++i];
|
||||
if (!value) return { error: `Missing host. ${STATS_DASHBOARD_USAGE}` };
|
||||
host = value;
|
||||
continue;
|
||||
}
|
||||
if (token.startsWith("--host=")) {
|
||||
const value = token.slice("--host=".length);
|
||||
if (!value) return { error: `Missing host. ${STATS_DASHBOARD_USAGE}` };
|
||||
host = value;
|
||||
continue;
|
||||
}
|
||||
return { error: `Unknown option: ${token}. ${STATS_DASHBOARD_USAGE}` };
|
||||
}
|
||||
|
||||
return { port };
|
||||
return { port, host };
|
||||
}
|
||||
|
||||
export async function launchStatsDashboard(args: StatsDashboardArgs): Promise<StatsDashboardLaunchResult> {
|
||||
const { processed, files } = await stats.syncAllSessions();
|
||||
const total = await stats.getTotalMessageCount();
|
||||
let requestedPortIgnored = false;
|
||||
let requestedAddressIgnored = false;
|
||||
|
||||
if (!activeStatsServer) {
|
||||
activeStatsServer = await stats.startServer(args.port);
|
||||
} else if (args.port !== activeStatsServer.port) {
|
||||
requestedPortIgnored = true;
|
||||
activeStatsServer = await stats.startServer(args.port, args.host);
|
||||
} else if (args.port !== activeStatsServer.port || args.host !== activeStatsServer.hostname) {
|
||||
requestedAddressIgnored = true;
|
||||
}
|
||||
|
||||
const url = `http://${activeStatsServer.hostname}:${activeStatsServer.port}`;
|
||||
const url = stats.formatStatsDashboardUrl(activeStatsServer.hostname, activeStatsServer.port);
|
||||
openUtils.openPath(url);
|
||||
|
||||
const serverLine = requestedPortIgnored
|
||||
? `Dashboard already running at: ${url} (requested port ${args.port} ignored)`
|
||||
const serverLine = requestedAddressIgnored
|
||||
? `Dashboard already running at: ${url} (requested ${args.host}:${args.port} ignored)`
|
||||
: `Dashboard available at: ${url}`;
|
||||
|
||||
return {
|
||||
|
||||
@@ -0,0 +1,467 @@
|
||||
/**
|
||||
* In-session fallbacks for permission-denied file writes and deletes.
|
||||
*
|
||||
* A host that embeds the agent inside an OS sandbox can grant a path mid-session
|
||||
* but cannot apply that grant to an in-process write, because the write happens
|
||||
* in the agent process under a profile fixed at launch. This module gives such a
|
||||
* host a seam to intercept a denied mutation and perform it through a privileged
|
||||
* channel, without reimplementing `write`/`edit` semantics: the native tool still
|
||||
* records its own snapshot under the real destination path once the fallback
|
||||
* reports success, so a follow-up hashline `edit` on that path keeps working.
|
||||
*
|
||||
* Writes and deletes have SEPARATE registries. A write handler brokers `content`
|
||||
* to `dst`, so a delete request reaching it with no content invites brokering an
|
||||
* empty write and truncating the file it was asked to remove. Opting into deletes
|
||||
* is therefore explicit; see {@link addFileDeleteFallback}.
|
||||
*
|
||||
* ## What is routed
|
||||
*
|
||||
* The byte-write that `write`, `edit` and `apply_patch` perform on an ordinary
|
||||
* file path goes through the same two-line primitive
|
||||
* (`file ? file.write(content) : Bun.write(dst, content)`). It has four call
|
||||
* sites, and all of them route here:
|
||||
*
|
||||
* - `writethroughNoop` and `runLspWritethrough`'s `writeContent` (`lsp/writethrough.ts`),
|
||||
* the `WritethroughCallback` that `write` and `edit` both write through.
|
||||
* `apply_patch` reaches it too: `LspFileSystem.write` (`edit/modes/patch.ts`),
|
||||
* which it always injects, delegates to the same callback.
|
||||
* - `HashlineFilesystem.move` (`edit/hashline/filesystem.ts`) — a hashline `MV`
|
||||
* destination, the one `edit` write that does not pass through the writethrough.
|
||||
* - `defaultFileSystem.write` (`edit/modes/patch.ts`), only the default parameter
|
||||
* for external `applyPatch` callers and tests.
|
||||
*
|
||||
* `apply_patch` also creates a missing parent directory before writing, via its
|
||||
* filesystem's `mkdir`. That `mkdir` consults {@link hasFileWriteFallback} so a
|
||||
* denial there falls through to the write and reaches a handler, instead of
|
||||
* throwing before the seam is ever consulted.
|
||||
*
|
||||
* The unlink that `edit` and `apply_patch` perform routes to the separate delete
|
||||
* seam ({@link deleteFileWithFallback}) at four sites: `HashlineFilesystem.delete`
|
||||
* (`edit`'s `REM`) and `HashlineFilesystem.move`'s source unlink, plus
|
||||
* `LspFileSystem.delete` and `defaultFileSystem.delete` for `apply_patch`.
|
||||
*
|
||||
* ## What is NOT routed
|
||||
*
|
||||
* This is deliberately not an exhaustive interception of every syscall the tools
|
||||
* can make. A permission error from any of these surfaces as it does today:
|
||||
*
|
||||
* - `write` to an archive member (`foo.zip:entry`) or a SQLite row. Neither is a
|
||||
* byte-write to `dst`: an archive member rewrite reads the whole archive, sets
|
||||
* one entry, writes a temp file and renames over the original, so the bytes on
|
||||
* disk are a whole binary container rather than the string the tool was given;
|
||||
* a SQLite write is a row operation inside the database engine with no byte
|
||||
* payload at all. Brokering either needs a different request shape than
|
||||
* "these exact bytes belong at this path".
|
||||
* - `acp-bridge.ts`'s `bridge.writeTextFile` — a remote-client transport.
|
||||
* - Removing a DIRECTORY is never the intent: the delete seam refuses to divert a
|
||||
* target it can confirm is one, and reports `confirmedFile: false` when the
|
||||
* target's metadata is behind the same boundary and the check cannot be resolved.
|
||||
* - The `lsp` tool's own writes: applying a workspace edit or a code action
|
||||
* (`lsp/edits.ts`), and the Biome formatter, which writes the buffer and then
|
||||
* shells out to `biome format --write` (`lsp/clients/biome-client.ts`) — a
|
||||
* subprocess write no in-process seam can reach anyway.
|
||||
*
|
||||
* ## Diverting
|
||||
*
|
||||
* Only a permission boundary diverts — `EPERM`, `EACCES`, `EROFS`, plus the one
|
||||
* case where Bun hides such a denial behind an `ENOENT` (see
|
||||
* {@link classifyWriteFailure}). Every other error rethrows untouched.
|
||||
*
|
||||
* With no handler registered this module is inert: the primitive runs exactly as
|
||||
* it did before, a failure rethrows from the same place, and no extra syscalls
|
||||
* are performed.
|
||||
*
|
||||
* ## The path a handler is given
|
||||
*
|
||||
* A handler is more privileged than the syscall that just failed, so it is never
|
||||
* handed the lexical path the tool used. A lexical path is not a destination: the
|
||||
* kernel follows every component above the last, so `ws/link/file` under a
|
||||
* `ws/link -> /elsewhere` link lands outside `ws` while still looking
|
||||
* in-workspace. That defeats the defence a helper author reaches for first, since
|
||||
* a prefix allowlist passes on the link's own path — and for writes the final
|
||||
* component is followed too, so a plain `ws/link` is enough.
|
||||
*
|
||||
* `req.dst` is therefore resolved through {@link resolveSyscallTarget} to the path
|
||||
* the failed syscall itself acted on: fully for a write, and up to the last
|
||||
* component for a delete, since `unlink` removes a link rather than following it.
|
||||
* Resolving rather than refusing also closes the TOCTOU window, because the
|
||||
* handler no longer traverses a link the agent could re-point after the check.
|
||||
*
|
||||
* A path that cannot be canonicalized — a dangling final link, or an ancestor
|
||||
* whose own resolution is denied — is not brokered at all. "Where would this
|
||||
* land" has no answer there, and a privileged writer is the wrong place to guess.
|
||||
* That narrows the seam for a sandbox that also hides the ancestors of a denied
|
||||
* path, which is the honest cost of not handing over an unverifiable target.
|
||||
*
|
||||
* That refusal is load-bearing for more than symlink safety, and relaxing it needs
|
||||
* care. `apply_patch`'s `create` and rename-destination refuse to overwrite, and
|
||||
* they decide that with `Bun.file(dst).exists()`, which reports `false` when the
|
||||
* parent hides the target's metadata rather than distinguishing "absent" from
|
||||
* "unknown". The non-overwrite contract holds today only because the same denied
|
||||
* `lstat` that fools that check also stops this seam from brokering — a privileged
|
||||
* writer, the one party that could enforce exclusivity itself, is never handed the
|
||||
* path. Broker an unverifiable destination and a `create` starts clobbering a
|
||||
* protected file it was told not to touch; a request field carrying explicit
|
||||
* exclusive-create intent would be the prerequisite for that change.
|
||||
*
|
||||
* ## Scope of the registry
|
||||
*
|
||||
* Handlers live in one process-wide list, and a process can host several sessions
|
||||
* (a subagent gets its own `ExtensionRunner`). A handler is therefore consulted
|
||||
* for denied mutations from ANY session in the process, not only the one whose
|
||||
* extension registered it. Filtering by session here would be wrong: a subagent
|
||||
* spawned with `restrictToolNames` loads no extensions of its own, so scoping
|
||||
* would leave its denied writes with nothing to broker them, and a host that
|
||||
* registers once in its top-level session expects subagent writes covered.
|
||||
*
|
||||
* So the request names its origin instead, and the policy stays with the party
|
||||
* that owns it. `req.sessionId` is the session that issued the mutation (see
|
||||
* {@link withFileMutationSession}); a handler compares it with
|
||||
* `ctx.sessionManager.getSessionId()` to decide. That matters most for a handler
|
||||
* that prompts: `ctx.ui` belongs to the session whose extension registered the
|
||||
* handler, which is not necessarily the session being asked about.
|
||||
*
|
||||
* Each list is iterated over a snapshot, because a concurrent session shutdown
|
||||
* splices the live array and a `for` over it would skip whichever handler shifted
|
||||
* into the hole.
|
||||
*/
|
||||
import { AsyncLocalStorage } from "node:async_hooks";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as path from "node:path";
|
||||
import { isEnoent, isFsError, logger } from "@oh-my-pi/pi-utils";
|
||||
import type { BunFile } from "bun";
|
||||
import type { ExtensionContext } from "../extensibility/extensions/types";
|
||||
import { resolveSyscallTarget } from "./path-utils";
|
||||
|
||||
/** A denied write, captured for a registered fallback to retry through a privileged channel. */
|
||||
export interface FileWriteFallbackRequest {
|
||||
/**
|
||||
* Absolute, symlink-resolved path to write the bytes to.
|
||||
*
|
||||
* This is where the failed in-process write would itself have landed, which is
|
||||
* not necessarily the path the tool was given: `open` follows every component,
|
||||
* so a link anywhere in that path redirects the bytes. Resolving it here is
|
||||
* what lets a handler's allowlist see the real destination instead of a
|
||||
* lexically innocent path, so a handler MUST treat this as authoritative and
|
||||
* MUST NOT re-derive the target from anything else.
|
||||
*/
|
||||
dst: string;
|
||||
/**
|
||||
* Session the denied write was issued from, or `undefined` when the mutation
|
||||
* did not happen inside a tool call (an external `applyPatch` caller, a test).
|
||||
*
|
||||
* The registry is process-wide, so a handler can be consulted for a write from
|
||||
* a session other than the one whose extension registered it. Compare this with
|
||||
* `ctx.sessionManager.getSessionId()` to tell the two apart — a handler that
|
||||
* prompts through `ctx.ui` needs to, since that UI belongs to ITS session and
|
||||
* not necessarily to the one being asked about.
|
||||
*/
|
||||
sessionId: string | undefined;
|
||||
/** The exact bytes the tool intended to write. */
|
||||
content: string;
|
||||
/**
|
||||
* The error that proves the write hit a permission boundary. Usually the write's
|
||||
* own `EPERM`/`EACCES`/`EROFS`; for a write into a directory the host may not
|
||||
* create, the denial raised by creating that directory, in which case `dst`'s
|
||||
* parent may not exist yet and the handler is responsible for creating it.
|
||||
*/
|
||||
cause: unknown;
|
||||
}
|
||||
|
||||
/** Extension-authored handler. Return `true` once `content` is durably on disk at `dst`. */
|
||||
export type FileWriteFallbackHandler = (req: FileWriteFallbackRequest, ctx: ExtensionContext) => Promise<boolean>;
|
||||
|
||||
/** A handler already bound to its owning extension's live context. */
|
||||
type BoundFileWriteFallbackHandler = (req: FileWriteFallbackRequest) => Promise<boolean>;
|
||||
|
||||
/** A denied unlink, captured for a registered fallback to perform through a privileged channel. */
|
||||
export interface FileDeleteFallbackRequest {
|
||||
/**
|
||||
* Absolute, symlink-resolved path the unlink was denied for.
|
||||
*
|
||||
* Every component ABOVE the last is resolved, so a handler cannot be walked
|
||||
* outside its allowed roots through a link in the path. The last component is
|
||||
* deliberately NOT resolved, because `unlink` removes a link itself rather
|
||||
* than its target — which is also why this may still name a symlink.
|
||||
*/
|
||||
dst: string;
|
||||
/** The `EPERM`/`EACCES`/`EROFS` that proves the unlink hit a permission boundary. */
|
||||
cause: unknown;
|
||||
/**
|
||||
* Whether `dst` was confirmed to be a plain regular file before diverting.
|
||||
*
|
||||
* `false` means the seam could not establish that, either because the target's
|
||||
* own metadata is behind the same boundary that denied the unlink — the common
|
||||
* sandbox case, since `unlink` on a directory also reports `EPERM` on Darwin —
|
||||
* or because `dst` is a symlink.
|
||||
*
|
||||
* A handler MUST remove `dst` with a plain unlink. It MUST NOT remove it
|
||||
* recursively, and MUST NOT resolve the path first: when this is `false` the
|
||||
* target may be a directory, and resolving a symlink would delete whatever it
|
||||
* points at instead of the link.
|
||||
*/
|
||||
confirmedFile: boolean;
|
||||
/** See {@link FileWriteFallbackRequest.sessionId}. */
|
||||
sessionId: string | undefined;
|
||||
}
|
||||
|
||||
/** Extension-authored handler. Return `true` once `dst` is gone from disk. */
|
||||
export type FileDeleteFallbackHandler = (req: FileDeleteFallbackRequest, ctx: ExtensionContext) => Promise<boolean>;
|
||||
|
||||
/** A handler already bound to its owning extension's live context. */
|
||||
type BoundFileDeleteFallbackHandler = (req: FileDeleteFallbackRequest) => Promise<boolean>;
|
||||
|
||||
const PERMISSION_DENIED_CODES: Record<string, true> = { EPERM: true, EACCES: true, EROFS: true };
|
||||
const PERMISSION_DENIED_MESSAGE = /\b(EPERM|EACCES|EROFS)\b/;
|
||||
|
||||
/** True for `EPERM`, `EACCES`, and `EROFS` — the sandbox-boundary write failures this seam exists for. */
|
||||
export function isPermissionDeniedError(error: unknown): boolean {
|
||||
// A structured `code` is authoritative. Checking the message as well would
|
||||
// misclassify any error whose path contains one of these names, and Bun embeds
|
||||
// the full path in its fs error messages (`ENOENT: ..., open '/x/EACCES/y'`).
|
||||
if (isFsError(error)) return PERMISSION_DENIED_CODES[error.code] === true;
|
||||
// Some write paths (e.g. a bridged transport) surface the denial as a plain
|
||||
// Error with no structured `code`, leaving only the message to go on.
|
||||
return error instanceof Error && PERMISSION_DENIED_MESSAGE.test(error.message);
|
||||
}
|
||||
|
||||
const fallbackHandlers: BoundFileWriteFallbackHandler[] = [];
|
||||
|
||||
/** Whether any fallback is registered. Lets a caller skip work that only this seam needs. */
|
||||
export function hasFileWriteFallback(): boolean {
|
||||
return fallbackHandlers.length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Append a fallback writer, consulted in registration order when a direct write is
|
||||
* permission-denied. Returns a disposer that removes this exact registration; the
|
||||
* runner calls it on session shutdown so no handler outlives its session.
|
||||
*/
|
||||
export function addFileWriteFallback(handler: BoundFileWriteFallbackHandler): () => void {
|
||||
fallbackHandlers.push(handler);
|
||||
return () => {
|
||||
const index = fallbackHandlers.indexOf(handler);
|
||||
if (index !== -1) fallbackHandlers.splice(index, 1);
|
||||
};
|
||||
}
|
||||
|
||||
const deleteFallbackHandlers: BoundFileDeleteFallbackHandler[] = [];
|
||||
|
||||
/** Whether any delete fallback is registered. */
|
||||
export function hasFileDeleteFallback(): boolean {
|
||||
return deleteFallbackHandlers.length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Append a fallback deleter, consulted in registration order when a direct unlink is
|
||||
* permission-denied. Deliberately a separate registry from
|
||||
* {@link addFileWriteFallback}: a write handler brokers `content` to `dst`, and
|
||||
* handing it a request with no content would let it "broker" an empty write and
|
||||
* truncate the file it was asked to remove. Opting in is explicit for that reason.
|
||||
*/
|
||||
export function addFileDeleteFallback(handler: BoundFileDeleteFallbackHandler): () => void {
|
||||
deleteFallbackHandlers.push(handler);
|
||||
return () => {
|
||||
const index = deleteFallbackHandlers.indexOf(handler);
|
||||
if (index !== -1) deleteFallbackHandlers.splice(index, 1);
|
||||
};
|
||||
}
|
||||
|
||||
const mutationSessionStorage = new AsyncLocalStorage<string>();
|
||||
|
||||
/**
|
||||
* Name the session whose tool call is about to run, so a denied mutation inside it
|
||||
* can tell a handler where the request came from.
|
||||
*
|
||||
* Entered once per tool call by `ExtensionToolWrapper` (`extensibility/extensions/
|
||||
* wrapper.ts`), which `sdk.ts` puts around the whole tool registry whenever an
|
||||
* `ExtensionRunner` exists — so the component that owns the handlers is the one
|
||||
* naming its own session, and no caller has to thread an `AgentToolContext`
|
||||
* through for attribution to work.
|
||||
*
|
||||
* That covers the deferred LSP write batch too: a batch id belongs to one
|
||||
* assistant turn of one session, and its flush is awaited inside a tool call of
|
||||
* that same session, so a write performed during a later call of the group is
|
||||
* still attributed to the session that issued it.
|
||||
*
|
||||
* Deliberately NOT a general "current session" accessor: nothing else enters this
|
||||
* scope, so outside a tool call it is empty by design — an external `applyPatch`
|
||||
* caller reports `undefined` rather than borrowing someone else's identity.
|
||||
*/
|
||||
export function withFileMutationSession<T>(sessionId: string | undefined, fn: () => T): T {
|
||||
// With nothing registered no scope is entered, keeping the seam's inertness
|
||||
// promise: a stock host pays one length check per tool call and no more.
|
||||
if (sessionId === undefined || (fallbackHandlers.length === 0 && deleteFallbackHandlers.length === 0)) return fn();
|
||||
return mutationSessionStorage.run(sessionId, fn);
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a file, consulting registered delete fallbacks when the unlink is denied.
|
||||
*
|
||||
* Unlike the write path there is no masked-`ENOENT` case to see through: nothing is
|
||||
* created on the way, so an `ENOENT` here means the file genuinely is not there and
|
||||
* must propagate — `edit`'s `REM` turns it into a `NotFoundError`.
|
||||
*/
|
||||
export async function deleteFileWithFallback(dst: string, file?: BunFile): Promise<void> {
|
||||
try {
|
||||
if (file) {
|
||||
await file.unlink();
|
||||
} else {
|
||||
await fs.unlink(dst);
|
||||
}
|
||||
} catch (error) {
|
||||
if (deleteFallbackHandlers.length === 0 || !isPermissionDeniedError(error)) throw error;
|
||||
// A handler is more privileged than the unlink that just failed, so it is told
|
||||
// which path really gets removed, not the lexical one the tool used. `unlink`
|
||||
// follows every component ABOVE the last, so `ws/link/victim` under a
|
||||
// `ws/link -> /elsewhere` link removes a file outside `ws` while a helper's
|
||||
// prefix allowlist still passes. The final component is deliberately left
|
||||
// unresolved: `unlink` removes the link itself, never its target.
|
||||
const target = await resolveSyscallTarget(dst, false);
|
||||
if (target === null) throw error;
|
||||
// `unlink` on a directory reports EPERM on Darwin (EISDIR on Linux), which is
|
||||
// indistinguishable from a sandbox denial by code alone, so check the target
|
||||
// before diverting: asking a privileged deleter to remove a DIRECTORY on
|
||||
// behalf of a tool that only ever removes one file would far exceed the
|
||||
// intent. `lstat` rather than `stat`, so the link itself is judged — removing
|
||||
// a symlink is a legitimate file removal, and following it here would ask the
|
||||
// wrong question.
|
||||
const stat = await fs.lstat(target).catch((statError: unknown) => {
|
||||
// A sandbox that denies the unlink usually denies the target's metadata
|
||||
// too, so a denied `lstat` is expected here and must still divert — it
|
||||
// just leaves the question unresolved, which `confirmedFile` reports.
|
||||
// Any OTHER `lstat` failure is not something this seam should paper over.
|
||||
if (isPermissionDeniedError(statError)) return null;
|
||||
throw error;
|
||||
});
|
||||
if (stat?.isDirectory()) throw error;
|
||||
// A symlink is safe to unlink but NOT safe to resolve: a helper that
|
||||
// realpaths `dst` for auditing, or removes it recursively, would act on the
|
||||
// link's target instead. Only a plain regular file is a confirmed file.
|
||||
const confirmedFile = stat?.isFile() ?? false;
|
||||
// The process-wide registry can hand this to a handler from another session,
|
||||
// so the request names the one that issued it.
|
||||
const sessionId = mutationSessionStorage.getStore();
|
||||
// Snapshot: a concurrent session shutdown splices the live array, and
|
||||
// iterating it directly would skip whichever handler shifted into the hole.
|
||||
for (const handler of [...deleteFallbackHandlers]) {
|
||||
try {
|
||||
if (await handler({ dst: target, cause: error, confirmedFile, sessionId })) return;
|
||||
} catch (handlerError) {
|
||||
logger.warn("File delete fallback handler threw; trying next handler", {
|
||||
dst: target,
|
||||
error: handlerError instanceof Error ? handlerError.message : String(handlerError),
|
||||
});
|
||||
}
|
||||
}
|
||||
// Always the ORIGINAL error, never a handler's, so behaviour matches a host
|
||||
// with no fallback registered.
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Outcome of inspecting a failed primitive write. `denied` diverts to the
|
||||
* registered handlers, `retry` repeats the write because this call repaired the
|
||||
* cause, and `rethrow` leaves the original error alone.
|
||||
*/
|
||||
type WriteFailureKind = { kind: "denied"; cause: unknown } | { kind: "retry" } | { kind: "rethrow" };
|
||||
|
||||
/**
|
||||
* Decide whether a failed write hit a permission boundary.
|
||||
*
|
||||
* `Bun.write` and `BunFile.write` create missing parent directories themselves,
|
||||
* but when that `mkdir` is the thing being denied they report the subsequent
|
||||
* `open()`'s `ENOENT` rather than the denial — so a sandboxed write into a new
|
||||
* out-of-tree directory is indistinguishable from an ordinary missing path.
|
||||
* Redoing the `mkdir` explicitly recovers the real errno, and because it runs
|
||||
* through the same enforcement path as the write it sees kernel-level denials
|
||||
* (Seatbelt, LSM) that a `stat`/`access` probe would report as writable.
|
||||
*
|
||||
* Only called with at least one handler registered, so a stock host never pays
|
||||
* for this.
|
||||
*/
|
||||
async function classifyWriteFailure(dst: string, error: unknown): Promise<WriteFailureKind> {
|
||||
if (isPermissionDeniedError(error)) return { kind: "denied", cause: error };
|
||||
if (!isEnoent(error)) return { kind: "rethrow" };
|
||||
try {
|
||||
await fs.mkdir(path.dirname(dst), { recursive: true });
|
||||
} catch (mkdirError) {
|
||||
// A denied `mkdir` is the boundary the write hid; anything else (`ENOTDIR`
|
||||
// for a file used as a directory, ...) is a genuine bad path.
|
||||
if (isPermissionDeniedError(mkdirError)) return { kind: "denied", cause: mkdirError };
|
||||
return { kind: "rethrow" };
|
||||
}
|
||||
// The parent exists now, so the `ENOENT` was a lost race rather than a
|
||||
// boundary. Any directory just created stays, matching what a permitted
|
||||
// `Bun.write` would have left behind; removing it could race a concurrent
|
||||
// writer that legitimately needs it.
|
||||
return { kind: "retry" };
|
||||
}
|
||||
|
||||
export async function writeFileWithFallback(dst: string, content: string, file?: BunFile): Promise<void> {
|
||||
// Attempt 0 is the plain write. The single retry is reachable only when the
|
||||
// first failure turned out to be a parent-directory race this call repaired,
|
||||
// which bounds the loop at two writes.
|
||||
for (let attempt = 0; ; attempt++) {
|
||||
try {
|
||||
if (file) {
|
||||
await file.write(content);
|
||||
} else {
|
||||
await Bun.write(dst, content);
|
||||
}
|
||||
return;
|
||||
} catch (error) {
|
||||
if (fallbackHandlers.length === 0) throw error;
|
||||
// On the second attempt a `retry` verdict can no longer change the
|
||||
// outcome, so skip the probe and let the error stand unless it is a
|
||||
// denial the handlers should see.
|
||||
const failure =
|
||||
attempt === 0
|
||||
? await classifyWriteFailure(dst, error)
|
||||
: isPermissionDeniedError(error)
|
||||
? ({ kind: "denied", cause: error } as const)
|
||||
: ({ kind: "rethrow" } as const);
|
||||
if (failure.kind === "retry") continue;
|
||||
if (failure.kind === "denied") {
|
||||
// A handler is more privileged than the write that just failed, so it is
|
||||
// told where the bytes would REALLY have landed rather than the lexical
|
||||
// path the tool used. `open` follows EVERY component, so `ws/link/file`
|
||||
// under a `ws/link -> /elsewhere` link writes outside `ws` while still
|
||||
// looking in-workspace — which defeats the defence a helper author
|
||||
// reaches for first, since a prefix allowlist passes on the link's own
|
||||
// path. Resolving closes that, and closes the TOCTOU window with it: the
|
||||
// helper no longer traverses a link the agent could re-point after the
|
||||
// check. A path that cannot be canonicalized is not brokered at all,
|
||||
// because "where would this land" then has no answer to hand over.
|
||||
const target = await resolveSyscallTarget(dst, true);
|
||||
// Snapshot: a concurrent session shutdown splices the live array, and
|
||||
// iterating it directly would skip whichever handler shifted into the hole.
|
||||
if (target !== null) {
|
||||
// The process-wide registry can hand this to a handler from another
|
||||
// session, so the request names the one that issued it.
|
||||
const sessionId = mutationSessionStorage.getStore();
|
||||
for (const handler of [...fallbackHandlers]) {
|
||||
try {
|
||||
if (await handler({ dst: target, content, cause: failure.cause, sessionId })) return;
|
||||
} catch (handlerError) {
|
||||
logger.warn("File write fallback handler threw; trying next handler", {
|
||||
dst: target,
|
||||
error: handlerError instanceof Error ? handlerError.message : String(handlerError),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Always the ORIGINAL error, never a handler's, so behaviour matches a
|
||||
// host with no fallback registered. When the real boundary was recovered
|
||||
// from behind a masked `ENOENT`, attach it so the denial is not lost:
|
||||
// without this the caller is told `ENOENT` for a path this code has
|
||||
// already proven is `EACCES`.
|
||||
if (failure.kind === "denied" && failure.cause !== error && error instanceof Error && error.cause == null) {
|
||||
error.cause = failure.cause;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -86,6 +86,7 @@ export * from "./debug";
|
||||
export * from "./essential-tools";
|
||||
export * from "./eval";
|
||||
export * from "./eval-backends";
|
||||
export * from "./file-write-fallback";
|
||||
export * from "./gh";
|
||||
export * from "./glob";
|
||||
export * from "./grep";
|
||||
|
||||
@@ -618,6 +618,85 @@ function isSymlink(target: string): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the path a syscall on `filePath` would really act on, or `null` when
|
||||
* that cannot be established.
|
||||
*
|
||||
* A lexical path is not a destination. The kernel follows every component above
|
||||
* the last, so `ws/link/file` under a `ws/link -> /elsewhere` link lands outside
|
||||
* `ws` while still looking relative and `..`-free. Handing such a path to a
|
||||
* privileged helper defeats the defence a helper author reaches for first — a
|
||||
* prefix allowlist passes, because the link sits inside the allowed root while
|
||||
* its target does not. Callers that hand a path to something more privileged
|
||||
* than the syscall that just failed resolve it here first.
|
||||
*
|
||||
* Rejecting symlinked components outright is not an option: `/var` and `/tmp`
|
||||
* are links on macOS, so every path under `os.tmpdir()` traverses one. They are
|
||||
* resolved instead, and only a path whose real destination cannot be established
|
||||
* is refused, because "where would this land" then has no answer to hand over.
|
||||
* {@link confineToWorkspace} refuses an unresolvable link for the same reason.
|
||||
*
|
||||
* @param followFinal `true` for a syscall that follows a link at the final
|
||||
* component (`open`, so every write), `false` for one that acts on the link
|
||||
* itself (`unlink`) and therefore needs it left alone.
|
||||
*/
|
||||
export async function resolveSyscallTarget(filePath: string, followFinal: boolean): Promise<string | null> {
|
||||
const target = path.resolve(filePath);
|
||||
if (followFinal) {
|
||||
const real = await tryRealpathAsync(target);
|
||||
if (real !== null) return real;
|
||||
// `realpath` also fails on a DANGLING link, which a write follows to a place
|
||||
// this cannot name, and on a path whose ancestor may not be searched. Neither
|
||||
// is proof the final component is a plain name, and only proof continues.
|
||||
if (!(await isProvenNotSymlink(target))) return null;
|
||||
}
|
||||
// Walk up to the deepest ancestor that does resolve, then re-apply the
|
||||
// components below it. A resolved ancestor vouches for the ones above it, so
|
||||
// re-applying them lexically matches what the kernel would have done.
|
||||
const tail: string[] = [path.basename(target)];
|
||||
let ancestor = path.dirname(target);
|
||||
for (;;) {
|
||||
const real = await tryRealpathAsync(ancestor);
|
||||
if (real !== null) return path.join(real, ...tail.reverse());
|
||||
// This component is about to be re-applied lexically without a resolved
|
||||
// ancestor vouching for it, which is exactly the escape being closed — so it
|
||||
// has to prove itself. `realpath` fails here for a component that does not
|
||||
// exist yet AND for one inside a directory the caller may not search (the
|
||||
// usual shape when a sandbox hides a denied path), and the second still
|
||||
// permits `lstat`.
|
||||
if (!(await isProvenNotSymlink(ancestor))) return null;
|
||||
const parent = path.dirname(ancestor);
|
||||
// Ran past the filesystem root: `realpath("/")` cannot fail, so only a
|
||||
// filesystem disappearing mid-walk gets here.
|
||||
if (parent === ancestor) return null;
|
||||
tail.push(path.basename(ancestor));
|
||||
ancestor = parent;
|
||||
}
|
||||
}
|
||||
|
||||
async function tryRealpathAsync(target: string): Promise<string | null> {
|
||||
try {
|
||||
// `fs.promises.realpath` has no `.native` variant under Bun, unlike its sync
|
||||
// counterpart; the JS implementation resolves links identically.
|
||||
return await fs.promises.realpath(target);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `target` is known NOT to redirect. A path that does not exist cannot
|
||||
* redirect anything, and nothing below it exists either; any other `lstat`
|
||||
* failure leaves the question unanswered, which is not proof.
|
||||
*/
|
||||
async function isProvenNotSymlink(target: string): Promise<boolean> {
|
||||
try {
|
||||
return !(await fs.promises.lstat(target)).isSymbolicLink();
|
||||
} catch (error) {
|
||||
return isEnoent(error);
|
||||
}
|
||||
}
|
||||
|
||||
export function formatPathRelativeToCwd(
|
||||
filePath: string,
|
||||
cwd: string,
|
||||
|
||||
@@ -3739,6 +3739,8 @@ describe("ExtensionRunner", () => {
|
||||
handlers: new Map([["input", [async (...args: unknown[]) => handler(args[0] as InputEvent)]]]),
|
||||
tools: new Map(),
|
||||
assistantThinkingRenderers: [],
|
||||
fileWriteFallbackHandlers: [],
|
||||
fileDeleteFallbackHandlers: [],
|
||||
messageRenderers: new Map(),
|
||||
commands: new Map(),
|
||||
flags: new Map(),
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, expect, spyOn, test } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { RpcClient } from "@oh-my-pi/pi-coding-agent/modes/rpc/rpc-client";
|
||||
import { ptree, TempDir } from "@oh-my-pi/pi-utils";
|
||||
import { type ChildProcess, ptree, TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
const MOCK_AGENT = path.join(import.meta.dir, "fixtures", "mock-rpc-agent.ts");
|
||||
|
||||
@@ -243,4 +243,41 @@ describe("RpcClient lifecycle (issue #4079 B)", () => {
|
||||
"Agent process exited with code 23. Stderr: fixture worker failed",
|
||||
);
|
||||
});
|
||||
|
||||
test("start() rejects instead of hanging when a pre-ready worker closes stdout and never exits", async () => {
|
||||
// The worker outlives its own stdout, so start() cannot learn an exit code
|
||||
// and must still fail: it waits a bounded time for the exit, then reports
|
||||
// the stream end. A regression stalls until the 30s ready timeout, which
|
||||
// this test's own timeout catches.
|
||||
let resolveExit: ((exitCode: number) => void) | undefined;
|
||||
let killCalls = 0;
|
||||
const fakeChild = {
|
||||
stdout: new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
controller.close();
|
||||
},
|
||||
}),
|
||||
stdin: { write: () => 0, flush: () => 0 },
|
||||
exited: new Promise<number>(resolve => {
|
||||
resolveExit = resolve;
|
||||
}),
|
||||
peekStderr: () => "worker went quiet",
|
||||
kill() {
|
||||
killCalls += 1;
|
||||
resolveExit?.(0);
|
||||
},
|
||||
};
|
||||
const spawn = spyOn(ptree, "spawn").mockImplementation(() => fakeChild as unknown as ChildProcess);
|
||||
|
||||
try {
|
||||
using client = new RpcClient({ cliPath: MOCK_AGENT, terminationGraceMs: 10 });
|
||||
await expect(client.start()).rejects.toThrow(
|
||||
"Agent output stream ended before ready. Stderr: worker went quiet",
|
||||
);
|
||||
// The failed start must also reap the orphan rather than leak it.
|
||||
expect(killCalls).toBe(1);
|
||||
} finally {
|
||||
spawn.mockRestore();
|
||||
}
|
||||
}, 5_000);
|
||||
});
|
||||
|
||||
@@ -506,6 +506,8 @@ describe("createAgentSession credential_disabled subscription", () => {
|
||||
]),
|
||||
tools: new Map(),
|
||||
assistantThinkingRenderers: [],
|
||||
fileWriteFallbackHandlers: [],
|
||||
fileDeleteFallbackHandlers: [],
|
||||
messageRenderers: new Map(),
|
||||
commands: new Map(),
|
||||
flags: new Map(),
|
||||
|
||||
@@ -0,0 +1,586 @@
|
||||
/**
|
||||
* End-to-end proof of the `registerFileWriteFallback` seam: a REAL extension,
|
||||
* loaded through the REAL loader/runner pipeline, registered on a REAL
|
||||
* `createAgentSession` session, intercepting a REAL EACCES raised by the
|
||||
* kernel for a genuinely-unwritable destination — not a fake resolver
|
||||
* standing in for the extension path.
|
||||
*
|
||||
* Permission denial is simulated without a sandbox, since real permission
|
||||
* bits behave differently depending on whether the destination already
|
||||
* exists:
|
||||
* - `write` targets a NEW file inside a directory chmod'd `0o500` (no write
|
||||
* bit). Creating a file needs write permission on the *directory*, so this
|
||||
* raises a real EACCES on `Bun.write`.
|
||||
* - `edit` overwrites an EXISTING file chmod'd `0o400` (no write bit). Bun
|
||||
* opens the existing inode directly for the rewrite, so a locked
|
||||
* *directory* alone does NOT block it (verified empirically) — only a
|
||||
* locked *file* does.
|
||||
*/
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, spyOn } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import type { AgentTool } from "@oh-my-pi/pi-agent-core";
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import type {
|
||||
ExtensionActions,
|
||||
ExtensionContextActions,
|
||||
ExtensionFactory,
|
||||
ExtensionRunner,
|
||||
} from "@oh-my-pi/pi-coding-agent/extensibility/extensions";
|
||||
import { type CreateAgentSessionOptions, createAgentSession, discoverAuthStorage } from "@oh-my-pi/pi-coding-agent/sdk";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import type { FileWriteFallbackRequest } from "@oh-my-pi/pi-coding-agent/tools/file-write-fallback";
|
||||
import { removeSyncWithRetries, Snowflake } from "@oh-my-pi/pi-utils";
|
||||
|
||||
/**
|
||||
* Drives `ExtensionRunner.initialize` with no-op stubs, mirroring what a mode
|
||||
* controller (interactive/RPC/ACP/print/subagent) does after
|
||||
* `createAgentSession` returns. Without this, `registerFileWriteFallback`
|
||||
* handlers never install: `ExtensionRunner` binds them to a live `ctx` inside
|
||||
* `initialize`, not at extension-load time.
|
||||
*/
|
||||
function initializeRunnerForTest(runner: ExtensionRunner | undefined): void {
|
||||
if (!runner) return;
|
||||
const actions: ExtensionActions = {
|
||||
sendMessage: () => {},
|
||||
sendUserMessage: () => {},
|
||||
appendEntry: () => {},
|
||||
setLabel: () => {},
|
||||
getActiveTools: () => [],
|
||||
getAllTools: () => [],
|
||||
setActiveTools: async () => {},
|
||||
getCommands: () => [],
|
||||
setModel: async () => false,
|
||||
getThinkingLevel: () => undefined,
|
||||
setThinkingLevel: () => {},
|
||||
getSessionName: () => undefined,
|
||||
setSessionName: async () => {},
|
||||
};
|
||||
const contextActions: ExtensionContextActions = {
|
||||
getModel: () => undefined,
|
||||
isIdle: () => true,
|
||||
abort: () => {},
|
||||
hasPendingMessages: () => false,
|
||||
shutdown: () => {},
|
||||
getContextUsage: () => undefined,
|
||||
compact: async () => {},
|
||||
getSystemPrompt: () => [],
|
||||
};
|
||||
runner.initialize(actions, contextActions);
|
||||
}
|
||||
|
||||
function resultText(result: { content: Array<{ type: string; text?: string }> }): string {
|
||||
return result.content
|
||||
.filter((b): b is { type: "text"; text: string } => b.type === "text" && typeof b.text === "string")
|
||||
.map(b => b.text)
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
const HASHLINE_HEADER_LINE = /^\[([^#\r\n]+)#([0-9A-F]{4})\]$/;
|
||||
|
||||
describe("registerFileWriteFallback end-to-end (real extension, real session)", () => {
|
||||
const tempDirs: string[] = [];
|
||||
const lockedDirs: string[] = [];
|
||||
let modelRegistry!: ModelRegistry;
|
||||
let registryAuthDir: string;
|
||||
|
||||
const makeTempDir = (): string => {
|
||||
const created = path.join(os.tmpdir(), `pi-file-write-fallback-e2e-${Snowflake.next()}`);
|
||||
fs.mkdirSync(created, { recursive: true });
|
||||
// The seam brokers a symlink-RESOLVED path, and `os.tmpdir()` sits under `/var`
|
||||
// — itself a link — on macOS. Canonicalizing the fixture up front keeps a
|
||||
// handler's `req.dst` comparable to the path a test built.
|
||||
const tempDir = fs.realpathSync.native(created);
|
||||
tempDirs.push(tempDir);
|
||||
return tempDir;
|
||||
};
|
||||
|
||||
/** Tighten a mode and register it for restoration in `afterEach`, not in the test body. */
|
||||
const lock = (target: string, mode: number): void => {
|
||||
lockedDirs.push(target);
|
||||
fs.chmodSync(target, mode);
|
||||
};
|
||||
|
||||
// Mode bits do not constrain a privileged user, so `chmod` denies nothing as root
|
||||
// and every expectation that depends on a real denial would fail for a reason
|
||||
// unrelated to the seam. Root is real for a Docker-based local run and for a
|
||||
// self-hosted CI runner. `getuid` is undefined on Windows, where these modes are
|
||||
// not enforced either.
|
||||
const itDenied = it.skipIf(process.platform === "win32" || process.getuid?.() === 0);
|
||||
|
||||
const baseOptions = (tempDir: string, extensions: ExtensionFactory[]): CreateAgentSessionOptions => ({
|
||||
cwd: tempDir,
|
||||
agentDir: tempDir,
|
||||
modelRegistry,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings: Settings.isolated(),
|
||||
model: getBundledModel("openai", "gpt-4o-mini"),
|
||||
disableExtensionDiscovery: true,
|
||||
extensions,
|
||||
skills: [],
|
||||
contextFiles: [],
|
||||
promptTemplates: [],
|
||||
slashCommands: [],
|
||||
enableMCP: false,
|
||||
enableLsp: false,
|
||||
rules: [],
|
||||
workspaceTree: { rootPath: tempDir, rendered: "", truncated: false, totalLines: 0, agentsMdFiles: [] },
|
||||
});
|
||||
|
||||
beforeAll(async () => {
|
||||
registryAuthDir = path.join(os.tmpdir(), `pi-file-write-fallback-e2e-auth-${Snowflake.next()}`);
|
||||
fs.mkdirSync(registryAuthDir, { recursive: true });
|
||||
modelRegistry = new ModelRegistry(await discoverAuthStorage(registryAuthDir));
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
removeSyncWithRetries(registryAuthDir);
|
||||
});
|
||||
|
||||
// Restore every mode this file tightened BEFORE removing the trees. A test that
|
||||
// throws before its own `finally` would otherwise leave a 0o500 directory behind,
|
||||
// and `removeSyncWithRetries` only retries on Windows — on macOS/Linux it throws
|
||||
// EACCES, aborting this loop after `splice(0)` already emptied the list, which
|
||||
// strands every remaining temp dir for the rest of the run.
|
||||
afterEach(() => {
|
||||
for (const dir of lockedDirs.splice(0)) {
|
||||
try {
|
||||
fs.chmodSync(dir, 0o700);
|
||||
} catch {
|
||||
// Already gone, or never created: nothing to restore.
|
||||
}
|
||||
}
|
||||
for (const tempDir of tempDirs.splice(0)) {
|
||||
removeSyncWithRetries(tempDir);
|
||||
}
|
||||
});
|
||||
|
||||
itDenied(
|
||||
"write: a permission-denied create succeeds through a registered fallback, and a follow-up hashline edit on the real path works",
|
||||
async () => {
|
||||
const tempDir = makeTempDir();
|
||||
const lockedDir = path.join(tempDir, "locked-write");
|
||||
fs.mkdirSync(lockedDir, { recursive: true });
|
||||
lock(lockedDir, 0o500); // no write bit: creating a file here needs dir-write
|
||||
|
||||
const received: FileWriteFallbackRequest[] = [];
|
||||
const ownSessionIds: string[] = [];
|
||||
const factory: ExtensionFactory = pi => {
|
||||
pi.registerFileWriteFallback(async (req, ctx) => {
|
||||
received.push(req);
|
||||
ownSessionIds.push(ctx.sessionManager.getSessionId());
|
||||
// Stand-in for an out-of-process privileged broker: this test's own
|
||||
// user cannot write into `lockedDir`, so relax the permission bit
|
||||
// just long enough to place the exact bytes the tool intended, then
|
||||
// restore it — proving the handler alone determined success, not
|
||||
// some ambient permission the tool already had.
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
try {
|
||||
fs.writeFileSync(req.dst, req.content);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o500);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, [factory]));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
try {
|
||||
const writeTool = session.getToolByName("write") as AgentTool | undefined;
|
||||
expect(writeTool).toBeDefined();
|
||||
|
||||
const targetPath = path.join(lockedDir, "new-file.txt");
|
||||
const content = "export const value = 42;\n";
|
||||
|
||||
const writeResult = await writeTool!.execute("call-write-1", { path: targetPath, content });
|
||||
|
||||
// (i) the tool call succeeds
|
||||
expect(writeResult.isError).not.toBe(true);
|
||||
// (ii) the fallback received the exact intended bytes and the real destination path
|
||||
expect(received).toHaveLength(1);
|
||||
expect(received[0]?.dst).toBe(targetPath);
|
||||
expect(received[0]?.content).toBe(content);
|
||||
// (ii-b) and it can tell WHOSE write it was: the registry is process-wide, so
|
||||
// the request names the issuing session and `ctx` names the handler's own.
|
||||
// Both defined and equal here, which only holds if the tool-execution scope
|
||||
// that carries the session id is actually entered.
|
||||
expect(ownSessionIds[0]).toMatch(/./);
|
||||
expect(received[0]?.sessionId).toBe(ownSessionIds[0]);
|
||||
expect(fs.readFileSync(targetPath, "utf8")).toBe(content);
|
||||
|
||||
const headerLine = resultText(writeResult).split("\n")[0] ?? "";
|
||||
expect(HASHLINE_HEADER_LINE.test(headerLine)).toBe(true);
|
||||
|
||||
// (iii) a subsequent hashline edit on the SAME real path works — this only
|
||||
// holds if the write tool recorded its snapshot under `targetPath` itself
|
||||
// (not a temp path the fallback happened to route through).
|
||||
const editTool = session.getToolByName("edit") as AgentTool | undefined;
|
||||
expect(editTool).toBeDefined();
|
||||
const editInput = `${headerLine}\nPUT 1-1:\n+export const value = 43;\n`;
|
||||
const editResult = await editTool!.execute("call-edit-1", { input: editInput });
|
||||
|
||||
expect(editResult.isError).not.toBe(true);
|
||||
expect(fs.readFileSync(targetPath, "utf8")).toBe("export const value = 43;\n");
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
await session.dispose();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
itDenied(
|
||||
"edit: a permission-denied overwrite of an existing file succeeds through a registered fallback",
|
||||
async () => {
|
||||
const tempDir = makeTempDir();
|
||||
const targetPath = path.join(tempDir, "existing.txt");
|
||||
const originalContent = "export const enabled = false;\n";
|
||||
fs.writeFileSync(targetPath, originalContent);
|
||||
lock(targetPath, 0o400); // no write bit on the file itself
|
||||
|
||||
const received: FileWriteFallbackRequest[] = [];
|
||||
const factory: ExtensionFactory = pi => {
|
||||
pi.registerFileWriteFallback(async req => {
|
||||
received.push(req);
|
||||
fs.chmodSync(targetPath, 0o600);
|
||||
try {
|
||||
fs.writeFileSync(req.dst, req.content);
|
||||
} finally {
|
||||
fs.chmodSync(targetPath, 0o400);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, [factory]));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
try {
|
||||
// A prior `read` is required to seed the file-snapshot tag the hashline
|
||||
// edit addresses, mirroring how the model would discover an existing
|
||||
// file's current tag before patching it.
|
||||
const readTool = session.getToolByName("read") as AgentTool | undefined;
|
||||
expect(readTool).toBeDefined();
|
||||
const readResult = await readTool!.execute("call-read-1", { path: targetPath });
|
||||
const readHeaderLine = resultText(readResult).split("\n")[0] ?? "";
|
||||
expect(HASHLINE_HEADER_LINE.test(readHeaderLine)).toBe(true);
|
||||
|
||||
const editTool = session.getToolByName("edit") as AgentTool | undefined;
|
||||
expect(editTool).toBeDefined();
|
||||
const editInput = `${readHeaderLine}\nPUT 1-1:\n+export const enabled = true;\n`;
|
||||
const editResult = await editTool!.execute("call-edit-2", { input: editInput });
|
||||
|
||||
expect(editResult.isError).not.toBe(true);
|
||||
expect(received).toHaveLength(1);
|
||||
expect(received[0]?.dst).toBe(targetPath);
|
||||
expect(received[0]?.content).toBe("export const enabled = true;\n");
|
||||
expect(fs.readFileSync(targetPath, "utf8")).toBe("export const enabled = true;\n");
|
||||
} finally {
|
||||
fs.chmodSync(targetPath, 0o600);
|
||||
await session.dispose();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
itDenied("edit: a hashline MV into an unwritable directory succeeds through a registered fallback", async () => {
|
||||
// `MV` is the one `edit` write that never passes through the LSP writethrough
|
||||
// (`HashlineFilesystem.move` writes the destination directly), so it needs its
|
||||
// own end-to-end proof that the seam covers it.
|
||||
const tempDir = makeTempDir();
|
||||
const sourcePath = path.join(tempDir, "source.txt");
|
||||
fs.writeFileSync(sourcePath, "export const stage = 1;\n");
|
||||
const lockedDir = path.join(tempDir, "locked-move");
|
||||
fs.mkdirSync(lockedDir, { recursive: true });
|
||||
lock(lockedDir, 0o500);
|
||||
const destPath = path.join(lockedDir, "moved.txt");
|
||||
|
||||
const received: FileWriteFallbackRequest[] = [];
|
||||
const factory: ExtensionFactory = pi => {
|
||||
pi.registerFileWriteFallback(async req => {
|
||||
received.push(req);
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
try {
|
||||
fs.writeFileSync(req.dst, req.content);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o500);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, [factory]));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
try {
|
||||
const readTool = session.getToolByName("read") as AgentTool | undefined;
|
||||
expect(readTool).toBeDefined();
|
||||
const readResult = await readTool!.execute("call-read-mv", { path: sourcePath });
|
||||
const readHeaderLine = resultText(readResult).split("\n")[0] ?? "";
|
||||
expect(HASHLINE_HEADER_LINE.test(readHeaderLine)).toBe(true);
|
||||
|
||||
const editTool = session.getToolByName("edit") as AgentTool | undefined;
|
||||
expect(editTool).toBeDefined();
|
||||
const editInput = [readHeaderLine, "PUT 1-1:", "+export const stage = 2;", `MV ${destPath}`, ""].join("\n");
|
||||
const editResult = await editTool!.execute("call-edit-mv", { input: editInput });
|
||||
|
||||
expect(editResult.isError).not.toBe(true);
|
||||
expect(received).toHaveLength(1);
|
||||
expect(received[0]?.dst).toBe(destPath);
|
||||
expect(received[0]?.content).toBe("export const stage = 2;\n");
|
||||
expect(fs.readFileSync(destPath, "utf8")).toBe("export const stage = 2;\n");
|
||||
// `move` unlinks the source after the destination lands.
|
||||
expect(fs.existsSync(sourcePath)).toBe(false);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
await session.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it("edit: a hashline MV with no handler registered behaves exactly as before", async () => {
|
||||
// Guards the seam's inertness claim on the one site that now reaches it
|
||||
// outside the writethrough: with nothing registered, a plain MV must still
|
||||
// move the file and this package has no other coverage for that path.
|
||||
const tempDir = makeTempDir();
|
||||
const sourcePath = path.join(tempDir, "plain-source.txt");
|
||||
fs.writeFileSync(sourcePath, "export const stage = 1;\n");
|
||||
const destPath = path.join(tempDir, "nested", "plain-dest.txt");
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, []));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
try {
|
||||
const readTool = session.getToolByName("read") as AgentTool | undefined;
|
||||
const readResult = await readTool!.execute("call-read-plain", { path: sourcePath });
|
||||
const readHeaderLine = resultText(readResult).split("\n")[0] ?? "";
|
||||
expect(HASHLINE_HEADER_LINE.test(readHeaderLine)).toBe(true);
|
||||
|
||||
const editTool = session.getToolByName("edit") as AgentTool | undefined;
|
||||
const editInput = [readHeaderLine, "PUT 1-1:", "+export const stage = 2;", `MV ${destPath}`, ""].join("\n");
|
||||
const editResult = await editTool!.execute("call-edit-plain-mv", { input: editInput });
|
||||
|
||||
expect(editResult.isError).not.toBe(true);
|
||||
expect(fs.readFileSync(destPath, "utf8")).toBe("export const stage = 2;\n");
|
||||
expect(fs.existsSync(sourcePath)).toBe(false);
|
||||
} finally {
|
||||
await session.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
itDenied("edit: a permission-denied REM succeeds through a registered delete fallback", async () => {
|
||||
// `REM` unlinks the file, which is a different primitive from the byte-write and
|
||||
// has its own seam. A write fallback must NOT be consulted for it: a write
|
||||
// handler brokers `content` to `dst`, so a delete arriving there would truncate
|
||||
// the file instead of removing it.
|
||||
const tempDir = makeTempDir();
|
||||
const lockedDir = path.join(tempDir, "locked-rem");
|
||||
fs.mkdirSync(lockedDir, { recursive: true });
|
||||
const targetPath = path.join(lockedDir, "doomed.txt");
|
||||
fs.writeFileSync(targetPath, "export const stage = 1;\n");
|
||||
lock(lockedDir, 0o500);
|
||||
|
||||
const deleted: string[] = [];
|
||||
const writeCalls: string[] = [];
|
||||
const factory: ExtensionFactory = pi => {
|
||||
pi.registerFileWriteFallback(async req => {
|
||||
writeCalls.push(req.dst);
|
||||
return false;
|
||||
});
|
||||
pi.registerFileDeleteFallback(async req => {
|
||||
deleted.push(req.dst);
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
try {
|
||||
fs.rmSync(req.dst);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o500);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, [factory]));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
try {
|
||||
const readTool = session.getToolByName("read") as AgentTool | undefined;
|
||||
const readResult = await readTool!.execute("call-read-rem", { path: targetPath });
|
||||
const readHeaderLine = resultText(readResult).split("\n")[0] ?? "";
|
||||
expect(HASHLINE_HEADER_LINE.test(readHeaderLine)).toBe(true);
|
||||
|
||||
const editTool = session.getToolByName("edit") as AgentTool | undefined;
|
||||
const editResult = await editTool!.execute("call-edit-rem", {
|
||||
input: [readHeaderLine, "REM", ""].join("\n"),
|
||||
});
|
||||
|
||||
expect(editResult.isError).not.toBe(true);
|
||||
expect(deleted).toEqual([targetPath]);
|
||||
expect(writeCalls).toEqual([]);
|
||||
expect(fs.existsSync(targetPath)).toBe(false);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
await session.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
itDenied("write: a throwing handler does not skip later handlers from the SAME extension", async () => {
|
||||
// The registry sees ONE trampoline per extension, so per-handler isolation has to
|
||||
// live inside that trampoline. Without it, a throw from the first handler escapes
|
||||
// to the registry, which advances to the next EXTENSION — so every later handler
|
||||
// this extension registered is skipped, breaking both the documented "a throwing
|
||||
// handler is skipped" rule and registration order for a backup-handler setup.
|
||||
const tempDir = makeTempDir();
|
||||
const lockedDir = path.join(tempDir, "locked-order");
|
||||
fs.mkdirSync(lockedDir, { recursive: true });
|
||||
lock(lockedDir, 0o500);
|
||||
|
||||
const order: string[] = [];
|
||||
const factory: ExtensionFactory = pi => {
|
||||
pi.registerFileWriteFallback(async () => {
|
||||
order.push("throws");
|
||||
throw new Error("first handler blew up");
|
||||
});
|
||||
pi.registerFileWriteFallback(async () => {
|
||||
order.push("declines");
|
||||
return false;
|
||||
});
|
||||
pi.registerFileWriteFallback(async req => {
|
||||
order.push("brokers");
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
try {
|
||||
fs.writeFileSync(req.dst, req.content);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o500);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, [factory]));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
try {
|
||||
const targetPath = path.join(lockedDir, "ordered.txt");
|
||||
const content = "export const value = 3;\n";
|
||||
const writeTool = session.getToolByName("write") as AgentTool | undefined;
|
||||
const writeResult = await writeTool!.execute("call-write-order", { path: targetPath, content });
|
||||
|
||||
expect(writeResult.isError).not.toBe(true);
|
||||
expect(order).toEqual(["throws", "declines", "brokers"]);
|
||||
expect(fs.readFileSync(targetPath, "utf8")).toBe(content);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
await session.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
itDenied("write: a handler sees the session's CURRENT cwd, not the one captured at init", async () => {
|
||||
// Handlers are installed once, at `ExtensionRunner.initialize`, but every other
|
||||
// extension dispatch builds its `ExtensionContext` per call — and `createContext`
|
||||
// materializes `cwd` as a value. A trampoline holding one context for the life of
|
||||
// the session would keep reporting the workspace it initialized in, so a handler
|
||||
// that scopes or prompts against `ctx.cwd` would allow the old workspace and deny
|
||||
// the new one after a `/move`.
|
||||
const tempDir = makeTempDir();
|
||||
const lockedDir = path.join(tempDir, "locked-cwd");
|
||||
fs.mkdirSync(lockedDir, { recursive: true });
|
||||
lock(lockedDir, 0o500);
|
||||
|
||||
const seenCwds: string[] = [];
|
||||
const factory: ExtensionFactory = pi => {
|
||||
pi.registerFileWriteFallback(async (req, ctx) => {
|
||||
seenCwds.push(ctx.cwd);
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
try {
|
||||
fs.writeFileSync(req.dst, req.content);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o500);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, [factory]));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
// Stands in for `SessionManager.moveTo()` without relocating real session files:
|
||||
// `getCwd()` is the session's own source of truth for its workspace, and it is
|
||||
// what `ExtensionRunner.cwd` reads.
|
||||
const moved = path.join(tempDir, "moved-workspace");
|
||||
fs.mkdirSync(moved, { recursive: true });
|
||||
const cwdSpy = spyOn(session.sessionManager, "getCwd").mockReturnValue(moved);
|
||||
|
||||
try {
|
||||
const writeTool = session.getToolByName("write") as AgentTool | undefined;
|
||||
const writeResult = await writeTool!.execute("call-write-cwd", {
|
||||
path: path.join(lockedDir, "after-move.txt"),
|
||||
content: "export const value = 4;\n",
|
||||
});
|
||||
|
||||
expect(writeResult.isError).not.toBe(true);
|
||||
expect(seenCwds).toEqual([moved]);
|
||||
} finally {
|
||||
cwdSpy.mockRestore();
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
await session.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
itDenied(
|
||||
"edit: a hashline MV OUT of an undeletable directory removes the source through the delete seam",
|
||||
async () => {
|
||||
// `HashlineFilesystem.move` writes the destination and then unlinks the source as
|
||||
// two separate primitives. Moving INTO a locked directory only exercises the
|
||||
// write seam, because the source sits in the writable workspace. This is the
|
||||
// mirror case, and the only end-to-end cover for the source-unlink site: before
|
||||
// the delete seam the destination landed and the unlink threw, so the move failed
|
||||
// with the original left behind.
|
||||
const tempDir = makeTempDir();
|
||||
const lockedDir = path.join(tempDir, "locked-source");
|
||||
fs.mkdirSync(lockedDir, { recursive: true });
|
||||
const sourcePath = path.join(lockedDir, "escaping.txt");
|
||||
fs.writeFileSync(sourcePath, "export const stage = 1;\n");
|
||||
const destPath = path.join(tempDir, "escaped.txt");
|
||||
lock(lockedDir, 0o500);
|
||||
|
||||
const deleted: string[] = [];
|
||||
const factory: ExtensionFactory = pi => {
|
||||
pi.registerFileDeleteFallback(async req => {
|
||||
deleted.push(req.dst);
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
try {
|
||||
fs.rmSync(req.dst);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o500);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const { session } = await createAgentSession(baseOptions(tempDir, [factory]));
|
||||
initializeRunnerForTest(session.extensionRunner);
|
||||
|
||||
try {
|
||||
const readTool = session.getToolByName("read") as AgentTool | undefined;
|
||||
const readResult = await readTool!.execute("call-read-mv-out", { path: sourcePath });
|
||||
const readHeaderLine = resultText(readResult).split("\n")[0] ?? "";
|
||||
expect(HASHLINE_HEADER_LINE.test(readHeaderLine)).toBe(true);
|
||||
|
||||
const editTool = session.getToolByName("edit") as AgentTool | undefined;
|
||||
const editResult = await editTool!.execute("call-edit-mv-out", {
|
||||
input: [readHeaderLine, "PUT 1-1:", "+export const stage = 2;", `MV ${destPath}`, ""].join("\n"),
|
||||
});
|
||||
|
||||
expect(editResult.isError).not.toBe(true);
|
||||
expect(deleted).toEqual([sourcePath]);
|
||||
expect(fs.readFileSync(destPath, "utf8")).toBe("export const stage = 2;\n");
|
||||
expect(fs.existsSync(sourcePath)).toBe(false);
|
||||
} finally {
|
||||
fs.chmodSync(lockedDir, 0o700);
|
||||
await session.dispose();
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
import { afterEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as statsCli from "../src/cli/stats-cli";
|
||||
import Stats from "../src/commands/stats";
|
||||
import * as theme from "../src/modes/theme/theme";
|
||||
import { parseStatsDashboardArgs } from "../src/slash-commands/helpers/stats-dashboard";
|
||||
|
||||
const TEST_CONFIG = { bin: "omp", version: "0.0.0-test", commands: new Map() };
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("stats dashboard host arguments", () => {
|
||||
it("forwards the real omp stats flags to the dashboard runner", async () => {
|
||||
vi.spyOn(theme, "initTheme").mockResolvedValue();
|
||||
const runStatsCommand = vi.spyOn(statsCli, "runStatsCommand").mockResolvedValue();
|
||||
const command = new Stats(["--host", "::", "--port", "3850"], TEST_CONFIG);
|
||||
|
||||
await command.run();
|
||||
|
||||
expect(runStatsCommand).toHaveBeenCalledWith({
|
||||
port: 3850,
|
||||
host: "::",
|
||||
json: false,
|
||||
summary: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the slash command loopback-only unless a host is requested", () => {
|
||||
expect(parseStatsDashboardArgs("")).toEqual({ port: 3847, host: "127.0.0.1" });
|
||||
expect(parseStatsDashboardArgs("--host 0.0.0.0")).toEqual({ port: 3847, host: "0.0.0.0" });
|
||||
});
|
||||
});
|
||||
@@ -163,5 +163,11 @@ try {
|
||||
rescue.close();
|
||||
await fs.rm(home, { recursive: true, force: true });
|
||||
}
|
||||
}, 30_000);
|
||||
// Budget must exceed the sum of the bounds inside the test: two 15s marker waits
|
||||
// plus the 5s shutdown probe are 35s of legitimate waiting, so a 30s cap let a
|
||||
// loaded runner kill the test mid-`waitUntil` and report only "timed out after
|
||||
// 30000ms" instead of the marker assertion that actually failed. Each consumer is
|
||||
// a cold `bun` process importing the daemon module graph, so the spawns are slow
|
||||
// exactly when the machine is busy.
|
||||
}, 60_000);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,796 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { applyPatch } from "@oh-my-pi/pi-coding-agent/edit/modes/patch";
|
||||
import {
|
||||
addFileDeleteFallback,
|
||||
addFileWriteFallback,
|
||||
deleteFileWithFallback,
|
||||
isPermissionDeniedError,
|
||||
withFileMutationSession,
|
||||
writeFileWithFallback,
|
||||
} from "@oh-my-pi/pi-coding-agent/tools/file-write-fallback";
|
||||
|
||||
/** Mimics a Node/Bun filesystem error with a structured `code`, without touching a real fs. */
|
||||
function fsError(code: string, message = `${code}: simulated`): NodeJS.ErrnoException {
|
||||
const error = new Error(message) as NodeJS.ErrnoException;
|
||||
error.code = code;
|
||||
return error;
|
||||
}
|
||||
|
||||
describe("isPermissionDeniedError", () => {
|
||||
it("is true for EPERM, EACCES, and EROFS error codes", () => {
|
||||
expect(isPermissionDeniedError(fsError("EPERM"))).toBe(true);
|
||||
expect(isPermissionDeniedError(fsError("EACCES"))).toBe(true);
|
||||
expect(isPermissionDeniedError(fsError("EROFS"))).toBe(true);
|
||||
});
|
||||
|
||||
it("is false for unrelated error codes", () => {
|
||||
expect(isPermissionDeniedError(fsError("ENOENT"))).toBe(false);
|
||||
expect(isPermissionDeniedError(fsError("EISDIR"))).toBe(false);
|
||||
expect(isPermissionDeniedError(fsError("ENOSPC"))).toBe(false);
|
||||
});
|
||||
|
||||
it("is false for a plain error with no code or matching message", () => {
|
||||
expect(isPermissionDeniedError(new Error("something else went wrong"))).toBe(false);
|
||||
expect(isPermissionDeniedError("not an error")).toBe(false);
|
||||
expect(isPermissionDeniedError(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it("defensively matches a permission code embedded only in the message", () => {
|
||||
// A bridged/transport write can surface a denial as a plain Error with no code.
|
||||
expect(isPermissionDeniedError(new Error("write failed: EACCES permission denied"))).toBe(true);
|
||||
});
|
||||
|
||||
it("trusts a structured code over a permission name appearing in the path", () => {
|
||||
// Bun embeds the full path in fs error messages, so a directory literally named
|
||||
// EACCES would otherwise make an ordinary missing-path ENOENT look like a denial
|
||||
// and divert a write that should just fail.
|
||||
expect(isPermissionDeniedError(fsError("ENOENT", "ENOENT: no such file, open '/repo/EACCES/x.txt'"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("writeFileWithFallback", () => {
|
||||
const disposers: Array<() => void> = [];
|
||||
afterEach(() => {
|
||||
for (const dispose of disposers.splice(0)) dispose();
|
||||
});
|
||||
|
||||
/** A `BunFile`-shaped stub whose `.write()` always fails with `error`. */
|
||||
function denyingFile(error: unknown): { write: (content: string) => Promise<number> } {
|
||||
return {
|
||||
write: async () => {
|
||||
throw error;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("diverts a permission-denied write to a registered handler", async () => {
|
||||
const seen: Array<{ dst: string; content: string }> = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
seen.push({ dst: req.dst, content: req.content });
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EACCES")) as never);
|
||||
|
||||
expect(seen).toEqual([{ dst: "/denied/path.txt", content: "payload" }]);
|
||||
});
|
||||
|
||||
it("names the session that issued the write, and reports none outside a tool call", async () => {
|
||||
// The registry is process-wide, so a handler can be asked about a write from a
|
||||
// session other than its own. Without this it cannot tell the difference, which
|
||||
// is what makes a per-session decision (or a prompt through the right session's
|
||||
// UI) impossible.
|
||||
const seen: Array<string | undefined> = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
seen.push(req.sessionId);
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await withFileMutationSession("session-a", () =>
|
||||
writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EACCES")) as never),
|
||||
);
|
||||
// No scope: an external `applyPatch` caller is not attributable to a session,
|
||||
// and inventing one would be worse than saying so.
|
||||
await writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EACCES")) as never);
|
||||
|
||||
expect(seen).toEqual(["session-a", undefined]);
|
||||
});
|
||||
|
||||
it("rethrows a non-permission error without consulting any handler", async () => {
|
||||
let called = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
called = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(
|
||||
writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EISDIR")) as never),
|
||||
).rejects.toMatchObject({ code: "EISDIR" });
|
||||
expect(called).toBe(false);
|
||||
});
|
||||
|
||||
it("retries an ENOENT at most once when the parent turns out to be creatable", async () => {
|
||||
// A creatable parent means the ENOENT was a race, not a boundary: the helper
|
||||
// creates the directory and repeats the write. This stub keeps failing, which
|
||||
// pins the retry at exactly one extra attempt instead of spinning.
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "fallback-race-"));
|
||||
let attempts = 0;
|
||||
let handlerCalled = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
handlerCalled = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
const file = {
|
||||
write: async () => {
|
||||
attempts += 1;
|
||||
throw fsError("ENOENT");
|
||||
},
|
||||
};
|
||||
|
||||
await expect(
|
||||
writeFileWithFallback(path.join(root, "fresh", "path.txt"), "payload", file as never),
|
||||
).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(attempts).toBe(2);
|
||||
expect(handlerCalled).toBe(false);
|
||||
// The repair is the reason the retry happened, so it must be observable.
|
||||
expect((await fs.stat(path.join(root, "fresh"))).isDirectory()).toBe(true);
|
||||
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("rethrows the ORIGINAL error when the handler returns false", async () => {
|
||||
const cause = fsError("EACCES");
|
||||
disposers.push(addFileWriteFallback(async () => false));
|
||||
|
||||
await expect(writeFileWithFallback("/denied/path.txt", "payload", denyingFile(cause) as never)).rejects.toBe(
|
||||
cause,
|
||||
);
|
||||
});
|
||||
|
||||
it("rethrows the ORIGINAL error when every handler throws", async () => {
|
||||
const cause = fsError("EACCES");
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
throw new Error("handler blew up");
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(writeFileWithFallback("/denied/path.txt", "payload", denyingFile(cause) as never)).rejects.toBe(
|
||||
cause,
|
||||
);
|
||||
});
|
||||
|
||||
it("falls through a throwing handler to the next registered handler", async () => {
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
throw new Error("first handler blew up");
|
||||
}),
|
||||
);
|
||||
let secondCalled = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
secondCalled = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EACCES")) as never);
|
||||
|
||||
expect(secondCalled).toBe(true);
|
||||
});
|
||||
|
||||
it("invokes handlers in registration order and stops at the first success", async () => {
|
||||
const order: string[] = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
order.push("first");
|
||||
return false;
|
||||
}),
|
||||
);
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
order.push("second");
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
order.push("third");
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EACCES")) as never);
|
||||
|
||||
expect(order).toEqual(["first", "second"]);
|
||||
});
|
||||
|
||||
it("stops receiving writes once its disposer runs", async () => {
|
||||
let calls = 0;
|
||||
// Registered through `disposers` as well: if an assertion below throws, afterEach
|
||||
// still removes the handler. A leaked registration is process-global and would
|
||||
// silently swallow denied writes in every later test file.
|
||||
const dispose = addFileWriteFallback(async () => {
|
||||
calls += 1;
|
||||
return true;
|
||||
});
|
||||
disposers.push(dispose);
|
||||
|
||||
await writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EACCES")) as never);
|
||||
expect(calls).toBe(1);
|
||||
|
||||
dispose();
|
||||
|
||||
await expect(
|
||||
writeFileWithFallback("/denied/path.txt", "payload", denyingFile(fsError("EACCES")) as never),
|
||||
).rejects.toMatchObject({ code: "EACCES" });
|
||||
expect(calls).toBe(1);
|
||||
});
|
||||
|
||||
// A privileged user is not constrained by mode bits, so `chmod 0o500` denies
|
||||
// nothing and every expectation here would fail for a reason unrelated to this
|
||||
// seam. Root is real for a Docker-based local run and for a self-hosted runner.
|
||||
describe.skipIf(process.getuid?.() === 0)("against real kernel permissions", () => {
|
||||
let root = "";
|
||||
|
||||
beforeEach(async () => {
|
||||
// Canonical from the start: the seam hands handlers a symlink-resolved path,
|
||||
// and `os.tmpdir()` is under `/var` — itself a link — on macOS, so a lexical
|
||||
// fixture path would differ from the brokered one for a reason unrelated to
|
||||
// what these tests are about.
|
||||
root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "fallback-kernel-")));
|
||||
});
|
||||
afterEach(async () => {
|
||||
// Restore the mode first: a 0o500 directory cannot be emptied.
|
||||
await fs.chmod(path.join(root, "locked"), 0o700).catch(() => {});
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/** A directory the current user may traverse and read, but not create inside. */
|
||||
async function lockedDir(): Promise<string> {
|
||||
const dir = path.join(root, "locked");
|
||||
await fs.mkdir(dir);
|
||||
await fs.chmod(dir, 0o500);
|
||||
return dir;
|
||||
}
|
||||
|
||||
it("diverts a real EACCES from creating a file in an unwritable directory", async () => {
|
||||
const dst = path.join(await lockedDir(), "new.txt");
|
||||
const seen: Array<{ dst: string; content: string; code: unknown }> = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
seen.push({ dst: req.dst, content: req.content, code: (req.cause as NodeJS.ErrnoException).code });
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await writeFileWithFallback(dst, "payload");
|
||||
|
||||
expect(seen).toEqual([{ dst, content: "payload", code: "EACCES" }]);
|
||||
});
|
||||
|
||||
it("unmasks a denied parent mkdir that Bun reports as ENOENT", async () => {
|
||||
// Bun's write creates missing parents itself and, when that mkdir is denied,
|
||||
// surfaces the open()'s ENOENT instead of the denial. Without unmasking, a
|
||||
// sandboxed write into a new out-of-tree directory never reaches a handler.
|
||||
const dst = path.join(await lockedDir(), "sub", "new.txt");
|
||||
const seen: Array<{ dst: string; content: string; code: unknown }> = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
seen.push({ dst: req.dst, content: req.content, code: (req.cause as NodeJS.ErrnoException).code });
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await writeFileWithFallback(dst, "payload");
|
||||
|
||||
expect(seen).toEqual([{ dst, content: "payload", code: "EACCES" }]);
|
||||
});
|
||||
|
||||
it("attaches the recovered denial as `cause` when no handler takes the write", async () => {
|
||||
// The thrown error stays the ENOENT Bun reported, so behaviour matches a host
|
||||
// with no fallback registered. But this code has already proven the real
|
||||
// boundary is EACCES, and discarding that would hand the caller back exactly
|
||||
// the misleading errno this module exists to see through.
|
||||
const dst = path.join(await lockedDir(), "sub", "new.txt");
|
||||
disposers.push(addFileWriteFallback(async () => false));
|
||||
|
||||
await expect(writeFileWithFallback(dst, "payload")).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
cause: { code: "EACCES" },
|
||||
});
|
||||
});
|
||||
|
||||
it("leaves an ENOENT alone when a path component is a file rather than a directory", async () => {
|
||||
const blocker = path.join(root, "blocker");
|
||||
await Bun.write(blocker, "not a directory");
|
||||
let called = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
called = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(writeFileWithFallback(path.join(blocker, "child.txt"), "payload")).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(ENOTDIR|ENOENT)$/),
|
||||
});
|
||||
expect(called).toBe(false);
|
||||
});
|
||||
|
||||
it("brokers the RESOLVED target for a write through a symlink", async () => {
|
||||
// The escape this closes: the agent creates a link inside a directory the
|
||||
// sandbox permits, pointing at a target it does not. The in-process write
|
||||
// follows the link, so the kernel denied the TARGET — but a handler given
|
||||
// the LINK would pass its own prefix allowlist, because the link sits inside
|
||||
// the allowed root while its target does not. The handler is told where the
|
||||
// bytes would really land, so its allowlist judges the real destination.
|
||||
const secretDir = path.join(root, "off-limits");
|
||||
await fs.mkdir(secretDir);
|
||||
const secret = path.join(secretDir, "authorized_keys");
|
||||
await Bun.write(secret, "original\n");
|
||||
await fs.chmod(secret, 0o400);
|
||||
await fs.chmod(secretDir, 0o500);
|
||||
|
||||
const link = path.join(root, "innocent-link");
|
||||
await fs.symlink(secret, link);
|
||||
|
||||
const seen: string[] = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
seen.push(req.dst);
|
||||
// Declining stands in for the allowlist refusal a real helper makes.
|
||||
return false;
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
await expect(writeFileWithFallback(link, "pwned\n")).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(EACCES|EPERM)$/),
|
||||
});
|
||||
expect(seen).toEqual([secret]);
|
||||
expect(await Bun.file(secret).text()).toBe("original\n");
|
||||
} finally {
|
||||
await fs.chmod(secretDir, 0o700);
|
||||
await fs.chmod(secret, 0o600);
|
||||
}
|
||||
});
|
||||
|
||||
it("resolves a symlinked ANCESTOR, not just a link at the last component", async () => {
|
||||
// `lstat(dst)` alone judges only the final component, so `ws/link/file` under
|
||||
// a `ws/link -> /outside` link is a lexically innocent path whose bytes land
|
||||
// outside. Every component above the last is followed by the kernel, so the
|
||||
// handler has to be told the resolved path for this shape too.
|
||||
const outside = path.join(root, "off-limits");
|
||||
await fs.mkdir(outside);
|
||||
const victim = path.join(outside, "secret.txt");
|
||||
await Bun.write(victim, "original\n");
|
||||
await fs.chmod(victim, 0o400);
|
||||
await fs.chmod(outside, 0o500);
|
||||
|
||||
const linkDir = path.join(root, "innocent-dir");
|
||||
await fs.symlink(outside, linkDir);
|
||||
|
||||
const seen: string[] = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
seen.push(req.dst);
|
||||
return false;
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
await expect(writeFileWithFallback(path.join(linkDir, "secret.txt"), "pwned\n")).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(EACCES|EPERM)$/),
|
||||
});
|
||||
expect(seen).toEqual([victim]);
|
||||
expect(await Bun.file(victim).text()).toBe("original\n");
|
||||
} finally {
|
||||
await fs.chmod(outside, 0o700);
|
||||
await fs.chmod(victim, 0o600);
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses to broker a write through a dangling symlink", async () => {
|
||||
// `realpath` cannot name where a dangling link points, and the write follows
|
||||
// it, so there is no destination to hand a privileged writer. Refusing is the
|
||||
// only honest answer, and it is the one `confineToWorkspace` already gives.
|
||||
const dir = path.join(root, "locked");
|
||||
await fs.mkdir(dir);
|
||||
const dangling = path.join(dir, "dangling");
|
||||
await fs.symlink(path.join(dir, "nowhere"), dangling);
|
||||
await fs.chmod(dir, 0o500);
|
||||
|
||||
let called = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
called = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(writeFileWithFallback(dangling, "payload")).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(EACCES|EPERM)$/),
|
||||
});
|
||||
expect(called).toBe(false);
|
||||
});
|
||||
|
||||
it("refuses to broker a write whose own metadata is behind the boundary", async () => {
|
||||
// A sandbox that denies the write often hides the target's metadata too, so
|
||||
// the final component cannot be shown to be a plain name rather than a link —
|
||||
// and `open` follows a link there. The delete seam keeps working in this shape
|
||||
// because `unlink` never follows the last component; a write cannot.
|
||||
const opaque = path.join(root, "opaque");
|
||||
await fs.mkdir(opaque);
|
||||
await fs.chmod(opaque, 0o000);
|
||||
|
||||
let called = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
called = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
await expect(writeFileWithFallback(path.join(opaque, "new.txt"), "payload")).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(EACCES|EPERM)$/),
|
||||
});
|
||||
expect(called).toBe(false);
|
||||
} finally {
|
||||
await fs.chmod(opaque, 0o700);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// `apply_patch` creates a missing parent before writing, so a denial there used
|
||||
// to throw before the write — and therefore before the seam — was ever reached.
|
||||
describe.skipIf(process.getuid?.() === 0)("apply_patch into a denied new directory", () => {
|
||||
let root = "";
|
||||
let locked = "";
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "fallback-patch-")));
|
||||
locked = path.join(root, "locked");
|
||||
await fs.mkdir(locked);
|
||||
await fs.chmod(locked, 0o500);
|
||||
});
|
||||
afterEach(async () => {
|
||||
await fs.chmod(locked, 0o700).catch(() => {});
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("reaches a registered handler with the bytes for the created file", async () => {
|
||||
const brokered: Array<{ dst: string; content: string }> = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
brokered.push({ dst: req.dst, content: req.content });
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
const target = path.join(locked, "sub", "new.txt");
|
||||
const result = await applyPatch({ path: target, op: "create", diff: "hello\n" }, { cwd: root });
|
||||
|
||||
expect(result.change).toMatchObject({ type: "create", path: target });
|
||||
expect(brokered).toEqual([{ dst: target, content: "hello\n" }]);
|
||||
});
|
||||
|
||||
it("still fails when no handler is registered", async () => {
|
||||
const target = path.join(locked, "sub", "new.txt");
|
||||
await expect(applyPatch({ path: target, op: "create", diff: "hello\n" }, { cwd: root })).rejects.toMatchObject(
|
||||
{
|
||||
code: expect.stringMatching(/^(EACCES|EPERM)$/),
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("never brokers an exclusive create whose destination cannot be proven absent", async () => {
|
||||
// `apply_patch`'s `create` refuses to overwrite, and it decides that with
|
||||
// `Bun.file(dst).exists()`, which reports `false` when the parent hides the
|
||||
// target's metadata instead of distinguishing "absent" from "unknown". The
|
||||
// non-overwrite contract survives regardless, because the same denied `lstat`
|
||||
// that fools the existence check also stops the seam from brokering: a
|
||||
// privileged writer is never handed a destination whose identity is unproven,
|
||||
// and it is the only party that could have enforced exclusivity itself.
|
||||
//
|
||||
// Those are two independent guards in two files, so this pins the pair. If the
|
||||
// seam is ever relaxed to broker an unverifiable path, a `create` would start
|
||||
// silently clobbering a protected file it was told not to touch.
|
||||
const opaque = path.join(root, "opaque");
|
||||
await fs.mkdir(opaque);
|
||||
const victim = path.join(opaque, "victim.txt");
|
||||
await Bun.write(victim, "original\n");
|
||||
await fs.chmod(opaque, 0o000);
|
||||
|
||||
let called = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
called = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
// The premise: the existence check cannot see the file it must not clobber.
|
||||
expect(await Bun.file(victim).exists()).toBe(false);
|
||||
|
||||
await expect(
|
||||
applyPatch({ path: victim, op: "create", diff: "clobbered\n" }, { cwd: root }),
|
||||
).rejects.toMatchObject({ code: expect.stringMatching(/^(EACCES|EPERM)$/) });
|
||||
expect(called).toBe(false);
|
||||
} finally {
|
||||
await fs.chmod(opaque, 0o700);
|
||||
}
|
||||
expect(await Bun.file(victim).text()).toBe("original\n");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("deleteFileWithFallback", () => {
|
||||
const disposers: Array<() => void> = [];
|
||||
afterEach(() => {
|
||||
for (const dispose of disposers.splice(0)) dispose();
|
||||
});
|
||||
|
||||
it("rethrows ENOENT without consulting a handler", async () => {
|
||||
// `edit`'s REM turns this into a NotFoundError, so it must not be diverted.
|
||||
let called = false;
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async () => {
|
||||
called = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(deleteFileWithFallback("/nonexistent/nope.txt")).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(called).toBe(false);
|
||||
});
|
||||
|
||||
it("does not consult a registered WRITE handler", async () => {
|
||||
// A write handler brokers `content` to `dst`. If a delete reached it, brokering
|
||||
// a request with no content would truncate the file instead of removing it.
|
||||
let writeCalled = false;
|
||||
disposers.push(
|
||||
addFileWriteFallback(async () => {
|
||||
writeCalled = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(deleteFileWithFallback("/nonexistent/nope.txt")).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(writeCalled).toBe(false);
|
||||
});
|
||||
|
||||
describe.skipIf(process.getuid?.() === 0)("against real kernel permissions", () => {
|
||||
let root = "";
|
||||
let locked = "";
|
||||
|
||||
beforeEach(async () => {
|
||||
// Canonical from the start; see the write-side note above.
|
||||
root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "fallback-del-")));
|
||||
locked = path.join(root, "locked");
|
||||
await fs.mkdir(locked);
|
||||
});
|
||||
afterEach(async () => {
|
||||
await fs.chmod(locked, 0o700).catch(() => {});
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/** A file whose containing directory denies the unlink. */
|
||||
async function lockedFile(name = "victim.txt"): Promise<string> {
|
||||
const target = path.join(locked, name);
|
||||
await Bun.write(target, "payload");
|
||||
await fs.chmod(locked, 0o500);
|
||||
return target;
|
||||
}
|
||||
|
||||
it("diverts a real denied unlink to a registered handler, naming its session", async () => {
|
||||
const target = await lockedFile();
|
||||
const seen: Array<{ dst: string; code: unknown; sessionId: string | undefined }> = [];
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
seen.push({ dst: req.dst, code: (req.cause as NodeJS.ErrnoException).code, sessionId: req.sessionId });
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await withFileMutationSession("session-del", () => deleteFileWithFallback(target));
|
||||
|
||||
expect(seen).toEqual([
|
||||
{ dst: target, code: expect.stringMatching(/^(EACCES|EPERM)$/), sessionId: "session-del" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("rethrows the ORIGINAL error when the handler declines", async () => {
|
||||
const target = await lockedFile();
|
||||
disposers.push(addFileDeleteFallback(async () => false));
|
||||
|
||||
await expect(deleteFileWithFallback(target)).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(EACCES|EPERM)$/),
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses to divert a directory it can confirm, reporting confirmedFile on files", async () => {
|
||||
// On Darwin `unlink` on a directory fails EPERM, which by code alone looks
|
||||
// exactly like a sandbox denial. Brokering it would ask a privileged deleter
|
||||
// to remove a whole directory for a tool that only ever removes one file.
|
||||
const dir = path.join(root, "a-directory");
|
||||
await fs.mkdir(dir);
|
||||
const seen: boolean[] = [];
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
seen.push(req.confirmedFile);
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(deleteFileWithFallback(dir)).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(EPERM|EISDIR)$/),
|
||||
});
|
||||
expect(seen).toEqual([]);
|
||||
expect((await fs.lstat(dir)).isDirectory()).toBe(true);
|
||||
|
||||
// A file under a directory that denies the unlink but still permits lstat
|
||||
// resolves the check, so the handler is told the target is a real file.
|
||||
const target = await lockedFile("confirmed.txt");
|
||||
await deleteFileWithFallback(target);
|
||||
expect(seen).toEqual([true]);
|
||||
});
|
||||
|
||||
it("still diverts, unresolved, when the target's own metadata is denied", async () => {
|
||||
// A sandbox that denies the unlink usually denies the metadata too, so the
|
||||
// directory check cannot run. The write must still reach a handler — that is
|
||||
// the whole point of the seam — but the handler has to be TOLD the check was
|
||||
// unresolved, or it may recursively remove a path that is really a directory.
|
||||
const opaque = path.join(root, "opaque");
|
||||
await fs.mkdir(opaque);
|
||||
const victim = path.join(opaque, "buried.txt");
|
||||
await Bun.write(victim, "payload");
|
||||
await fs.chmod(opaque, 0o000);
|
||||
|
||||
const seen: Array<{ dst: string; confirmedFile: boolean }> = [];
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
seen.push({ dst: req.dst, confirmedFile: req.confirmedFile });
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
await deleteFileWithFallback(victim);
|
||||
expect(seen).toEqual([{ dst: victim, confirmedFile: false }]);
|
||||
} finally {
|
||||
await fs.chmod(opaque, 0o700);
|
||||
}
|
||||
});
|
||||
|
||||
it("rethrows a non-permission lstat failure rather than diverting", async () => {
|
||||
// `ENOTDIR` from a path component that is a file is a genuinely bad path, not
|
||||
// a boundary, so the seam must not paper over it by consulting a handler.
|
||||
const blocker = path.join(root, "not-a-dir");
|
||||
await Bun.write(blocker, "payload");
|
||||
let called = false;
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async () => {
|
||||
called = true;
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(deleteFileWithFallback(path.join(blocker, "child.txt"))).rejects.toMatchObject({
|
||||
code: "ENOTDIR",
|
||||
});
|
||||
expect(called).toBe(false);
|
||||
});
|
||||
|
||||
it("resolves a symlinked ANCESTOR before brokering a delete", async () => {
|
||||
// `unlink` follows every component above the last, so a link in the path
|
||||
// removes a file outside the allowed root while the lexical path still looks
|
||||
// contained. The handler must be told which file actually disappears.
|
||||
const outside = path.join(root, "off-limits");
|
||||
await fs.mkdir(outside);
|
||||
const victim = path.join(outside, "keep.txt");
|
||||
await Bun.write(victim, "keep me");
|
||||
await fs.chmod(outside, 0o500);
|
||||
|
||||
const linkDir = path.join(root, "innocent-dir");
|
||||
await fs.symlink(outside, linkDir);
|
||||
|
||||
const seen: Array<{ dst: string; confirmedFile: boolean }> = [];
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
seen.push({ dst: req.dst, confirmedFile: req.confirmedFile });
|
||||
return false;
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
await expect(deleteFileWithFallback(path.join(linkDir, "keep.txt"))).rejects.toMatchObject({
|
||||
code: expect.stringMatching(/^(EACCES|EPERM)$/),
|
||||
});
|
||||
expect(seen).toEqual([{ dst: victim, confirmedFile: true }]);
|
||||
expect(await Bun.file(victim).text()).toBe("keep me");
|
||||
} finally {
|
||||
await fs.chmod(outside, 0o700);
|
||||
}
|
||||
});
|
||||
|
||||
it("leaves the LAST component unresolved, reporting confirmedFile false for a link", async () => {
|
||||
// `unlink` removes the link itself, so resolving the final component would
|
||||
// name the wrong file. Diverting is still right — unlinking a link is a
|
||||
// legitimate file removal — but a handler that realpaths `dst` for auditing,
|
||||
// or removes it recursively, would act on the link's TARGET, a directory tree
|
||||
// here. So the link is brokered as itself, and `confirmedFile` is false even
|
||||
// though `lstat` succeeded.
|
||||
const targetDir = path.join(root, "link-target-dir");
|
||||
await fs.mkdir(targetDir);
|
||||
await Bun.write(path.join(targetDir, "keep.txt"), "keep me");
|
||||
const link = path.join(locked, "dir-link");
|
||||
await fs.symlink(targetDir, link);
|
||||
await fs.chmod(locked, 0o500);
|
||||
|
||||
const seen: Array<{ dst: string; confirmedFile: boolean }> = [];
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
seen.push({ dst: req.dst, confirmedFile: req.confirmedFile });
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await deleteFileWithFallback(link);
|
||||
|
||||
expect(seen).toEqual([{ dst: link, confirmedFile: false }]);
|
||||
// The target must be untouched: the seam only ever asked for the link.
|
||||
expect(await Bun.file(path.join(targetDir, "keep.txt")).text()).toBe("keep me");
|
||||
});
|
||||
|
||||
it("diverts a denied unlink issued through a BunFile handle", async () => {
|
||||
// `LspFileSystem.delete` is the only caller that passes a `BunFile`, and it is
|
||||
// covered only transitively, so the `file.unlink()` branch would otherwise
|
||||
// never be exercised directly.
|
||||
const target = await lockedFile("via-handle.txt");
|
||||
const seen: string[] = [];
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
seen.push(req.dst);
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
await deleteFileWithFallback(target, Bun.file(target));
|
||||
|
||||
expect(seen).toEqual([target]);
|
||||
});
|
||||
|
||||
it("routes an apply_patch delete op through the seam", async () => {
|
||||
const target = await lockedFile("doomed.txt");
|
||||
const removed: string[] = [];
|
||||
disposers.push(
|
||||
addFileDeleteFallback(async req => {
|
||||
await fs.chmod(locked, 0o700);
|
||||
await fs.unlink(req.dst);
|
||||
removed.push(req.dst);
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await applyPatch({ path: target, op: "delete" }, { cwd: root });
|
||||
|
||||
expect(result.change).toMatchObject({ type: "delete", path: target });
|
||||
expect(removed).toEqual([target]);
|
||||
expect(await Bun.file(target).exists()).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,8 +1,10 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as path from "node:path";
|
||||
import { createLspWritethrough } from "@oh-my-pi/pi-coding-agent/lsp";
|
||||
import * as lspConfig from "@oh-my-pi/pi-coding-agent/lsp/config";
|
||||
import type { LinterClient, ServerConfig } from "@oh-my-pi/pi-coding-agent/lsp/types";
|
||||
import { addFileWriteFallback } from "@oh-my-pi/pi-coding-agent/tools/file-write-fallback";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
function createFormatter(format: (filePath: string, content: string) => Promise<string>): ServerConfig {
|
||||
@@ -180,3 +182,61 @@ describe("createLspWritethrough batching", () => {
|
||||
expect(await Bun.file(filePath).text()).toBe("const single = true;\n");
|
||||
});
|
||||
});
|
||||
|
||||
// A privileged user is not constrained by mode bits: a 0o000 file stays both
|
||||
// writable and readable, so the write would never be denied and the seam under
|
||||
// test would never engage.
|
||||
describe.skipIf(process.getuid?.() === 0)("createLspWritethrough batching with a brokered write", () => {
|
||||
let tempDir: TempDir;
|
||||
let root = "";
|
||||
const disposers: Array<() => void> = [];
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = TempDir.createSync("@omp-lsp-batch-broker-");
|
||||
// The seam hands handlers a symlink-resolved path and `os.tmpdir()` sits
|
||||
// under `/var` — itself a link — on macOS, so a lexical fixture root would
|
||||
// differ from the brokered path for a reason unrelated to this test.
|
||||
root = await fs.realpath(tempDir.path());
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
for (const dispose of disposers.splice(0)) dispose();
|
||||
vi.restoreAllMocks();
|
||||
await fs.chmod(path.join(root, "opaque.ts"), 0o600).catch(() => {});
|
||||
tempDir.removeSync();
|
||||
});
|
||||
|
||||
it("flushes a batch whose brokered destination cannot be read back", async () => {
|
||||
vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: {}, idleTimeoutMs: undefined });
|
||||
vi.spyOn(lspConfig, "getServersForFile").mockReturnValue([]);
|
||||
const writethrough = createLspWritethrough(root, { enableFormat: true, enableDiagnostics: true });
|
||||
|
||||
// Denied for writing and for reading at once, which is what a sandbox that
|
||||
// hides a path produces: the direct write fails, a privileged helper lands
|
||||
// the bytes, and this process still cannot read them back.
|
||||
const opaque = path.join(root, "opaque.ts");
|
||||
await Bun.write(opaque, "const before = true;\n");
|
||||
await fs.chmod(opaque, 0o000);
|
||||
|
||||
const brokered: Array<{ dst: string; content: string }> = [];
|
||||
disposers.push(
|
||||
addFileWriteFallback(async req => {
|
||||
brokered.push({ dst: req.dst, content: req.content });
|
||||
await fs.chmod(req.dst, 0o600);
|
||||
await Bun.write(req.dst, req.content);
|
||||
await fs.chmod(req.dst, 0o000);
|
||||
return true;
|
||||
}),
|
||||
);
|
||||
|
||||
const sibling = path.join(root, "sibling.ts");
|
||||
const batchId = `brokered-${Date.now()}`;
|
||||
await writethrough(opaque, "const after = true;\n", undefined, undefined, { id: batchId, flush: false });
|
||||
await writethrough(sibling, "const other = true;\n", undefined, undefined, { id: batchId, flush: true });
|
||||
|
||||
expect(brokered).toEqual([{ dst: opaque, content: "const after = true;\n" }]);
|
||||
expect(await Bun.file(sibling).text()).toBe("const other = true;\n");
|
||||
await fs.chmod(opaque, 0o400);
|
||||
expect(await Bun.file(opaque).text()).toBe("const after = true;\n");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the stats dashboard being unreachable from container hosts by accepting an explicit `--host` bind address while preserving loopback-only binding and same-origin API access by default.
|
||||
|
||||
## [17.3.0] - 2026-08-13
|
||||
|
||||
### Added
|
||||
|
||||
+33
-10
@@ -4,7 +4,7 @@ import { parseArgs } from "node:util";
|
||||
import { formatDuration, formatNumber, formatPercent } from "@oh-my-pi/pi-utils";
|
||||
import { getDashboardStats, getTotalMessageCount, syncAllSessions } from "./aggregator";
|
||||
import { closeDb } from "./db";
|
||||
import { startServer } from "./server";
|
||||
import { formatStatsDashboardUrl, startServer } from "./server";
|
||||
|
||||
export {
|
||||
getDashboardStats,
|
||||
@@ -17,7 +17,7 @@ export {
|
||||
} from "./aggregator";
|
||||
export { closeDb } from "./db";
|
||||
export { getGainDashboardStats } from "./gain-aggregator";
|
||||
export { startServer } from "./server";
|
||||
export { formatStatsDashboardUrl, startServer } from "./server";
|
||||
export type {
|
||||
GainDashboardStats,
|
||||
GainSource,
|
||||
@@ -96,19 +96,42 @@ async function printStats(): Promise<void> {
|
||||
console.log("");
|
||||
}
|
||||
|
||||
/**
|
||||
* Main CLI entry point.
|
||||
*/
|
||||
async function main(): Promise<void> {
|
||||
/** Parsed arguments for the standalone `omp-stats` entry point. */
|
||||
export interface StandaloneStatsArgs {
|
||||
port: number;
|
||||
host: string;
|
||||
json: boolean;
|
||||
sync: boolean;
|
||||
help: boolean;
|
||||
}
|
||||
|
||||
/** Parse the standalone `omp-stats` arguments used by the production entry point. */
|
||||
export function parseStandaloneStatsArgs(args: string[]): StandaloneStatsArgs {
|
||||
const { values } = parseArgs({
|
||||
args,
|
||||
options: {
|
||||
port: { type: "string", short: "p", default: "3847" },
|
||||
host: { type: "string", default: "127.0.0.1" },
|
||||
json: { type: "boolean", short: "j", default: false },
|
||||
sync: { type: "boolean", short: "s", default: false },
|
||||
help: { type: "boolean", short: "h", default: false },
|
||||
},
|
||||
allowPositionals: true,
|
||||
});
|
||||
return {
|
||||
port: parseInt(values.port || "3847", 10),
|
||||
host: values.host || "127.0.0.1",
|
||||
json: values.json ?? false,
|
||||
sync: values.sync ?? false,
|
||||
help: values.help ?? false,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Main CLI entry point.
|
||||
*/
|
||||
async function main(): Promise<void> {
|
||||
const values = parseStandaloneStatsArgs(process.argv.slice(2));
|
||||
|
||||
if (values.help) {
|
||||
console.log(`
|
||||
@@ -119,6 +142,7 @@ Usage:
|
||||
|
||||
Options:
|
||||
-p, --port <port> Port for the dashboard server (default: 3847)
|
||||
--host <host> Host to bind (default: 127.0.0.1)
|
||||
-j, --json Output stats as JSON and exit
|
||||
-s, --sync Sync session files and show summary
|
||||
-h, --help Show this help message
|
||||
@@ -126,7 +150,7 @@ Options:
|
||||
Examples:
|
||||
omp-stats # Start dashboard server
|
||||
omp-stats --json # Print stats as JSON
|
||||
omp-stats --port 8080 # Start on custom port
|
||||
omp-stats --host 0.0.0.0 # Explicitly expose on all IPv4 interfaces
|
||||
omp-stats --sync # Sync and show summary
|
||||
`);
|
||||
return;
|
||||
@@ -171,9 +195,8 @@ Examples:
|
||||
}
|
||||
|
||||
// Start server
|
||||
const port = parseInt(values.port || "3847", 10);
|
||||
const { hostname, port: actualPort } = await startServer(port);
|
||||
console.log(`Dashboard available at: http://${hostname}:${actualPort}`);
|
||||
const { port: actualPort } = await startServer(values.port, values.host);
|
||||
console.log(`Dashboard available at: ${formatStatsDashboardUrl(values.host, actualPort)}`);
|
||||
console.log("Press Ctrl+C to stop\n");
|
||||
|
||||
// Keep process running
|
||||
|
||||
@@ -17,23 +17,28 @@ interface PortHolder {
|
||||
|
||||
/** Header stamped on every dashboard response so reuse probes can identify us. */
|
||||
export const STATS_DASHBOARD_HEADER = "x-omp-stats-dashboard";
|
||||
/** Header recording the server's requested bind host so reuse cannot change its exposure scope. */
|
||||
export const STATS_DASHBOARD_HOSTNAME_HEADER = "x-omp-stats-hostname";
|
||||
|
||||
/** Identity-header value for dashboards enforcing loopback-only, same-origin access. */
|
||||
export const STATS_DASHBOARD_SECURITY_VERSION = "2";
|
||||
/** Identity-header value for dashboards enforcing an explicit bind host and same-origin access. */
|
||||
export const STATS_DASHBOARD_SECURITY_VERSION = "3";
|
||||
|
||||
/** IPv4 loopback address shared by the dashboard server and reuse probe. */
|
||||
export const STATS_DASHBOARD_HOSTNAME = "127.0.0.1";
|
||||
|
||||
type StatsDashboardProbe = "reusable" | "occupied" | "unreachable";
|
||||
|
||||
async function probeStatsDashboard(port: number): Promise<StatsDashboardProbe> {
|
||||
async function probeStatsDashboard(port: number, hostname: string): Promise<StatsDashboardProbe> {
|
||||
const probeHostname = hostname === "0.0.0.0" ? STATS_DASHBOARD_HOSTNAME : hostname === "::" ? "::1" : hostname;
|
||||
const urlHostname = probeHostname.includes(":") ? `[${probeHostname}]` : probeHostname;
|
||||
try {
|
||||
const response = await fetch(`http://${STATS_DASHBOARD_HOSTNAME}:${port}/api/stats/models`, {
|
||||
const response = await fetch(`http://${urlHostname}:${port}/api/stats/models`, {
|
||||
signal: AbortSignal.timeout(STATS_PROBE_TIMEOUT_MS),
|
||||
});
|
||||
const reusable =
|
||||
response.status === 200 &&
|
||||
response.headers.get(STATS_DASHBOARD_HEADER) === STATS_DASHBOARD_SECURITY_VERSION &&
|
||||
response.headers.get(STATS_DASHBOARD_HOSTNAME_HEADER) === hostname &&
|
||||
!response.headers.has("Access-Control-Allow-Origin");
|
||||
await response.body?.cancel();
|
||||
return reusable ? "reusable" : "occupied";
|
||||
@@ -248,16 +253,16 @@ async function reclaimStatsPort(port: number): Promise<"retry"> {
|
||||
* The preflight is needed on platforms that permit wildcard and loopback-specific
|
||||
* listeners to coexist on one port.
|
||||
*/
|
||||
export async function prepareStatsPort(port: number): Promise<"retry" | "reuse"> {
|
||||
export async function prepareStatsPort(port: number, hostname = STATS_DASHBOARD_HOSTNAME): Promise<"retry" | "reuse"> {
|
||||
if (port === 0) return "retry";
|
||||
const probe = await probeStatsDashboard(port);
|
||||
const probe = await probeStatsDashboard(port, hostname);
|
||||
if (probe === "reusable") return "reuse";
|
||||
if (probe === "occupied") return reclaimStatsPort(port);
|
||||
return "retry";
|
||||
}
|
||||
|
||||
/** Reuse or reclaim a listener found after the server bind reports EADDRINUSE. */
|
||||
export async function recoverStatsPort(port: number): Promise<"retry" | "reuse"> {
|
||||
if ((await probeStatsDashboard(port)) === "reusable") return "reuse";
|
||||
export async function recoverStatsPort(port: number, hostname = STATS_DASHBOARD_HOSTNAME): Promise<"retry" | "reuse"> {
|
||||
if ((await probeStatsDashboard(port, hostname)) === "reusable") return "reuse";
|
||||
return reclaimStatsPort(port);
|
||||
}
|
||||
|
||||
@@ -26,6 +26,7 @@ import {
|
||||
recoverStatsPort,
|
||||
STATS_DASHBOARD_HEADER,
|
||||
STATS_DASHBOARD_HOSTNAME,
|
||||
STATS_DASHBOARD_HOSTNAME_HEADER,
|
||||
STATS_DASHBOARD_SECURITY_VERSION,
|
||||
} from "./port-conflict";
|
||||
|
||||
@@ -306,10 +307,16 @@ async function handleStatic(requestPath: string): Promise<Response> {
|
||||
return new Response("Not Found", { status: 404 });
|
||||
}
|
||||
|
||||
function createDashboardServer(port: number) {
|
||||
/** Format a dashboard origin, including brackets required by IPv6 literals. */
|
||||
export function formatStatsDashboardUrl(hostname: string, port: number): string {
|
||||
const urlHostname = hostname.includes(":") && !hostname.startsWith("[") ? `[${hostname}]` : hostname;
|
||||
return `http://${urlHostname}:${port}`;
|
||||
}
|
||||
|
||||
function createDashboardServer(port: number, hostname: string) {
|
||||
const server = Bun.serve({
|
||||
port,
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
hostname,
|
||||
async fetch(req) {
|
||||
const url = new URL(req.url);
|
||||
const path = url.pathname;
|
||||
@@ -318,6 +325,7 @@ function createDashboardServer(port: number) {
|
||||
// recognize this dashboard without allowing cross-origin API reads.
|
||||
const dashboardHeaders: Record<string, string> = {
|
||||
[STATS_DASHBOARD_HEADER]: STATS_DASHBOARD_SECURITY_VERSION,
|
||||
[STATS_DASHBOARD_HOSTNAME_HEADER]: hostname,
|
||||
};
|
||||
|
||||
if (req.method === "OPTIONS") {
|
||||
@@ -358,37 +366,40 @@ function createDashboardServer(port: number) {
|
||||
/**
|
||||
* Start the HTTP server, reusing a live dashboard or reclaiming a stale omp listener.
|
||||
*/
|
||||
export async function startServer(port = 3847): Promise<{ hostname: string; port: number; stop: () => void }> {
|
||||
export async function startServer(
|
||||
port = 3847,
|
||||
hostname = STATS_DASHBOARD_HOSTNAME,
|
||||
): Promise<{ hostname: string; port: number; stop: () => void }> {
|
||||
await ensureClientBuild();
|
||||
const preparation = await prepareStatsPort(port);
|
||||
const preparation = await prepareStatsPort(port, hostname);
|
||||
if (preparation === "reuse") {
|
||||
return { hostname: STATS_DASHBOARD_HOSTNAME, port, stop: () => {} };
|
||||
return { hostname, port, stop: () => {} };
|
||||
}
|
||||
|
||||
try {
|
||||
const server = createDashboardServer(port);
|
||||
const server = createDashboardServer(port, hostname);
|
||||
return {
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
hostname,
|
||||
port: server.port ?? port,
|
||||
stop: () => server.stop(),
|
||||
};
|
||||
} catch (error) {
|
||||
if (!(error instanceof Error && "code" in error && error.code === "EADDRINUSE")) throw error;
|
||||
|
||||
const recovery = await recoverStatsPort(port);
|
||||
const recovery = await recoverStatsPort(port, hostname);
|
||||
if (recovery === "reuse") {
|
||||
return { hostname: STATS_DASHBOARD_HOSTNAME, port, stop: () => {} };
|
||||
return { hostname, port, stop: () => {} };
|
||||
}
|
||||
|
||||
try {
|
||||
const server = createDashboardServer(port);
|
||||
const server = createDashboardServer(port, hostname);
|
||||
return {
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
hostname,
|
||||
port: server.port ?? port,
|
||||
stop: () => server.stop(),
|
||||
};
|
||||
} catch (retryError) {
|
||||
throw new Error(`Failed to start stats dashboard on port ${port} after reclaiming it.`, {
|
||||
throw new Error(`Failed to start stats dashboard on ${hostname}:${port} after reclaiming it.`, {
|
||||
cause: retryError,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import { connect, type Subprocess } from "bun";
|
||||
import {
|
||||
STATS_DASHBOARD_HEADER,
|
||||
STATS_DASHBOARD_HOSTNAME,
|
||||
STATS_DASHBOARD_HOSTNAME_HEADER,
|
||||
STATS_DASHBOARD_SECURITY_VERSION,
|
||||
} from "../src/port-conflict";
|
||||
import { startServer } from "../src/server";
|
||||
@@ -25,6 +26,15 @@ async function tcpConnects(hostname: string, port: number): Promise<boolean> {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
function getNonLoopbackHostname(): string | undefined {
|
||||
const interfaces = networkInterfaces();
|
||||
for (const name in interfaces) {
|
||||
for (const address of interfaces[name] ?? []) {
|
||||
if (address.family === "IPv4" && !address.internal) return address.address;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const holderProcesses: Array<Subprocess<"ignore", "pipe", "pipe">> = [];
|
||||
|
||||
@@ -77,26 +87,34 @@ describe("startServer access", () => {
|
||||
const response = await fetch(`http://${server.hostname}:${server.port}/api/stats/models`);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe(STATS_DASHBOARD_SECURITY_VERSION);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HOSTNAME_HEADER)).toBe(STATS_DASHBOARD_HOSTNAME);
|
||||
expect(response.headers.get("Access-Control-Allow-Origin")).toBeNull();
|
||||
await response.body?.cancel();
|
||||
|
||||
let nonLoopbackHostname: string | undefined;
|
||||
const interfaces = networkInterfaces();
|
||||
for (const name in interfaces) {
|
||||
const addresses = interfaces[name] ?? [];
|
||||
for (const address of addresses) {
|
||||
if (address.family === "IPv4" && !address.internal) {
|
||||
nonLoopbackHostname = address.address;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (nonLoopbackHostname) break;
|
||||
}
|
||||
expect(nonLoopbackHostname).toBeDefined();
|
||||
const nonLoopbackHostname = getNonLoopbackHostname();
|
||||
expect(await tcpConnects(server.hostname, server.port)).toBe(true);
|
||||
if (nonLoopbackHostname) {
|
||||
expect(await tcpConnects(nonLoopbackHostname, server.port)).toBe(false);
|
||||
}
|
||||
if (nonLoopbackHostname) expect(await tcpConnects(nonLoopbackHostname, server.port)).toBe(false);
|
||||
} finally {
|
||||
server.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("serves non-loopback requests only when explicitly requested", async () => {
|
||||
const nonLoopbackHostname = getNonLoopbackHostname();
|
||||
if (!nonLoopbackHostname) return;
|
||||
|
||||
const server = await startServer(0, "0.0.0.0");
|
||||
|
||||
try {
|
||||
expect(server.hostname).toBe("0.0.0.0");
|
||||
expect(await tcpConnects(nonLoopbackHostname, server.port)).toBe(true);
|
||||
|
||||
const response = await fetch(`http://${STATS_DASHBOARD_HOSTNAME}:${server.port}/api/stats/models`);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe(STATS_DASHBOARD_SECURITY_VERSION);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HOSTNAME_HEADER)).toBe("0.0.0.0");
|
||||
expect(response.headers.get("Access-Control-Allow-Origin")).toBeNull();
|
||||
await response.body?.cancel();
|
||||
} finally {
|
||||
server.stop();
|
||||
}
|
||||
@@ -110,7 +128,12 @@ describe("startServer port conflicts", () => {
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
fetch: request =>
|
||||
new URL(request.url).pathname === "/api/stats/models"
|
||||
? Response.json([], { headers: { [STATS_DASHBOARD_HEADER]: STATS_DASHBOARD_SECURITY_VERSION } })
|
||||
? Response.json([], {
|
||||
headers: {
|
||||
[STATS_DASHBOARD_HEADER]: STATS_DASHBOARD_SECURITY_VERSION,
|
||||
[STATS_DASHBOARD_HOSTNAME_HEADER]: STATS_DASHBOARD_HOSTNAME,
|
||||
},
|
||||
})
|
||||
: new Response("dashboard"),
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { parseStandaloneStatsArgs } from "../src/index";
|
||||
import { formatStatsDashboardUrl } from "../src/server";
|
||||
|
||||
describe("standalone stats CLI", () => {
|
||||
it("keeps the production parser loopback-only by default", () => {
|
||||
expect(parseStandaloneStatsArgs([])).toEqual({
|
||||
port: 3847,
|
||||
host: "127.0.0.1",
|
||||
json: false,
|
||||
sync: false,
|
||||
help: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("forwards explicit bind hosts and formats IPv6 dashboard URLs", () => {
|
||||
expect(parseStandaloneStatsArgs(["--host", "::", "--port", "3850"])).toMatchObject({
|
||||
port: 3850,
|
||||
host: "::",
|
||||
});
|
||||
expect(formatStatsDashboardUrl("::", 3850)).toBe("http://[::]:3850");
|
||||
expect(formatStatsDashboardUrl("2001:db8::1", 3850)).toBe("http://[2001:db8::1]:3850");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user