Merge remote-tracking branch 'origin/farm/a2703356/enforced-rpc-behavior-left-over'
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed RPC/ACP startup clobbering explicit caller/project/global configuration for `task.isolation.{mode,merge,commits}`, `task.eager`, `task.batch`, `task.maxConcurrency`, `task.maxRecursionDepth`, `task.disabledAgents`, `task.agentModelOverrides`, `memory.backend`, `memories.enabled`, `advisor.{enabled,subagents,syncBacklog,immuneTurns}`, plus the RPC-only `async.{enabled,maxJobs}` and `bash.autoBackground.{enabled,thresholdMs}`. `applyDefaultSettingOverrides` re-asserted the schema default as a runtime override after settings load, regressing the `isConfigured()` guard added for #2598 and ignoring every explicit value the embedder, project, `--config` overlay, or global config had set. The guard is restored, so the host default now only fills holes ([#3207](https://github.com/can1357/oh-my-pi/issues/3207)).
|
||||
|
||||
## [16.1.11] - 2026-06-21
|
||||
|
||||
### Added
|
||||
|
||||
@@ -149,8 +149,16 @@ const RPC_BACKGROUND_DEFAULTED_SETTING_PATHS: SettingPath[] = [
|
||||
"bash.autoBackground.thresholdMs",
|
||||
];
|
||||
|
||||
// Protocol-mode hosts opt into a small set of paths whose host-default we
|
||||
// re-apply at startup so embedders inherit OMP's neutral defaults instead of
|
||||
// the local user's globally-persisted preferences for interactive use. The
|
||||
// guard preserves any explicit configuration — caller `Settings.isolated`
|
||||
// overrides, project `.claude/settings.yml`, `--config` overlays, or global
|
||||
// `config.yml` — so the host default only kicks in when nothing is set. Without
|
||||
// it the override clobbers every caller/host choice (#2598, #3207).
|
||||
function applyDefaultSettingOverrides(settingPaths: SettingPath[], targetSettings: Settings): void {
|
||||
for (const settingPath of settingPaths) {
|
||||
if (targetSettings.isConfigured(settingPath)) continue;
|
||||
targetSettings.override(settingPath, getDefault(settingPath));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -242,28 +242,57 @@ describe("ACP lazy startup", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("default-disables advisor for protocol hosts", async () => {
|
||||
it("honors explicit host-defaulted settings for protocol hosts", async () => {
|
||||
// Regression for #3207: in RPC/ACP startup, runtime overrides applied via
|
||||
// `applyDefaultSettingOverrides` previously clobbered any explicitly
|
||||
// configured value (caller, project, --config overlay, or global) with the
|
||||
// schema default. The fix (re-)added an `isConfigured` guard so explicit
|
||||
// configuration survives, and the schema default only fills holes.
|
||||
const { runRootCommand } = await import("@oh-my-pi/pi-coding-agent/main");
|
||||
|
||||
type ObservedAdvisorSettings = {
|
||||
enabled: boolean;
|
||||
subagents: boolean;
|
||||
syncBacklog: "off" | "1" | "3" | "5";
|
||||
immuneTurns: number;
|
||||
};
|
||||
const explicit = {
|
||||
"task.isolation.mode": "rcopy",
|
||||
"task.isolation.merge": "branch",
|
||||
"task.isolation.commits": "ai",
|
||||
"task.eager": "always",
|
||||
"task.batch": false,
|
||||
"task.maxConcurrency": 4,
|
||||
"task.maxRecursionDepth": 5,
|
||||
"task.disabledAgents": ["explore"],
|
||||
"task.agentModelOverrides": { task: "claude-sonnet-4-20250514" },
|
||||
"memory.backend": "local",
|
||||
"memories.enabled": true,
|
||||
"advisor.enabled": true,
|
||||
"advisor.subagents": true,
|
||||
"advisor.syncBacklog": "5",
|
||||
"advisor.immuneTurns": 7,
|
||||
} as const;
|
||||
const rpcOnlyExplicit = {
|
||||
"async.enabled": false,
|
||||
"async.maxJobs": 7,
|
||||
"bash.autoBackground.enabled": true,
|
||||
"bash.autoBackground.thresholdMs": 5_000,
|
||||
} as const;
|
||||
const allPaths = [
|
||||
...(Object.keys(explicit) as (keyof typeof explicit)[]),
|
||||
...(Object.keys(rpcOnlyExplicit) as (keyof typeof rpcOnlyExplicit)[]),
|
||||
];
|
||||
type ObservedSettings = Record<string, unknown>;
|
||||
|
||||
const runProtocolStartup = async (mode: "rpc" | "rpc-ui" | "acp"): Promise<ObservedAdvisorSettings> => {
|
||||
using tempDir = TempDir.createSync("@omp-protocol-advisor-settings-");
|
||||
const runProtocolStartup = async (mode: "rpc" | "rpc-ui" | "acp"): Promise<ObservedSettings> => {
|
||||
using tempDir = TempDir.createSync("@omp-protocol-host-defaulted-");
|
||||
const cwd = tempDir.path();
|
||||
const authStorage = await AuthStorage.create(path.join(cwd, "auth.db"));
|
||||
const settings = Settings.isolated({
|
||||
"advisor.enabled": true,
|
||||
"advisor.subagents": true,
|
||||
"advisor.syncBacklog": "5",
|
||||
"advisor.immuneTurns": 3,
|
||||
});
|
||||
let observed: ObservedAdvisorSettings | undefined;
|
||||
const stopMessage = "stop test protocol mode";
|
||||
const settings = Settings.isolated({ ...explicit, ...rpcOnlyExplicit });
|
||||
let observed: ObservedSettings | undefined;
|
||||
const stopMessage = "stop test host-defaulted settings";
|
||||
const observe = () => {
|
||||
observed = {};
|
||||
for (const key of allPaths) {
|
||||
observed[key] = settings.get(key);
|
||||
}
|
||||
throw new Error(stopMessage);
|
||||
};
|
||||
|
||||
try {
|
||||
await runRootCommand(
|
||||
@@ -284,24 +313,8 @@ describe("ACP lazy startup", () => {
|
||||
{
|
||||
discoverAuthStorage: async () => authStorage,
|
||||
settings,
|
||||
createAgentSession: async () => {
|
||||
observed = {
|
||||
enabled: settings.get("advisor.enabled"),
|
||||
subagents: settings.get("advisor.subagents"),
|
||||
syncBacklog: settings.get("advisor.syncBacklog"),
|
||||
immuneTurns: settings.get("advisor.immuneTurns"),
|
||||
};
|
||||
throw new Error(stopMessage);
|
||||
},
|
||||
runAcpMode: async () => {
|
||||
observed = {
|
||||
enabled: settings.get("advisor.enabled"),
|
||||
subagents: settings.get("advisor.subagents"),
|
||||
syncBacklog: settings.get("advisor.syncBacklog"),
|
||||
immuneTurns: settings.get("advisor.immuneTurns"),
|
||||
};
|
||||
throw new Error(stopMessage);
|
||||
},
|
||||
createAgentSession: async () => observe(),
|
||||
runAcpMode: async () => observe(),
|
||||
},
|
||||
);
|
||||
} catch (error) {
|
||||
@@ -319,12 +332,7 @@ describe("ACP lazy startup", () => {
|
||||
};
|
||||
|
||||
for (const mode of ["rpc", "rpc-ui", "acp"] as const) {
|
||||
await expect(runProtocolStartup(mode)).resolves.toEqual({
|
||||
enabled: false,
|
||||
subagents: false,
|
||||
syncBacklog: "off",
|
||||
immuneTurns: 3,
|
||||
});
|
||||
await expect(runProtocolStartup(mode)).resolves.toEqual({ ...explicit, ...rpcOnlyExplicit });
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user