Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 19 additions & 17 deletions docs/design/derived-config-ownership.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@

`Config` derivation is a state-ownership operation, not a clone. `deriveConfig` keeps the existing prototype overlay model behind one boundary while production callers are migrated incrementally.

| State | Ownership | Contract |
| -------------------------- | -------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- |
| workspace path and context | shared until explicitly overlaid | Worktree profiles must override the paired public getters and private reads together. |
| file service and discovery | shared until explicitly overlaid | Worktree profiles rebind both to the target workspace. |
| tool registry | shared or explicitly replaced | Agent profiles that rebuild it own cleanup of the replacement registry. |
| permission manager | shared or explicitly replaced | Agent profiles preserve the existing strip/restore lifecycle. |
| approval mode | shared | Derived profiles may read the inherited mode but cannot mutate it until they own an independent permission-manager lifecycle. |
| file-read cache | child-local | The first getter call installs a fresh cache on the derived Config. |
| memory-pressure monitor | child-local | The first getter call installs a new monitor using the inherited configuration snapshot. |
| active todo state | child-local | The first mutation installs independent maps. |
| chat recording service | shared unless hidden | Scoped profiles may hide it through a getter override. |
| goal runtime | prohibited | A derived Config cannot resolve the parent conversation's runtime. |
| session writer state | prohibited | Writer ownership stays with the canonical session Config. |
| canonical lifecycle | prohibited | A derived Config cannot initialize, start a session, relocate the workspace, or clean up inherited Team/Arena runtime resources. |
| approval mode mutation | prohibited | A derived Config cannot restore or strip rules on the canonical PermissionManager. |
| State | Ownership | Contract |
| -------------------------- | -------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- |
| workspace path and context | shared until explicitly overlaid | Worktree profiles must override the paired public getters and private reads together. |
| file service and discovery | shared until explicitly overlaid | Worktree profiles rebind both to the target workspace. |
| tool registry | shared or explicitly replaced | Agent profiles that rebuild it own cleanup of the replacement registry. |
| permission manager | shared or explicitly replaced | Agent profiles preserve the existing strip/restore lifecycle. |
| approval mode | shared or explicitly copied | Bare derived profiles cannot mutate it; agent execution profiles own child-local state and preserve the canonical permission lifecycle. |
| file-read cache | child-local | The first getter call installs a fresh cache on the derived Config. |
| memory-pressure monitor | child-local | The first getter call installs a new monitor using the inherited configuration snapshot. |
| active todo state | child-local | The first mutation installs independent maps. |
| chat recording service | shared unless hidden | Scoped profiles may hide it through a getter override. |
| goal runtime | prohibited | A derived Config cannot resolve the parent conversation's runtime. |
| session writer state | prohibited | Writer ownership stays with the canonical session Config. |
| canonical lifecycle | prohibited | A derived Config cannot initialize, start a session, relocate the workspace, or clean up inherited Team/Arena runtime resources. |
| approval mode mutation | prohibited or explicitly copied | Only the approval-profile factory may install a child-local transition method and cleanup contract. |

Migration order:

Expand All @@ -25,6 +25,8 @@ Migration order:
3. Scoped memory/remember/skill-review profiles.
4. Enforce that production prototype derivation occurs only inside `deriveConfig`.

The factory intentionally accepts public getter overrides only. Private field rebinding needed by worktree contexts remains a separate migration concern and must be encoded without exposing arbitrary Config mutation.
The generic factory intentionally accepts public getter overrides only. Named profiles keep private field rebinding and lifecycle management inside `config.ts` without exposing arbitrary Config mutation.

Approval-mode override wrappers that call Config prototype mutators remain outside `deriveConfig` until their strip/restore lifecycle is owned independently from the parent permission manager.
The approval profile owns child-local approval state and its parent permission-manager strip/restore contract. Callers remain responsible for invoking its cleanup callback when the agent lifecycle ends.

Production core modules that import `Config` are linted against non-null `Object.create(...)` calls, keeping new prototype overlays behind the generic or named factories.
59 changes: 59 additions & 0 deletions eslint-rules/no-config-object-create.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
function importsConfig(node) {
return (
typeof node.source.value === 'string' &&
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
node.source.value.endsWith('/config/config.js') &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The importsConfig predicate requires node.source.value.endsWith('/config/config.js'). A same-directory specifier './config.js' (used by siblings of config.ts within packages/core/src/config/) would evade the rule. The risk is narrow today — the exemption list already excludes config.ts itself, and no current sibling uses the short form — but a future utility added to that directory that derives Config would be silently unguarded. Extending the predicate to also match === './config.js' would close the gap. (Confirms qwen-code-ci-bot R1-6, still present.)

node.specifiers.some(
(specifier) =>
specifier.type === 'ImportSpecifier' &&
specifier.imported.type === 'Identifier' &&
specifier.imported.name === 'Config',
)
);
}

function isObjectCreate(node) {
return (
node.callee.type === 'MemberExpression' &&
!node.callee.computed &&
node.callee.object.type === 'Identifier' &&
node.callee.object.name === 'Object' &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'create' &&
!(node.arguments[0]?.type === 'Literal' && node.arguments[0].value === null)
);
}

export default {
meta: {
type: 'problem',
docs: {
description:
'Require Config prototype derivation to go through deriveConfig.',
},
schema: [],
messages: {
useDeriveConfig:
'Do not derive Config with Object.create(). Use deriveConfig() or a specialized Config factory.',
},
},

create(context) {
let hasConfigImport = false;
const objectCreateCalls = [];

return {
ImportDeclaration(node) {
if (importsConfig(node)) hasConfigImport = true;
},
CallExpression(node) {
if (isObjectCreate(node)) objectCreateCalls.push(node);
},
'Program:exit'() {
if (!hasConfigImport) return;
for (const node of objectCreateCalls) {
context.report({ node, messageId: 'useDeriveConfig' });
}
},
};
},
};
22 changes: 22 additions & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import noCoreRootBarrelImport from './eslint-rules/no-core-root-barrel-import.js
import noUtilsUpwardImport from './eslint-rules/no-utils-upward-import.js';
import noCoreUtilsUpwardImport from './eslint-rules/no-core-utils-upward-import.js';
import { legacyFilenames } from './eslint.legacy-filenames.mjs';
import noConfigObjectCreate from './eslint-rules/no-config-object-create.js';

// General syntax restrictions applied to every TS/TSX source file. Hoisted so
// surface-specific overrides (flat config keeps only the last
Expand Down Expand Up @@ -335,6 +336,27 @@ export default tseslint.config(
'no-console': 'off',
},
},
{
files: ['packages/core/src/**/*.ts'],
ignores: [
'packages/core/src/config/config.ts',
'**/*.test.ts',
'**/*.spec.ts',
'**/__tests__/**',
'**/generated/**',
'**/*.generated.ts',
],
plugins: {
'qwen-code': {
rules: {
'no-config-object-create': noConfigObjectCreate,
},
},
},
rules: {
'qwen-code/no-config-object-create': 'error',
},
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
},
{
// Enforce kebab-case filenames
files: ['packages/core/src/**/*.ts', 'packages/cli/src/**/*.ts'],
Expand Down
181 changes: 25 additions & 156 deletions packages/core/src/agents/backends/InProcessBackend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,23 @@

import path from 'node:path';
import { createDebugLogger } from '../../utils/debugLogger.js';
import { ApprovalMode, Config } from '../../config/config.js';
import {
ApprovalMode,
type Config,
type DerivedApprovalModeConfigHooks,
deriveApprovalModeConfig,
deriveAgentConfig,
} from '../../config/config.js';
import { Storage } from '../../config/storage.js';
import { type ContentGenerator } from '../../core/contentGenerator.js';
import type { RuntimeContentGeneratorView } from '../runtime/agent-context.js';
import type { ToolRegistry } from '../../tools/tool-registry.js';
import { WorkspaceContext } from '../../utils/workspaceContext.js';
import { FileDiscoveryService } from '../../services/fileDiscoveryService.js';
import { createRuntimeContentGeneratorView } from '../../models/content-generator-config.js';
import { AgentStatus, isTerminalStatus } from '../runtime/agent-types.js';
import { AgentCore } from '../runtime/agent-core.js';
import { AgentEventEmitter } from '../runtime/agent-events.js';
import { ContextState } from '../runtime/agent-headless.js';
import { AgentInteractive } from '../runtime/agent-interactive.js';
import { createDenialState } from '../../permissions/denialTracking.js';
import { runWithTeammateIdentity } from '../team/identity.js';
import type {
Backend,
Expand Down Expand Up @@ -492,49 +495,34 @@ async function createPerAgentConfig(
modelId?: string,
authOverrides?: InProcessSpawnConfig['authOverrides'],
approvalMode?: ApprovalMode,
approvalModeHooks?: ApprovalModeOverrideHooks,
approvalModeHooks?: DerivedApprovalModeConfigHooks,
): Promise<{
config: Config;
contentGenerator?: ContentGenerator;
runtimeView?: RuntimeContentGeneratorView;
cleanup: () => void;
}> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let override = Object.create(base) as any;
let dedicatedContentGenerator: ContentGenerator | undefined;
let runtimeView: RuntimeContentGeneratorView | undefined;
let cleanup = () => {};

if (approvalMode !== undefined) {
const handle = createApprovalModeConfigOverride(
base,
approvalMode,
approvalModeHooks,
);
override = handle.config as unknown as Record<string, unknown>;
cleanup = handle.cleanup;
}

let agentRegistry: ToolRegistry | undefined;
try {
override.getWorkingDir = () => cwd;
override.getTargetDir = () => cwd;
override.getProjectRoot = () => cwd;
override.getPlanFilePath = () => {
const approvalHandle =
approvalMode === undefined
? { config: base, cleanup: () => {} }
: deriveApprovalModeConfig(base, approvalMode, {
hooks: approvalModeHooks,
});
const handle = deriveAgentConfig(approvalHandle.config, cwd, {
customIgnoreFiles: base.getFileFilteringOptions().customIgnoreFiles,
getPlanFilePath: () => {
const sessionId = Storage.sanitizePlanSessionId(base.getSessionId());
const scopedAgentId = Storage.sanitizePlanSessionId(agentId);
return path.join(base.getPlansDir(), `${sessionId}-${scopedAgentId}.md`);
};

const agentWorkspace = new WorkspaceContext(cwd);
override.getWorkspaceContext = () => agentWorkspace;

const agentFileService = new FileDiscoveryService(
cwd,
base.getFileFilteringOptions().customIgnoreFiles,
);
override.getFileService = () => agentFileService;
},
});
const override = handle.config;
const cleanup = approvalHandle.cleanup;
let dedicatedContentGenerator: ContentGenerator | undefined;
let runtimeView: RuntimeContentGeneratorView | undefined;
let agentRegistry: ToolRegistry | undefined;

try {
// Delegated rather than re-enacted. The three steps below used to be
// inlined here, identical to the shared helper — and a second copy is a
// second place for an invariant to be broken: a change sharing the
Expand Down Expand Up @@ -598,125 +586,6 @@ async function createPerAgentConfig(
}
}

interface ApprovalModeOverrideHooks {
acquireAutoApprovalOverride(): boolean;
releaseAutoApprovalOverride(): void;
}

function createApprovalModeConfigOverride(
base: Config,
mode: ApprovalMode,
hooks?: ApprovalModeOverrideHooks,
): { config: Config; cleanup: () => void } {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const override = Object.create(base) as any;
const baseApprovalMode = base.getApprovalMode();
const initialMode = getTrustedInitialApprovalMode(base, mode);
let autoOverrideAcquired = false;
const acquireAutoOverride = () => {
if (autoOverrideAcquired || base.getApprovalMode() === ApprovalMode.AUTO) {
return;
}
if (hooks) {
autoOverrideAcquired = hooks.acquireAutoApprovalOverride();
return;
}
base.getPermissionManager?.()?.stripDangerousRulesForAutoMode();
autoOverrideAcquired = true;
};
const releaseAutoOverride = () => {
if (!autoOverrideAcquired) {
return;
}
if (hooks) {
hooks.releaseAutoApprovalOverride();
} else if (base.getApprovalMode() !== ApprovalMode.AUTO) {
base.getPermissionManager?.()?.restoreDangerousRules();
}
autoOverrideAcquired = false;
};

override.approvalMode = initialMode;
override.manualPlanExitNoticeEventState = {
...(override.manualPlanExitNoticeEventState ?? {
version: 0,
kind: 'clear',
}),
};
override.getApprovalMode = Config.prototype.getApprovalMode;
override.prePlanMode =
initialMode === ApprovalMode.PLAN
? baseApprovalMode === ApprovalMode.PLAN
? base.getPrePlanMode()
: baseApprovalMode
: undefined;
override.approvalModeRevision = 0;

override.setApprovalMode = (
nextMode: ApprovalMode,
options?: Parameters<Config['setApprovalMode']>[1],
) => {
const beforeMode = (override as Config).getApprovalMode();
const hadOwnPermissionManager = Object.prototype.hasOwnProperty.call(
override,
'permissionManager',
);
const ownPermissionManager = override.permissionManager;
override.permissionManager = null;
try {
Config.prototype.setApprovalMode.call(
override as Config,
nextMode,
options,
);
} finally {
if (hadOwnPermissionManager) {
override.permissionManager = ownPermissionManager;
} else {
delete override.permissionManager;
}
}

const afterMode = (override as Config).getApprovalMode();
if (beforeMode !== ApprovalMode.AUTO && afterMode === ApprovalMode.AUTO) {
acquireAutoOverride();
} else if (
beforeMode === ApprovalMode.AUTO &&
afterMode !== ApprovalMode.AUTO
) {
releaseAutoOverride();
}
};
override.autoModeDenialState = createDenialState();

const cleanup = () => {
releaseAutoOverride();
};

if (
initialMode === ApprovalMode.AUTO &&
base.getApprovalMode() !== ApprovalMode.AUTO
) {
acquireAutoOverride();
}

return { config: override as Config, cleanup };
}

function getTrustedInitialApprovalMode(
base: Config,
mode: ApprovalMode,
): ApprovalMode {
if (
!base.isTrustedFolder() &&
mode !== ApprovalMode.DEFAULT &&
mode !== ApprovalMode.PLAN
) {
return ApprovalMode.DEFAULT;
}
return mode;
}

function createRunInContext(
inProcessConfig: InProcessSpawnConfig,
): AgentInteractive['config']['runInContext'] {
Expand Down
Loading
Loading