mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 19:28:46 +01:00
merge: retain owned configuration and resolved Legacy limits
Co-Authored-By: Paperclip <noreply@paperclip.ing> * codex/claude-runner-default: fix(claude): preserve Legacy limits after automatic runner resolution fix(runner): initialize Claude settings under lifetime ownership
This commit is contained in:
commit
dc12ffcd23
12 files changed
+175
-9
No files matched your search
@@ -506,3 +506,36 @@ full cloud execution must validate them. Custom ACP Stop/recovery browser fixtur
|
||||
explicitly select Legacy for their custom commands/state paths; latest cloud
|
||||
browser execution remains required. No provider turns or local native/Docker
|
||||
builds were added by these corrections.
|
||||
|
||||
|
||||
## Lifetime ownership and resolved Legacy limit — 2026-10-12
|
||||
|
||||
The fresh protocol review identified a real ownership violation: sandbox
|
||||
preparation rewrote active Claude settings before a contender acquired the
|
||||
provider lifetime lease. A fake-provider/real-lease regression reproduced losing
|
||||
the active owner's saved high effort and marker when the second opener was
|
||||
rejected. Repair `4cdb5ec4b38d792b432993a9a4b722090813a8ef` defers the
|
||||
Claude settings write until after lifetime ownership is acquired and before
|
||||
provider launch. Cancellation retains the lease until that filesystem write
|
||||
settles. A later legitimate owner can initialize normally. Host, sandbox and
|
||||
profile controls pass 104/104; runner noemit and unchanged declaration generation
|
||||
pass. No provider process was launched by these controls.
|
||||
|
||||
Fresh behavior review also found the Legacy form's default 1000-turn cap was
|
||||
omitted for Auto even when the selected target resolved to Legacy. The form now
|
||||
passes a transient resolution to the builder; unknown/Legacy Auto and explicit
|
||||
Legacy retain the cap, while resolved native omits only that Legacy default.
|
||||
Explicit custom limits still reach compatibility validation. The hint is absent
|
||||
from execution config and API overrides. Onboarding uses its existing discovery;
|
||||
import excludes hint-only updates from meaningful edits. The same builder carries
|
||||
explicit native permission choices from the ordinary create form. Mounted forms,
|
||||
builder, imports and onboarding pass 298/298; adapter-utils TS build, Claude/UI
|
||||
noemit, maps and token gates pass. No local Rust or Docker build or new paid turn.
|
||||
|
||||
The prior corrective platform source `a97a681f1caf90627505097cdcd368356811ffc1`
|
||||
passed all required cloud checks and fresh 5/5 review. Behavior and protocol
|
||||
required CI aggregates passed before these two findings. New repair heads require
|
||||
fresh exact-head CI/review. Prerequisite #16028 is not merged: normal auto-merge
|
||||
is queued for actual code-owner approval. Dependent branches have not been
|
||||
retargeted as if master contains that prerequisite. Final packaged runtime and
|
||||
source reconciliation remain gated; the source owner retains merge ownership.
|
||||
@@ -725,6 +725,8 @@ export interface CLIAdapterModule {
|
||||
|
||||
export interface CreateConfigValues {
|
||||
runner?: "auto" | "paperclip" | "legacy";
|
||||
/** Transient form resolution; never an execution setting or API override. */
|
||||
resolvedRunner?: "paperclip" | "legacy";
|
||||
adapterType: string;
|
||||
codexPermissionMode?: "never" | "on-request" | "untrusted";
|
||||
/** Explicit Legacy permission choice; never persisted as a runner setting. */
|
||||
|
||||
@@ -37,6 +37,20 @@ function makeValues(overrides: Partial<CreateConfigValues> = {}): CreateConfigVa
|
||||
}
|
||||
|
||||
describe("buildClaudeLocalConfig", () => {
|
||||
it.each([undefined, "legacy"] as const)("retains the default turn cap when Auto has resolved to %s", resolvedRunner => {
|
||||
expect(buildClaudeLocalConfig(makeValues({ runner: "auto", resolvedRunner })).maxTurnsPerRun).toBe(1000);
|
||||
});
|
||||
it.each(["auto", "paperclip"] as const)("omits the Legacy default only for native %s", runner => {
|
||||
const config = buildClaudeLocalConfig(makeValues({ runner, resolvedRunner: "paperclip" }));
|
||||
expect(config).not.toHaveProperty("maxTurnsPerRun");
|
||||
expect(buildClaudeLocalConfig(makeValues({ runner, resolvedRunner: "paperclip", maxTurnsPerRun: 3 })).maxTurnsPerRun).toBe(3);
|
||||
});
|
||||
it("retains an explicit Legacy cap over a stale native form resolution", () => {
|
||||
expect(buildClaudeLocalConfig(makeValues({ runner: "legacy", resolvedRunner: "paperclip" })).maxTurnsPerRun).toBe(1000);
|
||||
});
|
||||
it.each(["approve-all", "approve-reads", "deny-all"])("carries the selected native %s permission to creation", acpxPermissionMode => {
|
||||
expect(buildClaudeLocalConfig(makeValues({ resolvedRunner: "paperclip", adapterSchemaValues: { acpxPermissionMode } })).acpxPermissionMode).toBe(acpxPermissionMode);
|
||||
});
|
||||
it("omits engine for the auto default so runtime fallback remains available", () => {
|
||||
const config = buildClaudeLocalConfig(makeValues({ claudeEngine: "auto" }));
|
||||
|
||||
|
||||
@@ -43,8 +43,14 @@ export function buildClaudeLocalConfig(v: CreateConfigValues): Record<string, un
|
||||
ac.graceSec = 15;
|
||||
const env = buildAdapterEnvConfig(v.envBindings, v.envVars);
|
||||
if (Object.keys(env).length > 0) ac.env = env;
|
||||
// Native creation omits the legacy form default; explicit limits remain visible to validation.
|
||||
if (v.runner === "legacy" || v.maxTurnsPerRun !== 1000) ac.maxTurnsPerRun = v.maxTurnsPerRun;
|
||||
const nativeSelected = v.runner === "paperclip"
|
||||
|| ((v.runner === undefined || v.runner === "auto") && v.resolvedRunner === "paperclip");
|
||||
// Only a resolved native form omits the Legacy default. Explicit limits
|
||||
// still reach the server's compatibility check.
|
||||
if (!nativeSelected || v.maxTurnsPerRun !== 1000) ac.maxTurnsPerRun = v.maxTurnsPerRun;
|
||||
if (nativeSelected && v.adapterSchemaValues?.acpxPermissionMode !== undefined) {
|
||||
ac.acpxPermissionMode = v.adapterSchemaValues.acpxPermissionMode;
|
||||
}
|
||||
ac.dangerouslySkipPermissions = v.dangerouslySkipPermissions;
|
||||
if (v.workspaceStrategyType === "git_worktree") {
|
||||
ac.workspaceStrategy = {
|
||||
|
||||
@@ -237,6 +237,66 @@ describe("ACPX runtime host", () => {
|
||||
expect(openRuntime).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("leaves an active Claude owner's effort settings unchanged when a second opener loses the lifetime lease", async () => {
|
||||
const fixture = await hostFixture();
|
||||
let settingsPath = "";
|
||||
const openRuntime = vi.fn(async () => runtimePort({ getStatus: async () => ({ models: { currentModelId: "claude-sonnet-5" } }) }));
|
||||
const dependencies = fixture.dependencies({ openRuntime });
|
||||
const prepare = dependencies.prepareSandbox!;
|
||||
dependencies.prepareSandbox = async input => {
|
||||
const sandbox = await prepare(input);
|
||||
settingsPath = join(sandbox.agentHomeDirectory, "settings.json");
|
||||
return sandbox;
|
||||
};
|
||||
const options = { ...fixture.options, agent: "claude" as const, model: "claude-sonnet-5", permissionMode: "approve-all" as const };
|
||||
const host = await AcpxRuntimeHost.open(options, dependencies);
|
||||
try {
|
||||
const ownedSettings = `${JSON.stringify({ ...JSON.parse(await readFile(settingsPath, "utf8")), effortLevel: "high", activeOwnerMarker: true })}\n`;
|
||||
await writeFile(settingsPath, ownedSettings, { mode: 0o600 });
|
||||
await expect(AcpxRuntimeHost.open(options, dependencies)).rejects.toThrow("already has an active lease");
|
||||
expect(await readFile(settingsPath, "utf8")).toBe(ownedSettings);
|
||||
expect(openRuntime).toHaveBeenCalledTimes(1);
|
||||
} finally { await host.close({ reason: "ownership regression complete" }); }
|
||||
const nextOwner = await waitForAcpxOperation(() => AcpxRuntimeHost.open(options, dependencies));
|
||||
try {
|
||||
expect(JSON.parse(await readFile(settingsPath, "utf8"))).not.toHaveProperty("activeOwnerMarker");
|
||||
expect(openRuntime).toHaveBeenCalledTimes(2);
|
||||
} finally { await nextOwner.close({ reason: "next owner complete" }); }
|
||||
});
|
||||
|
||||
it("keeps the Claude lifetime lease until an owned settings write settles after cancellation", async () => {
|
||||
const fixture = await hostFixture();
|
||||
const controller = new AbortController();
|
||||
let announceWrite!: () => void;
|
||||
let finishWrite!: () => void;
|
||||
const writeStarted = new Promise<void>(resolve => { announceWrite = resolve; });
|
||||
const writePending = new Promise<void>(resolve => { finishWrite = resolve; });
|
||||
const openRuntime = vi.fn(async () => runtimePort({ getStatus: async () => ({ models: { currentModelId: "claude-sonnet-5" } }) }));
|
||||
const dependencies = fixture.dependencies({ openRuntime });
|
||||
const prepare = dependencies.prepareSandbox!;
|
||||
dependencies.prepareSandbox = async input => {
|
||||
const sandbox = await prepare(input);
|
||||
const initialize = sandbox.initializeOwnedConfiguration;
|
||||
return { ...sandbox, initializeOwnedConfiguration: async () => {
|
||||
announceWrite();
|
||||
await writePending;
|
||||
await initialize?.();
|
||||
} };
|
||||
};
|
||||
const options = { ...fixture.options, agent: "claude" as const, model: "claude-sonnet-5", permissionMode: "approve-all" as const };
|
||||
const opening = AcpxRuntimeHost.open({ ...options, signal: controller.signal }, dependencies);
|
||||
const rejected = expect(opening).rejects.toThrow("settings admission cancelled");
|
||||
try {
|
||||
await writeStarted;
|
||||
controller.abort(new Error("settings admission cancelled"));
|
||||
await expect(AcpxRuntimeHost.open(options, fixture.dependencies({ openRuntime }))).rejects.toThrow("already has an active lease");
|
||||
} finally { finishWrite(); }
|
||||
await rejected;
|
||||
expect(openRuntime).not.toHaveBeenCalled();
|
||||
const nextOwner = await waitForAcpxOperation(() => AcpxRuntimeHost.open(options, fixture.dependencies({ openRuntime })));
|
||||
await nextOwner.close({ reason: "cancelled initializer ownership released" });
|
||||
});
|
||||
|
||||
it("registers the assigned connection gateway alongside the Claude task bridge", async () => {
|
||||
const fixture = await hostFixture();
|
||||
let servers: AcpxRuntimePortOpenOptions["mcpServers"] = [];
|
||||
|
||||
@@ -482,6 +482,11 @@ export class AcpxRuntimeHost {
|
||||
dependencies.reportRetainedCleanupFailure(failure),
|
||||
});
|
||||
}
|
||||
options.signal?.throwIfAborted();
|
||||
// Await this write under ownership even if cancellation arrives. Releasing
|
||||
// the lease before it settles could let a late write affect the next owner.
|
||||
await sandbox.initializeOwnedConfiguration?.();
|
||||
options.signal?.throwIfAborted();
|
||||
assertAcpxProfileEnvironment(options.agent, sandbox.launchEnvironment);
|
||||
let launchEnvironment = sandbox.launchEnvironment;
|
||||
// This copy is registered by the authenticated controller for its company,
|
||||
|
||||
@@ -98,6 +98,7 @@ describe("ACPX runtime sandbox", () => {
|
||||
binding: { ...fixture.binding, requestedModel: model },
|
||||
agent: "claude",
|
||||
});
|
||||
await sandbox.initializeOwnedConfiguration?.();
|
||||
const settingsPath = join(sandbox.agentHomeDirectory, "settings.json");
|
||||
expect(JSON.parse(await readFile(settingsPath, "utf8"))).toMatchObject({
|
||||
model,
|
||||
@@ -113,6 +114,7 @@ describe("ACPX runtime sandbox", () => {
|
||||
const binding = { ...fixture.binding, requestedModel: model, effectiveModel: model };
|
||||
for (let attempt = 0; attempt < 2; attempt += 1) {
|
||||
const sandbox = await prepareAcpxRuntimeSandbox({ binding, agent: "claude" });
|
||||
await sandbox.initializeOwnedConfiguration?.();
|
||||
const settings = JSON.parse(await readFile(join(sandbox.agentHomeDirectory, "settings.json"), "utf8"));
|
||||
expect(settings).toMatchObject({ model, availableModels: [model] });
|
||||
}
|
||||
@@ -125,11 +127,13 @@ describe("ACPX runtime sandbox", () => {
|
||||
const binding = { ...fixture.binding, claudeEffort };
|
||||
for (let attempt = 0; attempt < 2; attempt += 1) {
|
||||
const sandbox = await prepareAcpxRuntimeSandbox({ binding, agent: "claude" });
|
||||
await sandbox.initializeOwnedConfiguration?.();
|
||||
const settings = JSON.parse(await readFile(join(sandbox.agentHomeDirectory, "settings.json"), "utf8"));
|
||||
expect(settings.effortLevel).toBe(claudeEffort);
|
||||
}
|
||||
}
|
||||
const sandbox = await prepareAcpxRuntimeSandbox({ binding: fixture.binding, agent: "claude" });
|
||||
await sandbox.initializeOwnedConfiguration?.();
|
||||
expect(JSON.parse(await readFile(join(sandbox.agentHomeDirectory, "settings.json"), "utf8"))).not.toHaveProperty("effortLevel");
|
||||
});
|
||||
|
||||
|
||||
@@ -53,6 +53,8 @@ export interface AcpxRuntimeSandbox {
|
||||
cacheDirectory: string;
|
||||
agentHomeDirectory: string;
|
||||
workspaceRecordPath: string;
|
||||
/** Mutate provider settings only while the host holds its lifetime lease. */
|
||||
initializeOwnedConfiguration?: () => Promise<void>;
|
||||
launchEnvironment: Readonly<NodeJS.ProcessEnv>;
|
||||
persistedEnvironment: Readonly<NodeJS.ProcessEnv>;
|
||||
/** The host sandbox must protect these runtime/config paths from tools. */
|
||||
@@ -394,7 +396,7 @@ export async function prepareAcpxRuntimeSandbox(input: {
|
||||
const configuration = piProviderConfiguration(input.environment);
|
||||
if (configuration) await writePrivateFile(join(agentHomeDirectory, "models.json"), configuration.json);
|
||||
}
|
||||
if (input.agent === "claude") {
|
||||
const initializeOwnedConfiguration = input.agent === "claude" ? async () => {
|
||||
// ACP otherwise rewrites exact IDs (including user-entered model IDs) to
|
||||
// picker aliases such as "sonnet". Its supported availableModels setting
|
||||
// preserves our requested ID through selection and model verification.
|
||||
@@ -412,7 +414,7 @@ export async function prepareAcpxRuntimeSandbox(input: {
|
||||
: {}),
|
||||
})}\n`,
|
||||
);
|
||||
}
|
||||
} : undefined;
|
||||
if (input.agent === "grok") {
|
||||
// Native project rules merge with global rules. Ask rules win over project
|
||||
// allows, keeping operation approval at the runner's authenticated boundary.
|
||||
@@ -563,6 +565,7 @@ export async function prepareAcpxRuntimeSandbox(input: {
|
||||
cacheDirectory,
|
||||
agentHomeDirectory,
|
||||
workspaceRecordPath,
|
||||
...(initializeOwnedConfiguration ? { initializeOwnedConfiguration } : {}),
|
||||
protectedPaths,
|
||||
launchEnvironment: Object.freeze({ ...launchEnvironment }),
|
||||
persistedEnvironment: Object.freeze(persistedEnvironment),
|
||||
|
||||
@@ -18,6 +18,7 @@ import { ApiError } from "../api/client";
|
||||
import { aiConnectionsApi } from "../api/ai-connections";
|
||||
import { CodexLocalConfigFields } from "../adapters/codex-local/config-fields";
|
||||
import type { AdapterConfigFieldsProps } from "../adapters/types";
|
||||
import { buildClaudeLocalConfig } from "@paperclipai/adapter-claude-local/ui";
|
||||
import { DEFAULT_CODEX_LOCAL_MODEL } from "@paperclipai/adapter-codex-local";
|
||||
|
||||
const mockAgentsApi = vi.hoisted(() => ({
|
||||
@@ -73,9 +74,8 @@ vi.mock("../api/agents", () => ({
|
||||
agentsApi: mockAgentsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/adapters", () => ({
|
||||
adaptersApi: { list: async () => [{ type: "codex_local", supportedRunners: ["legacy", "paperclip"], defaultRunner: "legacy" }] },
|
||||
}));
|
||||
const mockAdaptersApi = vi.hoisted(() => ({ list: vi.fn(async () => [{ type: "codex_local", supportedRunners: ["legacy", "paperclip"], defaultRunner: "legacy" }]) }));
|
||||
vi.mock("../api/adapters", () => ({ adaptersApi: mockAdaptersApi }));
|
||||
|
||||
vi.mock("../api/environments", () => ({
|
||||
environmentsApi: mockEnvironmentsApi,
|
||||
@@ -660,6 +660,37 @@ async function flushUntil(check: () => boolean, timeoutMs = 4000) {
|
||||
describe("AgentConfigForm environment selector", () => {
|
||||
let roots: Root[] = [];
|
||||
|
||||
it.each(["paperclip", "legacy"] as const)("uses the resolved %s Claude form to preserve the correct turn limit", async defaultRunner => {
|
||||
mockAdaptersApi.list.mockResolvedValue([{ type: "claude_local", supportedRunners: defaultRunner === "paperclip" ? ["paperclip", "legacy"] : ["legacy"], defaultRunner }]);
|
||||
try {
|
||||
const values = { ...defaultCreateValues, adapterType: "claude_local", runner: "auto" as const };
|
||||
const result = await renderCreateForm([], values);
|
||||
roots.push(result.root);
|
||||
const resolved = result.onChange.mock.calls.reduce((current, [patch]) => ({ ...current, ...patch }), values);
|
||||
expect(resolved.resolvedRunner).toBe(defaultRunner);
|
||||
const config = buildClaudeLocalConfig(resolved);
|
||||
if (defaultRunner === "legacy") expect(config.maxTurnsPerRun).toBe(1000);
|
||||
else expect(config).not.toHaveProperty("maxTurnsPerRun");
|
||||
expect(config).not.toHaveProperty("resolvedRunner");
|
||||
} finally { mockAdaptersApi.list.mockResolvedValue([{ type: "codex_local", supportedRunners: ["legacy", "paperclip"], defaultRunner: "legacy" }]); }
|
||||
});
|
||||
|
||||
|
||||
it("carries a selected native Claude permission through the ordinary creation builder", async () => {
|
||||
mockAdaptersApi.list.mockResolvedValue([{ type: "claude_local", supportedRunners: ["paperclip", "legacy"], defaultRunner: "paperclip" }]);
|
||||
try {
|
||||
const values = { ...defaultCreateValues, adapterType: "claude_local", runner: "auto" as const };
|
||||
const result = await renderCreateForm([], values);
|
||||
roots.push(result.root);
|
||||
await clickElement(result.container.querySelector('[aria-label="Permission mode"]'));
|
||||
const deny = [...document.querySelectorAll<HTMLElement>('[role="option"]')].find(option => option.textContent === "Deny all");
|
||||
expect(deny).toBeTruthy();
|
||||
await clickElement(deny);
|
||||
const resolved = result.onChange.mock.calls.reduce((current, [patch]) => ({ ...current, ...patch }), values);
|
||||
expect(buildClaudeLocalConfig(resolved)).toMatchObject({ acpxPermissionMode: "deny-all" });
|
||||
} finally { mockAdaptersApi.list.mockResolvedValue([{ type: "codex_local", supportedRunners: ["legacy", "paperclip"], defaultRunner: "legacy" }]); }
|
||||
});
|
||||
|
||||
it("keeps an explicit Codex draft policy until a harness reset clears it", async () => {
|
||||
mockEnvironmentsApi.list.mockResolvedValue([]);
|
||||
const container = document.createElement("div");
|
||||
|
||||
@@ -628,6 +628,12 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
|
||||
const fieldsAdapterType = isRunnerHarness
|
||||
? (runner === "paperclip" || (runner === "auto" && runnerAvailability?.defaultRunner === "paperclip") ? "paperclip_runner" : harnessType)
|
||||
: adapterType;
|
||||
useEffect(() => {
|
||||
if (props.mode !== "create" || harnessType !== "claude_local") return;
|
||||
const resolvedRunner = runner === "auto" ? runnerAvailability?.defaultRunner : runner;
|
||||
if (props.values.resolvedRunner !== resolvedRunner) props.onChange({ resolvedRunner });
|
||||
}, [props.mode, harnessType, runner, runnerAvailability?.defaultRunner,
|
||||
isCreate ? props.values.resolvedRunner : undefined, isCreate ? props.onChange : undefined]);
|
||||
const getCapabilities = useAdapterCapabilities();
|
||||
const adapterCaps = getCapabilities(isRunnerHarness ? harnessType : adapterType);
|
||||
const isDotRunner = adapterType === "paperclip_runner" && (isCreate ? props.values.adapterSchemaValues?.provider : eff("adapterConfig", "provider", config.provider)) === "openai_dot";
|
||||
|
||||
@@ -1832,6 +1832,7 @@ function OnboardingWizardInner({
|
||||
...defaultCreateValues,
|
||||
adapterType,
|
||||
runner,
|
||||
resolvedRunner: runner === "auto" ? runnerDiscovery.data?.find(item => item.type === adapterType)?.defaultRunner : runner,
|
||||
model:
|
||||
adapterType === "gemini_local"
|
||||
? model || DEFAULT_GEMINI_LOCAL_MODEL
|
||||
|
||||
@@ -1626,7 +1626,7 @@ export function CompanyImport() {
|
||||
resetMutationState();
|
||||
const agent = adapterAgents.find((a) => a.slug === slug);
|
||||
const currentType = agent ? effectiveAdapterType(agent) : adapterOverrides[slug] ?? "claude_local";
|
||||
if (Object.keys(patch).some(key => key !== "runner")) {
|
||||
if (Object.keys(patch).some(key => key !== "runner" && key !== "resolvedRunner")) {
|
||||
setAdapterConfigEditedSlugs(prev => new Set([...prev, slug]));
|
||||
}
|
||||
setAdapterConfigValues((prev) => ({
|
||||
@@ -1736,7 +1736,8 @@ export function CompanyImport() {
|
||||
for (const agent of adapterAgents) {
|
||||
const selectedType = effectiveAdapterType(agent);
|
||||
const configVals = adapterConfigValues[agent.slug];
|
||||
if (selectedType === agent.adapterChoice && !configVals) continue;
|
||||
if (selectedType === agent.adapterChoice && !adapterConfigEditedSlugs.has(agent.slug)
|
||||
&& (!configVals || configVals.runner === importAdapterValues(selectedType, agent.adapterConfig, agent.adapterType, agent.runner).runner)) continue;
|
||||
const retainsNativeHarness = agent.adapterType === "paperclip_runner"
|
||||
&& selectedType === agent.adapterChoice && ["codex_local", "claude_local"].includes(selectedType);
|
||||
const apiType = selectedType === "openai_dot" || retainsNativeHarness ? "paperclip_runner" : selectedType;
|
||||
|
||||
Reference in new issue
Block a user