mirror of
https://github.com/earendil-works/pi.git
synced 2026-06-18 15:54:04 +08:00
refactor(coding-agent): simplify project trust changes
This commit is contained in:
@@ -822,7 +822,6 @@ export class DefaultPackageManager implements PackageManager {
|
||||
}
|
||||
|
||||
getInstalledPath(source: string, scope: "user" | "project"): string | undefined {
|
||||
this.assertProjectPiTrustedForScope(scope);
|
||||
const parsed = this.parseSource(source);
|
||||
if (parsed.type === "npm") {
|
||||
const path = this.getNpmInstallPath(parsed, scope);
|
||||
@@ -1293,7 +1292,6 @@ export class DefaultPackageManager implements PackageManager {
|
||||
}
|
||||
|
||||
private async installParsedSource(parsed: ParsedSource, scope: SourceScope): Promise<void> {
|
||||
this.assertProjectPiTrustedForScope(scope);
|
||||
if (parsed.type === "npm") {
|
||||
await this.installNpm(parsed, scope, scope === "temporary");
|
||||
return;
|
||||
|
||||
@@ -165,8 +165,6 @@ export interface SettingsManagerCreateOptions {
|
||||
|
||||
export interface SettingsStorage {
|
||||
withLock(scope: SettingsScope, fn: (current: string | undefined) => string | undefined): void;
|
||||
setProjectConfigTrusted?(trusted: boolean): void;
|
||||
isProjectConfigTrusted?(): boolean;
|
||||
}
|
||||
|
||||
export interface SettingsError {
|
||||
@@ -177,22 +175,12 @@ export interface SettingsError {
|
||||
export class FileSettingsStorage implements SettingsStorage {
|
||||
private globalSettingsPath: string;
|
||||
private projectSettingsPath: string;
|
||||
private projectConfigTrusted: boolean;
|
||||
|
||||
constructor(cwd: string, agentDir: string, options: SettingsManagerCreateOptions = {}) {
|
||||
constructor(cwd: string, agentDir: string) {
|
||||
const resolvedCwd = resolvePath(cwd);
|
||||
const resolvedAgentDir = resolvePath(agentDir);
|
||||
this.globalSettingsPath = join(resolvedAgentDir, "settings.json");
|
||||
this.projectSettingsPath = join(resolvedCwd, CONFIG_DIR_NAME, "settings.json");
|
||||
this.projectConfigTrusted = options.projectConfigTrusted ?? true;
|
||||
}
|
||||
|
||||
setProjectConfigTrusted(trusted: boolean): void {
|
||||
this.projectConfigTrusted = trusted;
|
||||
}
|
||||
|
||||
isProjectConfigTrusted(): boolean {
|
||||
return this.projectConfigTrusted;
|
||||
}
|
||||
|
||||
private acquireLockSyncWithRetry(path: string): () => void {
|
||||
@@ -223,10 +211,6 @@ export class FileSettingsStorage implements SettingsStorage {
|
||||
}
|
||||
|
||||
withLock(scope: SettingsScope, fn: (current: string | undefined) => string | undefined): void {
|
||||
if (scope === "project" && !this.projectConfigTrusted) {
|
||||
throw new Error("Project .pi is not trusted; refusing to access project settings");
|
||||
}
|
||||
|
||||
const path = scope === "global" ? this.globalSettingsPath : this.projectSettingsPath;
|
||||
const dir = dirname(path);
|
||||
|
||||
@@ -260,21 +244,8 @@ export class FileSettingsStorage implements SettingsStorage {
|
||||
export class InMemorySettingsStorage implements SettingsStorage {
|
||||
private global: string | undefined;
|
||||
private project: string | undefined;
|
||||
private projectConfigTrusted = true;
|
||||
|
||||
setProjectConfigTrusted(trusted: boolean): void {
|
||||
this.projectConfigTrusted = trusted;
|
||||
}
|
||||
|
||||
isProjectConfigTrusted(): boolean {
|
||||
return this.projectConfigTrusted;
|
||||
}
|
||||
|
||||
withLock(scope: SettingsScope, fn: (current: string | undefined) => string | undefined): void {
|
||||
if (scope === "project" && !this.projectConfigTrusted) {
|
||||
throw new Error("Project .pi is not trusted; refusing to access project settings");
|
||||
}
|
||||
|
||||
const current = scope === "global" ? this.global : this.project;
|
||||
const next = fn(current);
|
||||
if (next !== undefined) {
|
||||
@@ -327,15 +298,15 @@ export class SettingsManager {
|
||||
agentDir: string = getAgentDir(),
|
||||
options: SettingsManagerCreateOptions = {},
|
||||
): SettingsManager {
|
||||
const storage = new FileSettingsStorage(cwd, agentDir, options);
|
||||
return SettingsManager.fromStorage(storage);
|
||||
const storage = new FileSettingsStorage(cwd, agentDir);
|
||||
return SettingsManager.fromStorage(storage, options);
|
||||
}
|
||||
|
||||
/** Create a SettingsManager from an arbitrary storage backend */
|
||||
static fromStorage(storage: SettingsStorage): SettingsManager {
|
||||
const projectConfigTrusted = storage.isProjectConfigTrusted?.() ?? true;
|
||||
static fromStorage(storage: SettingsStorage, options: SettingsManagerCreateOptions = {}): SettingsManager {
|
||||
const projectConfigTrusted = options.projectConfigTrusted ?? true;
|
||||
const globalLoad = SettingsManager.tryLoadFromStorage(storage, "global");
|
||||
const projectLoad = SettingsManager.tryLoadFromStorage(storage, "project");
|
||||
const projectLoad = SettingsManager.tryLoadFromStorage(storage, "project", projectConfigTrusted);
|
||||
const initialErrors: SettingsError[] = [];
|
||||
if (globalLoad.error) {
|
||||
initialErrors.push({ scope: "global", error: globalLoad.error });
|
||||
@@ -363,8 +334,12 @@ export class SettingsManager {
|
||||
return SettingsManager.fromStorage(storage);
|
||||
}
|
||||
|
||||
private static loadFromStorage(storage: SettingsStorage, scope: SettingsScope): Settings {
|
||||
if (scope === "project" && storage.isProjectConfigTrusted?.() === false) {
|
||||
private static loadFromStorage(
|
||||
storage: SettingsStorage,
|
||||
scope: SettingsScope,
|
||||
projectConfigTrusted = true,
|
||||
): Settings {
|
||||
if (scope === "project" && !projectConfigTrusted) {
|
||||
return {};
|
||||
}
|
||||
|
||||
@@ -384,9 +359,10 @@ export class SettingsManager {
|
||||
private static tryLoadFromStorage(
|
||||
storage: SettingsStorage,
|
||||
scope: SettingsScope,
|
||||
projectConfigTrusted = true,
|
||||
): { settings: Settings; error: Error | null } {
|
||||
try {
|
||||
return { settings: SettingsManager.loadFromStorage(storage, scope), error: null };
|
||||
return { settings: SettingsManager.loadFromStorage(storage, scope, projectConfigTrusted), error: null };
|
||||
} catch (error) {
|
||||
return { settings: {}, error: error as Error };
|
||||
}
|
||||
@@ -468,7 +444,6 @@ export class SettingsManager {
|
||||
|
||||
setProjectConfigTrusted(trusted: boolean): void {
|
||||
this.projectConfigTrusted = trusted;
|
||||
this.storage.setProjectConfigTrusted?.(trusted);
|
||||
if (!trusted) {
|
||||
this.projectSettings = {};
|
||||
this.projectSettingsLoadError = null;
|
||||
@@ -494,7 +469,7 @@ export class SettingsManager {
|
||||
this.modifiedProjectFields.clear();
|
||||
this.modifiedProjectNestedFields.clear();
|
||||
|
||||
const projectLoad = SettingsManager.tryLoadFromStorage(this.storage, "project");
|
||||
const projectLoad = SettingsManager.tryLoadFromStorage(this.storage, "project", this.projectConfigTrusted);
|
||||
if (!projectLoad.error) {
|
||||
this.projectSettings = projectLoad.settings;
|
||||
this.projectSettingsLoadError = null;
|
||||
@@ -558,6 +533,9 @@ export class SettingsManager {
|
||||
private enqueueWrite(scope: SettingsScope, task: () => void): void {
|
||||
this.writeQueue = this.writeQueue
|
||||
.then(() => {
|
||||
if (scope === "project") {
|
||||
this.assertProjectConfigTrustedForWrite();
|
||||
}
|
||||
task();
|
||||
this.clearModifiedScope(scope);
|
||||
})
|
||||
@@ -638,6 +616,14 @@ export class SettingsManager {
|
||||
});
|
||||
}
|
||||
|
||||
private updateProjectSettings(field: keyof Settings, update: (settings: Settings) => void): void {
|
||||
this.assertProjectConfigTrustedForWrite();
|
||||
const projectSettings = structuredClone(this.projectSettings);
|
||||
update(projectSettings);
|
||||
this.markProjectModified(field);
|
||||
this.saveProjectSettings(projectSettings);
|
||||
}
|
||||
|
||||
async flush(): Promise<void> {
|
||||
await this.writeQueue;
|
||||
}
|
||||
@@ -908,11 +894,9 @@ export class SettingsManager {
|
||||
}
|
||||
|
||||
setProjectPackages(packages: PackageSource[]): void {
|
||||
this.assertProjectConfigTrustedForWrite();
|
||||
const projectSettings = structuredClone(this.projectSettings);
|
||||
projectSettings.packages = packages;
|
||||
this.markProjectModified("packages");
|
||||
this.saveProjectSettings(projectSettings);
|
||||
this.updateProjectSettings("packages", (settings) => {
|
||||
settings.packages = packages;
|
||||
});
|
||||
}
|
||||
|
||||
getExtensionPaths(): string[] {
|
||||
@@ -926,11 +910,9 @@ export class SettingsManager {
|
||||
}
|
||||
|
||||
setProjectExtensionPaths(paths: string[]): void {
|
||||
this.assertProjectConfigTrustedForWrite();
|
||||
const projectSettings = structuredClone(this.projectSettings);
|
||||
projectSettings.extensions = paths;
|
||||
this.markProjectModified("extensions");
|
||||
this.saveProjectSettings(projectSettings);
|
||||
this.updateProjectSettings("extensions", (settings) => {
|
||||
settings.extensions = paths;
|
||||
});
|
||||
}
|
||||
|
||||
getSkillPaths(): string[] {
|
||||
@@ -944,11 +926,9 @@ export class SettingsManager {
|
||||
}
|
||||
|
||||
setProjectSkillPaths(paths: string[]): void {
|
||||
this.assertProjectConfigTrustedForWrite();
|
||||
const projectSettings = structuredClone(this.projectSettings);
|
||||
projectSettings.skills = paths;
|
||||
this.markProjectModified("skills");
|
||||
this.saveProjectSettings(projectSettings);
|
||||
this.updateProjectSettings("skills", (settings) => {
|
||||
settings.skills = paths;
|
||||
});
|
||||
}
|
||||
|
||||
getPromptTemplatePaths(): string[] {
|
||||
@@ -962,11 +942,9 @@ export class SettingsManager {
|
||||
}
|
||||
|
||||
setProjectPromptTemplatePaths(paths: string[]): void {
|
||||
this.assertProjectConfigTrustedForWrite();
|
||||
const projectSettings = structuredClone(this.projectSettings);
|
||||
projectSettings.prompts = paths;
|
||||
this.markProjectModified("prompts");
|
||||
this.saveProjectSettings(projectSettings);
|
||||
this.updateProjectSettings("prompts", (settings) => {
|
||||
settings.prompts = paths;
|
||||
});
|
||||
}
|
||||
|
||||
getThemePaths(): string[] {
|
||||
@@ -980,11 +958,9 @@ export class SettingsManager {
|
||||
}
|
||||
|
||||
setProjectThemePaths(paths: string[]): void {
|
||||
this.assertProjectConfigTrustedForWrite();
|
||||
const projectSettings = structuredClone(this.projectSettings);
|
||||
projectSettings.themes = paths;
|
||||
this.markProjectModified("themes");
|
||||
this.saveProjectSettings(projectSettings);
|
||||
this.updateProjectSettings("themes", (settings) => {
|
||||
settings.themes = paths;
|
||||
});
|
||||
}
|
||||
|
||||
getEnableSkillCommands(): boolean {
|
||||
|
||||
@@ -4947,9 +4947,10 @@ export class InteractiveMode {
|
||||
return "ask";
|
||||
}
|
||||
|
||||
private async selectTrustDecision(): Promise<ProjectTrustDecision | undefined> {
|
||||
const trustStore = new ProjectTrustStore(getAgentDir());
|
||||
const cwd = this.sessionManager.getCwd();
|
||||
private async selectTrustDecision(
|
||||
trustStore: ProjectTrustStore,
|
||||
cwd: string,
|
||||
): Promise<ProjectTrustDecision | undefined> {
|
||||
const current = this.formatTrustDecision(trustStore.get(cwd));
|
||||
const choice = await this.showExtensionSelector(
|
||||
`Trust project .pi directory?\nCurrent setting: ${current}\nLoad .pi from ${cwd}?\nWarning: Project extensions can execute code.`,
|
||||
@@ -4977,10 +4978,12 @@ export class InteractiveMode {
|
||||
return;
|
||||
}
|
||||
|
||||
const trustStore = new ProjectTrustStore(getAgentDir());
|
||||
const cwd = this.sessionManager.getCwd();
|
||||
const rawArg = text === "/trust" ? "" : text.slice("/trust".length).trim().toLowerCase();
|
||||
let decision: ProjectTrustDecision | undefined;
|
||||
if (!rawArg) {
|
||||
decision = await this.selectTrustDecision();
|
||||
decision = await this.selectTrustDecision(trustStore, cwd);
|
||||
} else if (rawArg === "yes") {
|
||||
decision = true;
|
||||
} else if (rawArg === "no") {
|
||||
@@ -4996,8 +4999,6 @@ export class InteractiveMode {
|
||||
return;
|
||||
}
|
||||
|
||||
const trustStore = new ProjectTrustStore(getAgentDir());
|
||||
const cwd = this.sessionManager.getCwd();
|
||||
trustStore.set(cwd, decision);
|
||||
const projectPiTrusted = this.options.forceProjectPiTrust === true || decision === true;
|
||||
this.settingsManager.setProjectConfigTrusted(projectPiTrusted);
|
||||
|
||||
@@ -300,18 +300,6 @@ describe("parseArgs", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("--force flag", () => {
|
||||
test("parses --force flag", () => {
|
||||
const result = parseArgs(["--force"]);
|
||||
expect(result.force).toBe(true);
|
||||
});
|
||||
|
||||
test("parses -f shorthand", () => {
|
||||
const result = parseArgs(["-f"]);
|
||||
expect(result.force).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("--offline flag", () => {
|
||||
test("parses --offline flag", () => {
|
||||
const result = parseArgs(["--offline"]);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -214,60 +214,6 @@ describe("package commands", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("does not treat update --force as a project trust override", async () => {
|
||||
const globalPrefix = join(tempDir, "global-prefix");
|
||||
const selfPackageDir = join(globalPrefix, "lib", "node_modules", "@earendil-works", "pi-coding-agent");
|
||||
const fakeNpmPath = join(tempDir, "fake-npm-record.cjs");
|
||||
const recordPath = join(tempDir, "update-force-records.json");
|
||||
mkdirSync(selfPackageDir, { recursive: true });
|
||||
mkdirSync(join(projectDir, ".pi"), { recursive: true });
|
||||
writeFileSync(
|
||||
fakeNpmPath,
|
||||
`const fs=require("node:fs"),path=require("node:path"),args=process.argv.slice(2),prefix=args[args.indexOf("--prefix")+1];
|
||||
if(args.includes("root")) {
|
||||
console.log(path.join(prefix,"lib","node_modules"));
|
||||
process.exit(0);
|
||||
}
|
||||
const records=fs.existsSync(${JSON.stringify(recordPath)})?JSON.parse(fs.readFileSync(${JSON.stringify(recordPath)},"utf-8")):[];
|
||||
records.push(args);
|
||||
fs.writeFileSync(${JSON.stringify(recordPath)},JSON.stringify(records));
|
||||
`,
|
||||
);
|
||||
writeFileSync(
|
||||
join(agentDir, "settings.json"),
|
||||
JSON.stringify({ npmCommand: [originalExecPath, fakeNpmPath, "--prefix", globalPrefix] }, null, 2),
|
||||
);
|
||||
writeFileSync(
|
||||
join(projectDir, ".pi", "settings.json"),
|
||||
JSON.stringify({ packages: ["npm:@project/pkg"] }, null, 2),
|
||||
);
|
||||
process.env.PI_PACKAGE_DIR = selfPackageDir;
|
||||
Object.defineProperty(process, "execPath", {
|
||||
value: join(selfPackageDir, "dist", "cli.js"),
|
||||
configurable: true,
|
||||
});
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
await expect(main(["update", "--force"])).resolves.toBeUndefined();
|
||||
|
||||
expect(process.exitCode).toBeUndefined();
|
||||
expect(errorSpy).not.toHaveBeenCalled();
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
expect(existsSync(recordPath)).toBe(true);
|
||||
const recordedCalls = JSON.parse(readFileSync(recordPath, "utf-8")) as string[][];
|
||||
expect(recordedCalls.some((args) => args.some((arg) => arg.includes("@project/pkg")))).toBe(false);
|
||||
expect(recordedCalls).toEqual([expect.arrayContaining(["install", "-g", PACKAGE_NAME])]);
|
||||
} finally {
|
||||
logSpy.mockRestore();
|
||||
errorSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("uses global npmCommand and current package name for forced self updates without checking the api", async () => {
|
||||
const globalPrefix = join(tempDir, "global-prefix");
|
||||
const projectPrefix = join(tempDir, "project-prefix");
|
||||
|
||||
@@ -329,19 +329,6 @@ Content`,
|
||||
expect(loader.getSystemPrompt()).toBe("You are a helpful assistant.");
|
||||
});
|
||||
|
||||
it("should skip .pi SYSTEM.md when project .pi is not trusted", async () => {
|
||||
const piDir = join(cwd, ".pi");
|
||||
mkdirSync(piDir, { recursive: true });
|
||||
writeFileSync(join(piDir, "SYSTEM.md"), "Project system prompt.");
|
||||
writeFileSync(join(agentDir, "SYSTEM.md"), "Global system prompt.");
|
||||
const settingsManager = SettingsManager.create(cwd, agentDir, { projectConfigTrusted: false });
|
||||
|
||||
const loader = new DefaultResourceLoader({ cwd, agentDir, settingsManager });
|
||||
await loader.reload();
|
||||
|
||||
expect(loader.getSystemPrompt()).toBe("Global system prompt.");
|
||||
});
|
||||
|
||||
it("should skip .pi resources when project .pi is not trusted", async () => {
|
||||
const piDir = join(cwd, ".pi");
|
||||
const extensionsDir = join(piDir, "extensions");
|
||||
@@ -352,6 +339,8 @@ Content`,
|
||||
mkdirSync(skillDir, { recursive: true });
|
||||
mkdirSync(promptsDir, { recursive: true });
|
||||
mkdirSync(themesDir, { recursive: true });
|
||||
writeFileSync(join(piDir, "SYSTEM.md"), "Project system prompt.");
|
||||
writeFileSync(join(agentDir, "SYSTEM.md"), "Global system prompt.");
|
||||
writeFileSync(join(extensionsDir, "project.ts"), `throw new Error("should not load");`);
|
||||
writeFileSync(
|
||||
join(skillDir, "SKILL.md"),
|
||||
@@ -372,6 +361,7 @@ Project skill content`,
|
||||
const loader = new DefaultResourceLoader({ cwd, agentDir, settingsManager });
|
||||
await loader.reload();
|
||||
|
||||
expect(loader.getSystemPrompt()).toBe("Global system prompt.");
|
||||
expect(loader.getExtensions().extensions).toHaveLength(0);
|
||||
expect(loader.getExtensions().errors).toEqual([]);
|
||||
expect(loader.getSkills().skills.some((skill) => skill.name === "project-skill")).toBe(false);
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import lockfile from "proper-lockfile";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { hasProjectPiDirectory, ProjectTrustStore } from "../src/core/trust-manager.ts";
|
||||
|
||||
@@ -44,29 +43,6 @@ describe("ProjectTrustStore", () => {
|
||||
expect(readFileSync(trustPath, "utf-8")).toBe("{not json");
|
||||
});
|
||||
|
||||
it("does not read trust.json while another process holds the trust lock", () => {
|
||||
const trustPath = join(agentDir, "trust.json");
|
||||
writeFileSync(trustPath, "{partial", "utf-8");
|
||||
const release = lockfile.lockSync(agentDir, { realpath: false, lockfilePath: `${trustPath}.lock` });
|
||||
const store = new ProjectTrustStore(agentDir);
|
||||
|
||||
try {
|
||||
let error: unknown;
|
||||
try {
|
||||
store.get(cwd);
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect((error as { code?: unknown }).code).toBe("ELOCKED");
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
||||
expect(() => store.get(cwd)).toThrow(/Failed to read trust store/);
|
||||
});
|
||||
|
||||
it("detects project .pi directories", () => {
|
||||
expect(hasProjectPiDirectory(cwd)).toBe(false);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user