Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a983ece
refactor(cli): enforce utils leaf-layer dependency direction (#9146)
yiliang114 Aug 22, 2026
7735a3b
fix: use Qwen Team 2026 license header on new files (#9146)
yiliang114 Aug 22, 2026
204b3c3
chore: refresh stale utils/ path references after leaf-layer move (#9…
yiliang114 Aug 22, 2026
21d21a0
Merge remote-tracking branch 'origin/main' into codex/9146-utils-leaf…
yiliang114 Aug 22, 2026
a81cbe7
docs: reconcile no-utils-upward-import header with the allowed type-o…
yiliang114 Aug 22, 2026
beab2dc
fix(cli): allowlist sandbox process.env accesses after leaf-layer mov…
yiliang114 Aug 22, 2026
1e48458
chore(ci): re-record qwen-autofix.yml size baseline after #9677 (#9146)
yiliang114 Aug 22, 2026
0566edc
fix(review): drop the stale utils/findings.ts digest root after the l…
yiliang114 Aug 22, 2026
0dca7b0
fix(review): colocate seatbelt profiles with the sandbox module (#9146)
yiliang114 Aug 22, 2026
25670ab
fix(review): exempt inline type-only specifiers from the utils upward…
yiliang114 Aug 22, 2026
21cfacf
fix(review): report upward inline type-specifier imports under verbat…
yiliang114 Aug 22, 2026
6980025
test(review): pin mixed-specifier and zero-specifier upward imports i…
yiliang114 Aug 22, 2026
bf6b9ba
test(review): anchor the nested-checkout utils rule fixture on the la…
yiliang114 Aug 22, 2026
8071456
test(review): pin that the utils/findings.ts digest root stays remove…
yiliang114 Aug 22, 2026
256abb1
Merge remote-tracking branch 'origin/main' into codex/9146-utils-leaf…
yiliang114 Aug 23, 2026
1b870e2
fix(review): reword stale-bundle SCOPE header to the post-move helper…
yiliang114 Aug 23, 2026
ab039d6
test(review): drop the pre-move utils/findings.ts from the skill-pari…
yiliang114 Aug 23, 2026
25687c4
test(serve): derive the seatbelt colocation tripwire from BUILTIN_SEA…
yiliang114 Aug 23, 2026
764d603
fix(architecture): fail closed on computed dynamic imports in the uti…
yiliang114 Aug 23, 2026
98f0134
fix(cli): point settings.test.ts at the post-move settingsUtils path …
yiliang114 Aug 23, 2026
e81731f
fix(cli): close utils boundary review gaps
yiliang114 Aug 23, 2026
85320da
Merge remote-tracking branch 'origin/main' into codex/pr-9737-closeou…
yiliang114 Aug 23, 2026
1445a76
test(cli): cover utils boundary allow paths
yiliang114 Aug 23, 2026
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
115 changes: 115 additions & 0 deletions eslint-rules/no-utils-upward-import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

import path from 'node:path';

/**
* `packages/cli/src/utils/` is the leaf layer that every other directory
* imports. It must not import back up into a domain directory (`config/`,
* `ui/`, `i18n/`, `nonInteractive/`, `commands/`, `serve/`,
* `acp-integration/`, ...): that is the dependency-direction invariant
* tracked in #9146.
*
* The only permitted "upward" references are `import type` specifiers. A
* type-only import is erased at compile time, so it cannot create a runtime
* module cycle. The two remaining instances (`Settings` in
* `modelConfigUtils.ts`, `CommandContext` in `sessionPaths.ts`) are this
* irreducible type-level coupling.
Comment thread
yiliang114 marked this conversation as resolved.
Outdated
*/

const CLI_UTILS_MARKER = 'packages/cli/src/utils/';
const TEST_OR_FIXTURE_SEGMENTS = new Set(['__tests__', 'fixtures']);

function isCliUtilsProductionFile(filename) {
if (!filename || filename === '<input>' || filename === '<text>') {
return false;
}
const normalized = path.normalize(filename).replaceAll('\\', '/');
const start = normalized.lastIndexOf(CLI_UTILS_MARKER);
if (start < 0) {
return false;
}
const relativePath = normalized.slice(start + CLI_UTILS_MARKER.length);
if (/\.(test|spec)\.[cm]?[jt]sx?$/.test(relativePath)) {
return false;
}
return !relativePath.split('/').some((s) => TEST_OR_FIXTURE_SEGMENTS.has(s));
}

function escapesUtils(filename, importedPath) {
const normalized = path.normalize(filename).replaceAll('\\', '/');
const utilsRoot = normalized.slice(
0,
normalized.lastIndexOf(CLI_UTILS_MARKER) + CLI_UTILS_MARKER.length,
);
const resolved = path.resolve(path.dirname(filename), importedPath);
return path
.relative(utilsRoot, resolved)
.replaceAll('\\', '/')
.startsWith('..');
}

export default {
meta: {
type: 'problem',
docs: {
description:
'packages/cli/src/utils must not import outside utils/ (leaf-layer dependency direction).',
},
messages: {
noUtilsUpwardImport:
'packages/cli/src/utils must not import outside utils/. ' +
'Invert the dependency (pass the value in) or move the module to the ' +
'domain directory that owns it (#9146).',
},
},
create(context) {
const { filename } = context;
if (!isCliUtilsProductionFile(filename)) {
return {};
}

const reportIfEscaping = (sourceNode, importedPath) => {
if (
typeof importedPath === 'string' &&
importedPath.startsWith('.') &&
Comment thread
yiliang114 marked this conversation as resolved.
Outdated
escapesUtils(filename, importedPath)
) {
Comment thread
yiliang114 marked this conversation as resolved.
Comment thread
yiliang114 marked this conversation as resolved.
context.report({ node: sourceNode, messageId: 'noUtilsUpwardImport' });
}
};

const checkStatic = (node) => {
// Type-only imports (`import type`, `export type ... from`) are erased at
// compile time and cannot create a runtime cycle.
if (node.importKind === 'type' || node.exportKind === 'type') {
return;
}
Comment thread
yiliang114 marked this conversation as resolved.
reportIfEscaping(node.source, node.source?.value);
};

const checkDynamic = (node) => {
const { source } = node;
if (source.type === 'Literal') {
reportIfEscaping(source, source.value);
} else if (
source.type === 'TemplateLiteral' &&
source.quasis.length === 1
) {
Comment thread
yiliang114 marked this conversation as resolved.
Outdated
Comment thread
yiliang114 marked this conversation as resolved.
Outdated
reportIfEscaping(source, source.quasis[0].value.cooked);
}
};

return {
ImportDeclaration: checkStatic,
ExportNamedDeclaration: checkStatic,
ExportAllDeclaration: checkStatic,
ImportExpression: checkDynamic,
// TSImportType (`import('../config/x').T`) is type-only by definition, so
// it is intentionally not reported.
};
},
};
29 changes: 13 additions & 16 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import globals from 'globals';
import storybook from 'eslint-plugin-storybook';
import checkFile from 'eslint-plugin-check-file';
import noCoreRootBarrelImport from './eslint-rules/no-core-root-barrel-import.js';
import noUtilsUpwardImport from './eslint-rules/no-utils-upward-import.js';
import { legacyFilenames } from './eslint.legacy-filenames.mjs';

// General syntax restrictions applied to every TS/TSX source file. Hoisted so
Expand Down Expand Up @@ -109,24 +110,20 @@ export default tseslint.config(
},
},
{
// `utils/` is the layer every other directory imports, so it must not
// import back into one. The daemon direction is clean and enforced here;
// the remaining `ui/`, `config/`, `i18n/` and `nonInteractive/` edges are
// tracked in #9146 and will be added to this group as they are resolved.
// `utils/` is the leaf layer that every other directory imports, so it
// must not import back up into a domain directory. Type-only imports are
// exempt: they are erased at compile time and cannot create a runtime
// cycle. See #9146.
files: ['packages/cli/src/utils/**/*.{ts,tsx}'],
rules: {
'no-restricted-imports': [
'error',
{
patterns: [
{
group: ['**/serve/*', '**/serve/**'],
message:
'packages/cli/src/utils must not import serve/. Move lifecycle-free logic down into utils/ instead (#9146).',
},
],
plugins: {
architecture: {
rules: {
'no-utils-upward-import': noUtilsUpwardImport,
},
],
},
},
rules: {
'architecture/no-utils-upward-import': 'error',
},
},
{
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const { mockMcpPoolDrainAll } = vi.hoisted(() => ({
vi.mock('../utils/cleanup.js', () => ({
runExitCleanup: mockRunExitCleanup,
}));
vi.mock('../utils/housekeeping/scheduler.js', () => ({
vi.mock('../services/housekeeping/scheduler.js', () => ({
startNonInteractiveOpenAILogHousekeeping:
mockStartNonInteractiveOpenAILogHousekeeping,
}));
Expand Down Expand Up @@ -872,7 +872,7 @@ vi.mock('./session/Session.js', () => {
}),
};
});
vi.mock('../utils/languageUtils.js', () => ({
vi.mock('../i18n/languageUtils.js', () => ({
updateOutputLanguageFile: vi.fn(),
writeOutputLanguageAndRegisterPath: vi.fn(
(
Expand Down Expand Up @@ -994,7 +994,7 @@ import {
resolveOutputLanguageOrPreserveAuto,
updateOutputLanguageFile,
writeOutputLanguageAndRegisterPath,
} from '../utils/languageUtils.js';
} from '../i18n/languageUtils.js';
import { buildAuthMethods } from './authMethods.js';
import {
ACTIVE_WORK_HEARTBEAT_META_KEY,
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ import {
ACP_EVENT_LOOP_STALL_RESTART_MS,
CHANNEL_PROMPT_META_KEY,
} from '@qwen-code/channel-base';
import { observeAcpToolResultWire } from '../utils/tool-result-boundary-diagnostics.js';
import { observeAcpToolResultWire } from '../nonInteractive/tool-result-boundary-diagnostics.js';
import { Readable, Writable } from 'node:stream';
import { normalizeDisabledToolList } from '../config/normalizeDisabledTools.js';
import { pipeline } from 'node:stream/promises';
Expand Down Expand Up @@ -278,11 +278,11 @@ import {
resolveOutputLanguageOrPreserveAuto,
getOutputLanguageFilePath,
writeOutputLanguageAndRegisterPath,
} from '../utils/languageUtils.js';
} from '../i18n/languageUtils.js';
import { runWithAcpRuntimeOutputDir } from './runtimeOutputDirContext.js';
import { ACP_ERROR_CODES } from './errorCodes.js';
import { runExitCleanup } from '../utils/cleanup.js';
import { startNonInteractiveOpenAILogHousekeeping } from '../utils/housekeeping/scheduler.js';
import { startNonInteractiveOpenAILogHousekeeping } from '../services/housekeeping/scheduler.js';
import { appEvents, AppEvent } from '../utils/events.js';
import {
setLanguageAsync,
Expand Down
8 changes: 3 additions & 5 deletions packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,8 @@ import type {
AgentSideConnection,
} from '@agentclientprotocol/sdk';
import { SettingScope, type LoadedSettings } from '../../config/settings.js';
import {
insertAfterFunctionResponses,
normalizePartList,
} from '../../utils/nonInteractiveHelpers.js';
import { insertAfterFunctionResponses } from '../../nonInteractive/nonInteractiveHelpers.js';
import { normalizePartList } from '../../utils/normalize-part-list.js';
import { prefixMidTurnUserMessageParts } from '../../utils/midTurnUserMessage.js';
import {
handleSlashCommand,
Expand Down Expand Up @@ -320,7 +318,7 @@ import type {
} from './types.js';
import { HistoryReplayer } from './history-replayer.js';
import { projectAcpToolResultUpdate } from './acp-tool-result-text-projection.js';
import { observeAcpToolResultProjection } from '../../utils/tool-result-boundary-diagnostics.js';
import { observeAcpToolResultProjection } from '../../nonInteractive/tool-result-boundary-diagnostics.js';
import { ToolCallEmitter } from './emitters/tool-call-emitter.js';
import { ToolCallPreparationTracker } from './tool-call-preparation-tracker.js';
import { PlanEmitter } from './emitters/PlanEmitter.js';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
createTranscriptToolCallStartUpdate,
} from '@qwen-code/acp-bridge/transcriptReplay';
import { sanitizeTerminalText } from '../../../ui/utils/textUtils.js';
import { associateAcpToolResultArtifact } from '../../../utils/tool-result-boundary-diagnostics.js';
import { associateAcpToolResultArtifact } from '../../../nonInteractive/tool-result-boundary-diagnostics.js';

const KIND_MAP: Record<Kind, ToolKind> = {
[Kind.Read]: 'read',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ import {

const observeAcpProjectionMock = vi.hoisted(() => vi.fn());
vi.mock(
'../../utils/tool-result-boundary-diagnostics.js',
'../../nonInteractive/tool-result-boundary-diagnostics.js',
async (original) => ({
...(await original<
typeof import('../../utils/tool-result-boundary-diagnostics.js')
typeof import('../../nonInteractive/tool-result-boundary-diagnostics.js')
>()),
observeAcpToolResultProjection: observeAcpProjectionMock,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type { SessionUpdate } from '@agentclientprotocol/sdk';
import type { TranscriptReplayStateV1 } from '@qwen-code/acp-bridge/transcriptReplay';
import { Buffer } from 'node:buffer';
import { projectAcpToolResultUpdate } from './acp-tool-result-text-projection.js';
import { observeAcpToolResultProjection } from '../../utils/tool-result-boundary-diagnostics.js';
import { observeAcpToolResultProjection } from '../../nonInteractive/tool-result-boundary-diagnostics.js';
import { HistoryReplayer } from './history-replayer.js';
import type { PendingReplayToolCall } from './history-replayer.js';
import type { CumulativeUsage, SessionEmitterContext } from './types.js';
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { Argv, CommandModule } from 'yargs';
import { parseArgsCommand } from './review/parse-args.js';
import { matchRemoteCommand } from './review/match-remote.js';
import { composeReviewCommand } from './review/compose-review.js';
import { findingsCommand } from '../utils/findings.js';
import { findingsCommand } from './review/findings.js';
import { recoverFindingsCommand } from './review/recover-findings.js';
import { fetchPrCommand } from './review/fetch-pr.js';
import { captureLocalCommand } from './review/capture-local.js';
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/review/compose-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {
SOURCES,
type Severity,
type Source,
} from '../../utils/findings.js';
} from './findings.js';
import { BRIEFS } from './lib/agent-briefs.js';
import {
budgetStopDisclosure,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ import {
} from 'node:fs';
import type { Stats } from 'node:fs';
import { dirname, resolve, sep } from 'node:path';
import { writeStdoutLine, writeStderrLine } from './stdioHelpers.js';
import type { AnchorRequest } from '../commands/review/lib/anchors.js';
import { isSameFile } from '../commands/review/lib/same-file.js';
import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js';
import type { AnchorRequest } from './lib/anchors.js';
import { isSameFile } from './lib/same-file.js';

// These four lists have a second consumer: the Web Shell review renderer
// (packages/web-shell/client/components/artifacts/CodeReviewArtifactDetail.tsx)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ vi.mock('../../../config/settings.js', async (importOriginal) => {
return { ...actual, loadSettings: loadSettingsMock };
});
import { operatorReviewSettings } from './review-settings.js';
import { getDialogSettingKeys } from '../../../utils/settingsUtils.js';
import { getDialogSettingKeys } from '../../../config/settingsUtils.js';

function setReview(review: unknown): void {
loadSettingsMock.mockReturnValue({ merged: { review } });
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/review/lib/shell-quote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* plain `'…'` wrap traded that for breaking on the first embedded apostrophe
* (`~/Documents/John's Projects/…` is an ordinary macOS workspace). The
* `'\''` dance closes both: end the quote, emit a literal `'`, reopen.
* Same pattern as `shellQuoteForSh` in utils/standalone-update.ts.
* Same pattern as `shellQuoteForSh` in ui/standalone-update.ts.
*/
export function shellQuotePath(p: string): string {
return `'${p.replace(/'/g, "'\\''")}'`;
Expand Down
6 changes: 1 addition & 5 deletions packages/cli/src/commands/review/publish-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ import {
type AssetsManifest,
type PublishedAsset,
} from './lib/assets.js';
import {
validateFindings,
buildReport,
type Finding,
} from '../../utils/findings.js';
import { validateFindings, buildReport, type Finding } from './findings.js';

interface PublishAssetsArgs {
pr: number;
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/review/save-artifact.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { tmpdir } from 'node:os';
import { join } from 'node:path';
import yargs from 'yargs';
import type { Argv } from 'yargs';
import { buildReport, type Finding } from '../../utils/findings.js';
import { buildReport, type Finding } from './findings.js';
import { saveArtifactCommand, saveReviewArtifact } from './save-artifact.js';

// On a case-sensitive filesystem the alias below never exists, so that test
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/review/save-artifact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
buildReport,
type FindingsReport,
validateFindings,
} from '../../utils/findings.js';
} from './findings.js';
import { EFFORT_LEVELS, type ReviewEffort } from './parse-args.js';
import { REVIEWS_DIR } from './lib/paths.js';
import { isSameFile } from './lib/same-file.js';
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ vi.mock('../utils/installationInfo.js', () => ({
getInstallationInfo,
resolveUpdateCommand,
}));
vi.mock('../utils/standalone-update.js', () => ({ performStandaloneUpdate }));
vi.mock('../ui/standalone-update.js', () => ({ performStandaloneUpdate }));
vi.mock('../utils/package.js', () => ({ getPackageJson }));
vi.mock('../utils/stdioHelpers.js', () => ({
writeStdoutLine,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const updateCommand: CommandModule = {
import('../config/settings.js'),
import('../ui/utils/updateCheck.js'),
import('../utils/installationInfo.js'),
import('../utils/standalone-update.js'),
import('../ui/standalone-update.js'),
import('../utils/stdioHelpers.js'),
import('../utils/updateEventEmitter.js'),
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

import type { LoadedSettings } from '../config/settings.js';
import { SettingScope } from '../config/settings.js';
import type { LoadedSettings } from './settings.js';
import { SettingScope } from './settings.js';
import { settingExistsInScope } from './settingsUtils.js';

/**
Expand Down
5 changes: 2 additions & 3 deletions packages/cli/src/config/loadedSettingsAdapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ import { SettingScope } from './settings.js';

// settingsUtils makes real fs calls in backup/restore — stub them out so the
// tests can focus on adapter behavior without touching disk.
vi.mock('../utils/settingsUtils.js', async (importOriginal) => {
const actual =
await importOriginal<typeof import('../utils/settingsUtils.js')>();
vi.mock('./settingsUtils.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('./settingsUtils.js')>();
return {
...actual,
backupSettingsFile: vi.fn(),
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/config/loadedSettingsAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
cleanupSettingsBackup,
restoreSettingsFromBackup,
getNestedProperty,
} from '../utils/settingsUtils.js';
} from './settingsUtils.js';

export function createLoadedSettingsAdapter(
settings: LoadedSettings,
Expand Down
Loading
Loading