Merge remote-tracking branch 'upstream/main' into feat/error-notify
This commit is contained in:
@@ -275,6 +275,16 @@ export class CollabGuestLink {
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (frame.t === "error" && !this.#welcomed && !this.#left) {
|
||||
// Pre-welcome errors are the host's targeted reply to our
|
||||
// hello (e.g. protocol mismatch): no welcome will follow.
|
||||
// Fail the join with the host's message instead of hanging
|
||||
// until the welcome timeout.
|
||||
this.#clearWelcomeTimer();
|
||||
if (joined) this.#ctx.showError(`Collab host: ${frame.message}`);
|
||||
else firstWelcome.reject(new Error(frame.message));
|
||||
return;
|
||||
}
|
||||
if (!this.#welcomed || this.#left) return;
|
||||
this.#applyFrame(frame);
|
||||
})
|
||||
|
||||
@@ -65,7 +65,9 @@ declare module "@oh-my-pi/pi-tui" {
|
||||
* Resolve default image-paste shortcuts for the current terminal platform.
|
||||
*/
|
||||
export function getDefaultPasteImageKeys(platform: NodeJS.Platform = process.platform): KeyId[] {
|
||||
return platform === "win32" ? ["ctrl+v", "alt+v"] : ["ctrl+v"];
|
||||
if (platform === "win32") return ["ctrl+v", "alt+v"];
|
||||
if (platform === "darwin") return ["ctrl+v", "super+v"];
|
||||
return ["ctrl+v"];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -52,7 +52,7 @@ import type { ApiKeyResolver, FetchImpl } from "@oh-my-pi/pi-ai";
|
||||
import { registerOAuthProvider, unregisterOAuthProviders } from "@oh-my-pi/pi-ai/oauth";
|
||||
import type { OAuthCredentials, OAuthLoginCallbacks } from "@oh-my-pi/pi-ai/oauth/types";
|
||||
import { getBundledModelReferenceIndex, resolveModelReference } from "@oh-my-pi/pi-catalog/identity";
|
||||
import { isBunTestRuntime, isRecord, logger } from "@oh-my-pi/pi-utils";
|
||||
import { isBunTestRuntime, isRecord, logger, wrapFetchForExtraCa } from "@oh-my-pi/pi-utils";
|
||||
import { parseModelString, resolveProviderModelReference } from "../config/model-resolver";
|
||||
import type { AuthStorage, OAuthCredential } from "../session/auth-storage";
|
||||
import { type ApiKeyResolverModel, type ApiKeyResolverOptions, createApiKeyResolver } from "./api-key-resolver";
|
||||
@@ -763,7 +763,7 @@ export class ModelRegistry {
|
||||
options?.fetch ??
|
||||
(isBunTestRuntime()
|
||||
? () => Promise.reject(new Error("network disabled in model-registry runtime test"))
|
||||
: fetch);
|
||||
: wrapFetchForExtraCa(fetch));
|
||||
this.#modelsConfigFile = ModelsConfigFile.relocate(modelsPath);
|
||||
this.#cacheDbPath = modelsPath ? path.join(path.dirname(modelsPath), "models.db") : undefined;
|
||||
// Set up fallback resolver for custom provider API keys
|
||||
|
||||
@@ -180,7 +180,8 @@ function resolvePreviewEdits(args: {
|
||||
}): readonly Edit[] {
|
||||
const { section, absolutePath, normalized, snapshots, expected, liveMatches, edits } = args;
|
||||
if (!hasBlockEdit(edits)) return edits;
|
||||
const baseText = expected === undefined || liveMatches ? normalized : snapshots.byHash(absolutePath, expected)?.text;
|
||||
const baseText =
|
||||
expected === undefined || liveMatches ? normalized : snapshots.byHashExact(absolutePath, expected)?.text;
|
||||
if (baseText === undefined) {
|
||||
throw createMismatchError(section, absolutePath, normalized, snapshots, expected ?? "");
|
||||
}
|
||||
@@ -199,7 +200,17 @@ function applyPreviewEdits(args: {
|
||||
if (!options.skipHashValidation && expected === undefined) {
|
||||
throw new Error(missingSnapshotTagMessage(section.path));
|
||||
}
|
||||
const liveMatches = expected !== undefined && computeFileHash(normalized) === expected;
|
||||
// A 16-bit tag can collide across two different file states, so hash
|
||||
// equality alone does not prove the live text IS the snapshot the model's
|
||||
// anchors were minted against (mirrors Patcher's apply-time guard). When
|
||||
// the store retains text for the tag, require it to be unambiguous and
|
||||
// byte-identical to live; otherwise fall through to recovery/reject below
|
||||
// exactly as if the hash had not matched.
|
||||
const liveMatches =
|
||||
expected !== undefined &&
|
||||
computeFileHash(normalized) === expected &&
|
||||
(snapshots.byHash(absolutePath, expected) === null ||
|
||||
snapshots.byHashExact(absolutePath, expected)?.text === normalized);
|
||||
const edits = parsePreviewEdits(section, options.streaming);
|
||||
const resolved = resolvePreviewEdits({ section, absolutePath, normalized, snapshots, expected, liveMatches, edits });
|
||||
if (options.skipHashValidation || expected === undefined || liveMatches) return applyEdits(normalized, resolved);
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import { setKittyProtocolActive } from "@oh-my-pi/pi-tui/keys";
|
||||
import { $ } from "bun";
|
||||
import { getDefaultPasteImageKeys } from "../../config/keybindings";
|
||||
import { getEditorTheme, initTheme } from "../theme/theme";
|
||||
import {
|
||||
CustomEditor,
|
||||
@@ -124,6 +126,24 @@ describe("CustomEditor bracketed path paste", () => {
|
||||
expect(imagePathCalls).toBe(0);
|
||||
});
|
||||
});
|
||||
describe("CustomEditor configured paste image keys", () => {
|
||||
it("routes Ghostty Cmd+V kitty key events through the macOS image-paste default", () => {
|
||||
const { editor } = makeEditor();
|
||||
const onPasteImage = vi.fn();
|
||||
editor.onPasteImage = onPasteImage;
|
||||
editor.setActionKeys("app.clipboard.pasteImage", getDefaultPasteImageKeys("darwin"));
|
||||
setKittyProtocolActive(true);
|
||||
|
||||
try {
|
||||
editor.handleInput("\x1b[118;9u");
|
||||
} finally {
|
||||
setKittyProtocolActive(false);
|
||||
}
|
||||
|
||||
expect(onPasteImage).toHaveBeenCalledTimes(1);
|
||||
expect(editor.getText()).toBe("");
|
||||
});
|
||||
});
|
||||
|
||||
describe("extractImagePathFromText (issue #3506)", () => {
|
||||
it("returns the path when the text is a single image file path", () => {
|
||||
|
||||
@@ -794,9 +794,15 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
getDraftText: () => this.editor.getText(),
|
||||
beginDispose: () => this.session.beginDispose(),
|
||||
saveDraft: text => this.sessionManager.saveDraft(text),
|
||||
disposeSession: () => this.session.dispose({ mnemopiConsolidateTimeoutMs: SHUTDOWN_CONSOLIDATE_BUDGET_MS }),
|
||||
disposeSession: reason =>
|
||||
this.session.dispose({ mnemopiConsolidateTimeoutMs: SHUTDOWN_CONSOLIDATE_BUDGET_MS, reason }),
|
||||
});
|
||||
this.#cleanupUnsubscribe = postmortem.register("session-teardown", () => this.#signalTeardown!());
|
||||
// Forward the postmortem reason (SIGTERM/SIGHUP/uncaughtException/…) so the
|
||||
// persisted `session_exit` diagnostic carries the real trigger. Postmortem
|
||||
// runs callbacks in REVERSE registration order — this callback (registered
|
||||
// after the AgentSession constructor's `agent-session:<id>` recorder) runs
|
||||
// FIRST and its dispose() would otherwise persist the generic "dispose".
|
||||
this.#cleanupUnsubscribe = postmortem.register("session-teardown", reason => this.#signalTeardown!(reason));
|
||||
|
||||
// Wire the report_tool_issue consent gate to the Yes/No dialog popup.
|
||||
// The handler is process-global — subagent tools (which can't reach
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { postmortem } from "@oh-my-pi/pi-utils";
|
||||
import { createSessionTeardown } from "./session-teardown";
|
||||
|
||||
/**
|
||||
@@ -156,4 +157,63 @@ describe("createSessionTeardown", () => {
|
||||
|
||||
expect(captured).toBe("before");
|
||||
});
|
||||
|
||||
it("forwards the postmortem reason into disposeSession so signal exits record the real trigger", async () => {
|
||||
const received: Array<postmortem.Reason | undefined> = [];
|
||||
|
||||
const teardown = createSessionTeardown({
|
||||
getDraftText: () => "",
|
||||
beginDispose: () => {},
|
||||
saveDraft: async () => {},
|
||||
disposeSession: async reason => {
|
||||
received.push(reason);
|
||||
},
|
||||
});
|
||||
|
||||
await teardown(postmortem.Reason.SIGTERM);
|
||||
|
||||
expect(received).toEqual([postmortem.Reason.SIGTERM]);
|
||||
});
|
||||
|
||||
it("keypress path passes no reason — dispose falls back to a normal exit record", async () => {
|
||||
const received: Array<postmortem.Reason | undefined> = [];
|
||||
|
||||
const teardown = createSessionTeardown({
|
||||
getDraftText: () => "",
|
||||
beginDispose: () => {},
|
||||
saveDraft: async () => {},
|
||||
disposeSession: async reason => {
|
||||
received.push(reason);
|
||||
},
|
||||
});
|
||||
|
||||
await teardown();
|
||||
|
||||
expect(received).toEqual([undefined]);
|
||||
});
|
||||
|
||||
it("first call's reason wins: a later caller with a different reason awaits the same promise", async () => {
|
||||
const received: Array<postmortem.Reason | undefined> = [];
|
||||
const release = Promise.withResolvers<void>();
|
||||
|
||||
const teardown = createSessionTeardown({
|
||||
getDraftText: () => "",
|
||||
beginDispose: () => {},
|
||||
saveDraft: async () => {},
|
||||
disposeSession: async reason => {
|
||||
received.push(reason);
|
||||
await release.promise;
|
||||
},
|
||||
});
|
||||
|
||||
// SIGTERM lands first; postmortem.quit(0)'s MANUAL pass arrives while the
|
||||
// teardown is still draining — it must not restart the run or mutate the
|
||||
// recorded reason.
|
||||
const first = teardown(postmortem.Reason.SIGTERM);
|
||||
const second = teardown(postmortem.Reason.MANUAL);
|
||||
release.resolve();
|
||||
await Promise.all([first, second]);
|
||||
|
||||
expect(received).toEqual([postmortem.Reason.SIGTERM]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
* Extracted (rather than inlined into `InteractiveMode`) so the callback body
|
||||
* is directly unit-testable without instantiating the full TUI stack.
|
||||
*/
|
||||
import { logger } from "@oh-my-pi/pi-utils";
|
||||
import { logger, type postmortem } from "@oh-my-pi/pi-utils";
|
||||
|
||||
/** Dependencies the teardown captures at construction time. */
|
||||
export interface SessionTeardownDeps {
|
||||
@@ -26,12 +26,22 @@ export interface SessionTeardownDeps {
|
||||
* previously-persisted draft sidecar is cleared on a clean exit.
|
||||
*/
|
||||
saveDraft: (text: string) => Promise<void>;
|
||||
/** Dispose the session — emits `session_shutdown`, drains async jobs, closes the manager. */
|
||||
disposeSession: () => Promise<void>;
|
||||
/**
|
||||
* Dispose the session — emits `session_shutdown`, drains async jobs, closes
|
||||
* the manager. Receives the postmortem reason that triggered the teardown
|
||||
* (undefined on the keypress/`/exit` path) so `AgentSession.dispose()` can
|
||||
* persist the real exit reason instead of the generic `"dispose"`.
|
||||
*/
|
||||
disposeSession: (reason?: postmortem.Reason) => Promise<void>;
|
||||
}
|
||||
|
||||
/** Idempotent teardown: concurrent/repeat invocations share one settled promise. */
|
||||
export type SessionTeardown = () => Promise<void>;
|
||||
/**
|
||||
* Idempotent teardown: concurrent/repeat invocations share one settled
|
||||
* promise. The optional `reason` is the postmortem reason that triggered the
|
||||
* teardown (`sigterm`, `sighup`, `uncaught_exception`, …); only the FIRST
|
||||
* call's reason is used — later callers await the same settled promise.
|
||||
*/
|
||||
export type SessionTeardown = (reason?: postmortem.Reason) => Promise<void>;
|
||||
|
||||
/**
|
||||
* Build a promise-memoized teardown function. The first call snapshots the
|
||||
@@ -42,13 +52,20 @@ export type SessionTeardown = () => Promise<void>;
|
||||
* double-emit `session_shutdown`, double-dispose the session's async-job
|
||||
* manager, or race each other.
|
||||
*
|
||||
* The postmortem callback forwards its `Reason` so the persisted
|
||||
* `session_exit` diagnostic carries the real trigger (`sigterm`, `sighup`,
|
||||
* `uncaught_exception`, …) instead of the generic `"dispose"` that plain
|
||||
* programmatic disposal records. First call wins: a signal arriving after a
|
||||
* keypress-initiated teardown awaits the in-flight promise and its reason is
|
||||
* dropped — by then the exit entry is already being written as a normal exit.
|
||||
*
|
||||
* `saveDraft` failures are logged but never abort the disposal chain — a
|
||||
* draft-write error must not leak background bash/task jobs or skip the
|
||||
* extension `session_shutdown` event.
|
||||
*/
|
||||
export function createSessionTeardown(deps: SessionTeardownDeps): SessionTeardown {
|
||||
let pending: Promise<void> | undefined;
|
||||
const run = async (): Promise<void> => {
|
||||
const run = async (reason?: postmortem.Reason): Promise<void> => {
|
||||
const draftText = deps.getDraftText();
|
||||
deps.beginDispose();
|
||||
try {
|
||||
@@ -56,10 +73,10 @@ export function createSessionTeardown(deps: SessionTeardownDeps): SessionTeardow
|
||||
} catch (err) {
|
||||
logger.warn("Failed to save session draft during teardown", { error: String(err) });
|
||||
}
|
||||
await deps.disposeSession();
|
||||
await deps.disposeSession(reason);
|
||||
};
|
||||
return () => {
|
||||
if (!pending) pending = run();
|
||||
return (reason?: postmortem.Reason) => {
|
||||
if (!pending) pending = run(reason);
|
||||
return pending;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -458,6 +458,13 @@ export const SHUTDOWN_CONSOLIDATE_BUDGET_MS = 1_500;
|
||||
|
||||
export interface AgentSessionDisposeOptions {
|
||||
mnemopiConsolidateTimeoutMs?: number;
|
||||
/**
|
||||
* Postmortem reason that triggered this dispose (signal/fatal teardown
|
||||
* paths). When set, the persisted `session_exit` diagnostic records it
|
||||
* instead of the generic `"dispose"` used for normal programmatic disposal
|
||||
* (`/quit`, test teardown, subagent completion).
|
||||
*/
|
||||
reason?: postmortem.Reason;
|
||||
}
|
||||
|
||||
type CompactionCheckResult = Readonly<{
|
||||
@@ -5343,7 +5350,7 @@ export class AgentSession {
|
||||
|
||||
async #doDispose(options: AgentSessionDisposeOptions = {}): Promise<void> {
|
||||
this.beginDispose();
|
||||
this.#recordSessionExit("dispose");
|
||||
this.#recordSessionExit(options.reason ?? "dispose");
|
||||
this.#cancelExitRecorder?.();
|
||||
this.#cancelExitRecorder = undefined;
|
||||
try {
|
||||
|
||||
@@ -814,6 +814,17 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
const startedAt = Date.now();
|
||||
const semaphore = this.#getSpawnSemaphore();
|
||||
let semaphoreHeld = false;
|
||||
// Every release funnels through here: the flag flips before the
|
||||
// release so no path — acquire-time abort, executor failure, or a
|
||||
// future refactor that reorders the branches — can return a permit
|
||||
// twice. Releasing a permit this job never acquired would steal one
|
||||
// from a running job and let a later spawn start past
|
||||
// task.maxConcurrency.
|
||||
const releasePermit = () => {
|
||||
if (!semaphoreHeld) return;
|
||||
semaphoreHeld = false;
|
||||
this.#releaseSpawnSemaphore();
|
||||
};
|
||||
try {
|
||||
await semaphore.acquire(runSignal);
|
||||
semaphoreHeld = true;
|
||||
@@ -825,7 +836,7 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
}
|
||||
const acquiredAt = Date.now();
|
||||
if (!semaphoreHeld || runSignal.aborted) {
|
||||
if (semaphoreHeld) this.#releaseSpawnSemaphore();
|
||||
releasePermit();
|
||||
progress.status = "aborted";
|
||||
onSettled?.(true);
|
||||
throw new Error("Aborted before execution");
|
||||
@@ -897,7 +908,7 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
const hint = AgentRegistry.global().get(agentId) ? buildFollowUpHint(false) : "";
|
||||
throw new TaskJobError(`${message}${hint}`);
|
||||
} finally {
|
||||
this.#releaseSpawnSemaphore();
|
||||
releasePermit();
|
||||
}
|
||||
},
|
||||
{
|
||||
|
||||
@@ -305,16 +305,13 @@ export async function applyNestedPatches(
|
||||
}
|
||||
} finally {
|
||||
if (stashed) {
|
||||
try {
|
||||
await git.stash.pop(nestedDir, { index: true });
|
||||
} catch (popErr) {
|
||||
const message = popErr instanceof Error ? popErr.message : String(popErr);
|
||||
const restored = await git.stash.tryPop(nestedDir, { index: true });
|
||||
if (!restored) {
|
||||
logger.warn("Pre-existing nested-repo dirty state could not be auto-restored", {
|
||||
nestedDir,
|
||||
error: message,
|
||||
});
|
||||
warnings.push(
|
||||
`Pre-existing dirty state in nested repo \`${relativePath}\` could not be auto-restored after the agent commit; stash entry preserved (${message}).`,
|
||||
`Pre-existing dirty state in nested repo \`${relativePath}\` could not be auto-restored after the agent commit; stash entry preserved.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -755,13 +752,15 @@ export async function mergeTaskBranches(
|
||||
}
|
||||
} finally {
|
||||
if (didStash) {
|
||||
try {
|
||||
await git.stash.pop(repoRoot, { index: true });
|
||||
} catch {
|
||||
// Stash-pop conflicts mean the replayed changes clash with the user's
|
||||
// uncommitted edits. The cherry-picked commits are already on HEAD, so
|
||||
// the merged branches DID land — report them as merged and surface the
|
||||
// stash conflict separately instead of claiming they are unmerged.
|
||||
const restored = await git.stash.tryPop(repoRoot, { index: true });
|
||||
if (!restored) {
|
||||
// Stash pop would leave stage 1/2/3 unmerged entries in `.git/index`
|
||||
// that overlay-isolated subsequent tasks inherit through the lower
|
||||
// layer, corrupting every downstream `captureRepoDeltaPatch`. `tryPop`
|
||||
// short-circuits the pop when the WIP would conflict with the
|
||||
// cherry-picked HEAD (and reset-cleans up if a rarer conflict slips
|
||||
// past). The merged branches DID land — surface a stash-restore
|
||||
// warning without claiming the merge failed.
|
||||
logger.warn("Failed to restore stashed changes after task merge; stash entry preserved");
|
||||
const stashConflict =
|
||||
"stash pop: cherry-picked changes conflict with uncommitted edits. The merged commits are on HEAD; run `git stash pop` and resolve manually.";
|
||||
|
||||
@@ -27,7 +27,7 @@ import { finalizeOutput, loadPage, looksLikeHtml, MAX_BYTES, MAX_OUTPUT_CHARS }
|
||||
import { convertWithMarkit, fetchBinary } from "../web/scrapers/utils";
|
||||
import { applyListLimit } from "./list-limit";
|
||||
import { formatStyledArtifactReference, type OutputMeta } from "./output-meta";
|
||||
import { type LineRange, parseLineRanges } from "./path-utils";
|
||||
import { isReadableUrlPath, type LineRange, parseLineRanges } from "./path-utils";
|
||||
import { formatBytes, formatExpandHint, getDomain, replaceTabs } from "./render-utils";
|
||||
import { listTables, looksLikeSqlite, renderTableList } from "./sqlite-reader";
|
||||
import { ToolAbortError, ToolError } from "./tool-errors";
|
||||
@@ -145,10 +145,6 @@ function normalizeUrl(url: string): string {
|
||||
return url;
|
||||
}
|
||||
|
||||
export function isReadableUrlPath(value: string): boolean {
|
||||
return /^https?:\/\/?/i.test(value) || /^www\./i.test(value);
|
||||
}
|
||||
|
||||
// URL line selectors mirror the file form: `:50`, `:50-100`, `:50+150`, `:5-10,20-30`, `:raw`,
|
||||
// or `:raw:N-M` / `:N-M:raw` to combine raw mode with a range. If a URL would otherwise look
|
||||
// like `host:port`, add a trailing slash before the selector (e.g. `https://example.com/:80`
|
||||
|
||||
@@ -139,14 +139,38 @@ interface SectionLabelMetadata {
|
||||
isKnown(label: string): boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive incremental-label metadata from top-level schema closure.
|
||||
*
|
||||
* The unknown-label gate (`rejectUnknownSections`) engages when the schema constrains top-level
|
||||
* property names anywhere: a closed conjunct (root or recursive `allOf` child with
|
||||
* `additionalProperties: false`) or a `oneOf`/`anyOf` union whose EVERY variant is closed. A label
|
||||
* is known iff every closed conjunct accepts it AND, per closed union, at least one variant
|
||||
* accepts it (union semantics are disjunctive — the assembled output only has to match one
|
||||
* variant). Unions containing any open variant never gate: the open variant accepts arbitrary
|
||||
* labels, so rejection would be a false positive.
|
||||
*/
|
||||
function buildSectionLabelMetadata(jsonSchema: Record<string, unknown>): SectionLabelMetadata {
|
||||
const closedSchemas = collectClosedTopLevelSchemas(jsonSchema);
|
||||
const labels = [...new Set(closedSchemas.flatMap(schema => declaredPropertyLabels(schema)))];
|
||||
const closedConjuncts = collectClosedTopLevelSchemas(jsonSchema);
|
||||
const closedUnions = collectClosedTopLevelUnions(jsonSchema);
|
||||
const closed = closedConjuncts.length > 0 || closedUnions.length > 0;
|
||||
const acceptedByAll = (conjuncts: readonly Record<string, unknown>[], label: string): boolean =>
|
||||
conjuncts.every(schema => schemaAcceptsSectionLabel(schema, label));
|
||||
const labels = [
|
||||
...new Set([
|
||||
...closedConjuncts.flatMap(schema => declaredPropertyLabels(schema)),
|
||||
...closedUnions.flatMap(variants =>
|
||||
variants.flatMap(conjuncts => conjuncts.flatMap(schema => declaredPropertyLabels(schema))),
|
||||
),
|
||||
]),
|
||||
];
|
||||
return {
|
||||
labels,
|
||||
rejectUnknownSections: closedSchemas.length > 0,
|
||||
rejectUnknownSections: closed,
|
||||
isKnown: label =>
|
||||
closedSchemas.length === 0 || closedSchemas.every(schema => schemaAcceptsSectionLabel(schema, label)),
|
||||
!closed ||
|
||||
(acceptedByAll(closedConjuncts, label) &&
|
||||
closedUnions.every(variants => variants.some(conjuncts => acceptedByAll(conjuncts, label)))),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -164,6 +188,43 @@ function collectClosedTopLevelSchemas(jsonSchema: Record<string, unknown>): Reco
|
||||
return schemas;
|
||||
}
|
||||
|
||||
/** One fully-closed `oneOf`/`anyOf` union: per variant, that variant's closed conjunct schemas. */
|
||||
type ClosedUnionVariants = Record<string, unknown>[][];
|
||||
|
||||
/**
|
||||
* Collect top-level `oneOf`/`anyOf` unions in which EVERY variant is closed — i.e. each variant
|
||||
* (or one of its `allOf` conjuncts, resolved via `collectClosedTopLevelSchemas`) carries
|
||||
* `additionalProperties: false`. JTD discriminator output schemas compile to exactly this shape:
|
||||
* a root `oneOf` of closed object variants. Unions with any open (or non-object) variant are
|
||||
* skipped entirely so the unknown-label gate cannot fire false rejections. Unions nested under
|
||||
* `allOf` conjuncts gate identically (intersection semantics).
|
||||
*/
|
||||
function collectClosedTopLevelUnions(jsonSchema: Record<string, unknown>): ClosedUnionVariants[] {
|
||||
const unions: ClosedUnionVariants[] = [];
|
||||
for (const key of ["oneOf", "anyOf"] as const) {
|
||||
const rawVariants = jsonSchema[key];
|
||||
if (!Array.isArray(rawVariants) || rawVariants.length === 0) continue;
|
||||
const variants: ClosedUnionVariants = [];
|
||||
let allClosed = true;
|
||||
for (const raw of rawVariants) {
|
||||
const conjuncts = isRecord(raw) ? collectClosedTopLevelSchemas(raw) : [];
|
||||
if (conjuncts.length === 0) {
|
||||
allClosed = false;
|
||||
break;
|
||||
}
|
||||
variants.push(conjuncts);
|
||||
}
|
||||
if (allClosed) unions.push(variants);
|
||||
}
|
||||
const allOf = jsonSchema.allOf;
|
||||
if (Array.isArray(allOf)) {
|
||||
for (const raw of allOf) {
|
||||
if (isRecord(raw)) unions.push(...collectClosedTopLevelUnions(raw));
|
||||
}
|
||||
}
|
||||
return unions;
|
||||
}
|
||||
|
||||
function declaredPropertyLabels(jsonSchema: Record<string, unknown>): string[] {
|
||||
const properties = jsonSchema.properties;
|
||||
if (properties === null || typeof properties !== "object" || Array.isArray(properties)) return [];
|
||||
|
||||
@@ -2,7 +2,7 @@ import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import * as url from "node:url";
|
||||
import { isEnoent, stripWindowsExtendedLengthPathPrefix } from "@oh-my-pi/pi-utils";
|
||||
import { isEnoent, isEnotdir, stripWindowsExtendedLengthPathPrefix } from "@oh-my-pi/pi-utils";
|
||||
import type { Skill } from "../extensibility/skills";
|
||||
import { InternalUrlRouter, type LocalProtocolOptions } from "../internal-urls";
|
||||
import { ToolError } from "./tool-errors";
|
||||
@@ -435,6 +435,16 @@ export function isSshUrl(path: string): boolean {
|
||||
return /^ssh:\/\//i.test(path.trim());
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the read tool's URL parser (`parseReadUrlTarget` in fetch.ts) would
|
||||
* recognize this path as a readable external URL: a strict `http(s)://`, a
|
||||
* collapsed `http(s):/host` (Node path normalization folds `//` → `/`), or a
|
||||
* scheme-less `www.` spelling. Keep in sync with `parseReadUrlTarget`.
|
||||
*/
|
||||
export function isReadableUrlPath(value: string): boolean {
|
||||
return /^https?:\/\/?/i.test(value) || /^www\./i.test(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a path relative to the given cwd.
|
||||
* Handles ~ expansion and absolute paths.
|
||||
@@ -1090,14 +1100,30 @@ export async function resolveToolSearchScope(opts: ToolScopeOptions): Promise<To
|
||||
if (rawPaths.some(rawPath => rawPath.length === 0)) {
|
||||
throw new ToolError("`paths` must contain non-empty paths or globs");
|
||||
}
|
||||
const externalUrlRe = /^(?:https?|ftp|file|ws|wss):\/\//i;
|
||||
// Strict external-URL schemes. `file://` is intentionally absent: it has
|
||||
// local-path semantics (expandPath strips it downstream), so it flows through
|
||||
// the ordinary filesystem pipeline instead of the external-URL resolver.
|
||||
const strictExternalUrlRe = /^(?:https?|ftp|ws|wss):\/\//i;
|
||||
const internalRouter = InternalUrlRouter.instance();
|
||||
const resolvedPathInputs: string[] = [];
|
||||
const immutableSourcePaths = new Set<string>();
|
||||
for (const rawPath of rawPaths) {
|
||||
const externalUrl = externalUrlRe.test(rawPath);
|
||||
if (externalUrl && opts.resolveExternalUrl) {
|
||||
const resolved = await opts.resolveExternalUrl(rawPath);
|
||||
let externalUrl = strictExternalUrlRe.test(rawPath);
|
||||
if (!externalUrl && isReadableUrlPath(rawPath) && !hasGlobPathChars(rawPath)) {
|
||||
// Fuzzy spelling the read parser accepts (`www.host/…`, collapsed
|
||||
// `https:/host/…`). An existing local path wins over URL
|
||||
// interpretation so a directory literally named `www.foo` stays
|
||||
// searchable; only a definitive ENOENT/ENOTDIR flips to URL handling
|
||||
// (any other stat error means the path exists — let the local
|
||||
// pipeline surface it).
|
||||
try {
|
||||
await fs.promises.stat(resolveToCwd(rawPath, cwd));
|
||||
} catch (err) {
|
||||
externalUrl = isEnoent(err) || isEnotdir(err);
|
||||
}
|
||||
}
|
||||
if (externalUrl) {
|
||||
const resolved = opts.resolveExternalUrl ? await opts.resolveExternalUrl(rawPath) : undefined;
|
||||
if (resolved) {
|
||||
resolvedPathInputs.push(resolved.sourcePath);
|
||||
if (opts.trackImmutableSources && resolved.immutable) {
|
||||
@@ -1105,6 +1131,12 @@ export async function resolveToolSearchScope(opts: ToolScopeOptions): Promise<To
|
||||
}
|
||||
continue;
|
||||
}
|
||||
// Resolver missing or declined (e.g. ftp/ws/wss): fail explicitly
|
||||
// instead of letting the local-path fallthrough surface a confusing
|
||||
// "Path not found" for a URL-shaped input.
|
||||
throw new ToolError(
|
||||
`Cannot ${internalUrlAction} external URL: ${rawPath}. Use \`read\` to fetch web content, then search the returned text.`,
|
||||
);
|
||||
}
|
||||
if (!internalRouter.canHandle(rawPath)) {
|
||||
resolvedPathInputs.push(rawPath);
|
||||
|
||||
@@ -77,7 +77,6 @@ import {
|
||||
} from "./conflict-detect";
|
||||
import {
|
||||
executeReadUrl,
|
||||
isReadableUrlPath,
|
||||
loadReadUrlCacheEntry,
|
||||
parseReadUrlTarget,
|
||||
type ReadUrlToolDetails,
|
||||
@@ -95,6 +94,7 @@ import {
|
||||
import {
|
||||
expandPath,
|
||||
formatPathRelativeToCwd,
|
||||
isReadableUrlPath,
|
||||
type LineRange,
|
||||
parseLineRanges,
|
||||
pathTargetsSsh,
|
||||
|
||||
@@ -390,8 +390,9 @@ export class YieldTool implements AgentTool<TSchema, YieldDetails> {
|
||||
}
|
||||
|
||||
/**
|
||||
* Return incremental yield labels that are not declared as top-level properties of a closed
|
||||
* caller schema. Open schemas (no `additionalProperties: false`) accept any label.
|
||||
* Return incremental yield labels the closed caller schema does not accept. Closure covers the
|
||||
* root, `allOf` conjuncts, and `oneOf`/`anyOf` unions whose every variant is closed (e.g. JTD
|
||||
* discriminators). Open schemas accept any label.
|
||||
*/
|
||||
#unknownIncrementalLabels(labels: string[]): string[] {
|
||||
if (!this.#rejectUnknownSections) return [];
|
||||
|
||||
@@ -1750,6 +1750,69 @@ export const stash = {
|
||||
if (options?.index) args.push("--index");
|
||||
await runEffect(cwd, args);
|
||||
},
|
||||
/**
|
||||
* Return the working-tree patch that `stash@{0}` would apply, in a form
|
||||
* that `git apply --check` can consume. Empty string when no stash entry
|
||||
* exists or the stash contains no diffable working-tree changes.
|
||||
*/
|
||||
async showPatch(cwd: string): Promise<string> {
|
||||
return (await tryText(cwd, ["stash", "show", "-p", "--binary", "stash@{0}"], { readOnly: true })) ?? "";
|
||||
},
|
||||
/** Return untracked paths stored in the top stash entry. */
|
||||
async untrackedFiles(cwd: string): Promise<string[]> {
|
||||
const output = await tryText(cwd, ["ls-tree", "-r", "-z", "--name-only", "stash@{0}^3"], { readOnly: true });
|
||||
return output?.split("\0").filter(Boolean) ?? [];
|
||||
},
|
||||
/**
|
||||
* Attempt to restore the top stash entry. On success returns `true` and
|
||||
* git drops the stash entry. On conflict returns `false`, leaves the stash
|
||||
* entry preserved for manual resolution, and guarantees the failed restore
|
||||
* leaves no unmerged index entries or partially-restored untracked files.
|
||||
*
|
||||
* The historical raw `pop` catches the failure in a `finally` block and
|
||||
* only logs — it leaves `.git/index` with stage 1/2/3 unmerged entries
|
||||
* that survive indefinitely, corrupting every subsequent overlay-isolated
|
||||
* task that reads through this repo's `.git/`. See issue #4175.
|
||||
*/
|
||||
async tryPop(cwd: string, options?: { index?: boolean }): Promise<boolean> {
|
||||
// Preflight: `git stash pop` internally does a 3-way merge, so a plain
|
||||
// `git apply --check` is too strict — it rejects hunks whose context
|
||||
// drifted from HEAD even when 3-way merge would resolve them cleanly.
|
||||
// Match pop's semantics with `--3way --check`, which succeeds iff the
|
||||
// patch either applies directly or merges without conflict against
|
||||
// the patch's `index abc..def` base blobs.
|
||||
const workingPatch = await stash.showPatch(cwd);
|
||||
if (workingPatch.trim() && !(await patch.canApplyText(cwd, workingPatch, { threeWay: true }))) {
|
||||
return false;
|
||||
}
|
||||
const restoredUntracked = await stash.untrackedFiles(cwd);
|
||||
try {
|
||||
await stash.pop(cwd, options);
|
||||
return true;
|
||||
} catch {
|
||||
// Preflight can still miss mode-only or delete/modify conflicts. If
|
||||
// the pop left unmerged entries, wipe them: HEAD holds the merged
|
||||
// state so `reset --hard HEAD` restores a clean index and working
|
||||
// tree without losing the cherry-picked commits. A failed pop can
|
||||
// still restore unrelated untracked files before exiting while
|
||||
// preserving the stash entry, so clean only the untracked paths
|
||||
// recorded in that stash. The user's WIP remains recoverable via
|
||||
// `git stash pop`.
|
||||
try {
|
||||
await reset(cwd, { hard: true });
|
||||
} catch {
|
||||
/* best-effort cleanup — do not mask the primary conflict */
|
||||
}
|
||||
if (restoredUntracked.length > 0) {
|
||||
try {
|
||||
await clean(cwd, { includeIgnored: true, literalPathspecs: true, paths: restoredUntracked });
|
||||
} catch {
|
||||
/* best-effort cleanup — do not mask the primary conflict */
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════════
|
||||
@@ -1819,9 +1882,18 @@ export async function reset(
|
||||
|
||||
export async function clean(
|
||||
cwd: string,
|
||||
options: { ignoredOnly?: boolean; paths?: readonly string[]; signal?: AbortSignal } = {},
|
||||
options: {
|
||||
ignoredOnly?: boolean;
|
||||
includeIgnored?: boolean;
|
||||
literalPathspecs?: boolean;
|
||||
paths?: readonly string[];
|
||||
signal?: AbortSignal;
|
||||
} = {},
|
||||
): Promise<void> {
|
||||
const args = ["clean", options.ignoredOnly ? "-fdX" : "-fd"];
|
||||
const args = [options.literalPathspecs ? "--literal-pathspecs" : undefined, "clean"].filter(
|
||||
(arg): arg is string => arg !== undefined,
|
||||
);
|
||||
args.push(options.ignoredOnly ? "-fdX" : options.includeIgnored ? "-fdx" : "-fd");
|
||||
if (options.paths?.length) args.push("--", ...options.paths);
|
||||
await runEffect(cwd, args, { signal: options.signal });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user