Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions packages/acp-bridge/src/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,15 +307,17 @@ export interface ServeSessionSupportedCommandsStatus {
* so the SDK reducer can render any of these with one pattern.
*/

export type ServeContextFileScope = 'workspace' | 'global';
export type ServeContextFileScope = 'workspace' | 'local' | 'global';

export interface ServeWorkspaceMemoryFile {
kind: 'memory_file';
/** Absolute path to the discovered memory file. */
path: string;
/**
* 'workspace' for files under the bound workspace tree, 'global' for
* `~/.qwen/QWEN.md` style entries. Helps adapters render scope chips.
* 'workspace' for files under the bound workspace tree, 'local' for
* `.qwen/QWEN.local.md` (gitignored project-local instructions),
* 'global' for `~/.qwen/QWEN.md` style entries.
* Helps adapters render scope chips.
*/
scope: ServeContextFileScope;
/** Size in bytes of the file's serialized contents on disk. */
Expand Down
48 changes: 44 additions & 4 deletions packages/cli/src/serve/workspaceMemory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ import {
WorkspaceMemoryFileTooLargeError,
WorkspaceMemoryWriteTimeoutError,
getAllGeminiMdFilenames,
getLocalContextFilePath,
writeWorkspaceContextFile,
type WriteContextFileScope,
} from '@qwen-code/qwen-code-core';
import { findProjectRoot } from '@qwen-code/qwen-code-core';
import { writeStderrLine } from '../utils/stdioHelpers.js';
import { isServeDebugMode } from './debugMode.js';
import type { HttpAcpBridge } from './httpAcpBridge.js';
Expand Down Expand Up @@ -78,6 +81,13 @@ export interface WorkspaceMemoryRouteDeps {

const MAX_MEMORY_CONTENT_BYTES = 1024 * 1024;

const VALID_SCOPES: ReadonlySet<string> = new Set([

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.

[Suggestion] VALID_SCOPES is a runtime Set<string>, while WriteContextFileScope is a compile-time type union. They are maintained independently — adding a scope to one without the other causes silent drift (route rejects type-valid scopes, or resolver falls through to global for unknown scopes).

Derive the type from the set:

const VALID_SCOPES = ['workspace', 'global', 'local', 'auto'] as const;
type WriteContextFileScope = (typeof VALID_SCOPES)[number];

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

'workspace',
'global',
'local',
'auto',
]);

/** Mount the two memory routes on the supplied Express app. */
export function mountWorkspaceMemoryRoutes(
app: Application,
Expand Down Expand Up @@ -116,9 +126,9 @@ export function mountWorkspaceMemoryRoutes(
const body = deps.safeBody(req);

const scope = body['scope'];
if (scope !== 'workspace' && scope !== 'global') {
if (!VALID_SCOPES.has(scope as string)) {
res.status(400).json({
error: '`scope` must be "workspace" or "global"',
error: `\`scope\` must be one of: ${[...VALID_SCOPES].map((s) => `"${s}"`).join(', ')}`,
code: 'invalid_scope',
});
return;
Expand Down Expand Up @@ -183,7 +193,7 @@ export function mountWorkspaceMemoryRoutes(

try {
const result = await writeWorkspaceContextFile({

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.

[Suggestion] When scope='auto' resolves to global, writeContextFile.ts:182 throws a plain Error. The route's generic catch-all (line ~299) maps it to HTTP 500 with code: 'file_error', but this is semantically a client-side resolution failure ("create a project context file first"), not a server error. SDK consumers branching on HTTP status or code would misclassify this as a transient infrastructure failure.

Consider defining a typed error class (e.g. AutoScopeUnresolvableError) and catching it before the generic handler to return 400 with code: 'auto_scope_unresolvable'.

— qwen3.7-max via Qwen Code /review

scope,
scope: scope as WriteContextFileScope,

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.

[Critical] The POST handler now accepts 'local' and 'auto' scopes, and the raw scope value from the request body is passed directly into memory_changed events (line 218). However, DaemonMemoryChangedData.scope in packages/sdk-typescript/src/daemon/events.ts:149 is typed as 'workspace' | 'global', and the isMemoryChangedData type guard at line 1071 only checks for those two values. Events emitted with 'local' or 'auto' scope will fail the SDK type guard, causing SDK consumers to silently drop or misclassify these events.

Fix: Update DaemonMemoryChangedData.scope to include 'local' | 'auto' and update the isMemoryChangedData validator accordingly in events.ts.

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

mode,
content,
projectRoot: deps.boundWorkspace,
Expand All @@ -204,7 +214,7 @@ export function mountWorkspaceMemoryRoutes(
deps.bridge.publishWorkspaceEvent({
type: 'memory_changed',
data: {
scope,
scope: result.resolvedScope,

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.

[Suggestion] The synchronous POST response (line ~202) omits resolvedScope and resolvedRoot, so callers using scope: 'auto' must parse the filePath string to infer whether the write went to workspace, local, or global. The memory_changed SSE event here already emits scope: result.resolvedScope — add the same fields to the response body for consistency.

— qwen3.7-max via Qwen Code /review

filePath: result.filePath,
mode,
bytesWritten: result.bytesWritten,
Expand Down Expand Up @@ -337,6 +347,36 @@ async function collectWorkspaceMemoryStatus(
);

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.

[Suggestion] No test covers this new collectWorkspaceMemoryStatus local file probe (success or error path). Also, no route-level test verifies that scope: 'local' or scope: 'auto' are accepted by the POST route, or that the memory_changed event emits result.resolvedScope instead of the raw scope. Additionally, writeContextFile.ts:551 (WorkspaceMemoryFileTooLargeError / 16MB cap) has no test coverage.

— qwen3.7-max via Qwen Code /review

files.push(...workspaceFiles);

// Probe for .qwen/QWEN.local.md at the project root (git root, not
// necessarily the bound workspace). Uses findProjectRoot to match
// the discovery path in memoryDiscovery.ts — in a monorepo where
// the daemon is bound to a subdirectory, both must probe the same
// directory or writes will silently disappear from the prompt.
const resolvedWorkspace = path.resolve(boundWorkspace);
const foundProjectRoot = await findProjectRoot(resolvedWorkspace);
const effectiveRoot = foundProjectRoot ?? resolvedWorkspace;
const localPath = getLocalContextFilePath(effectiveRoot);
try {
const stat = await fs.stat(localPath);

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.

[Suggestion] fs.stat follows symlinks, but the prompt-loading path (memoryDiscovery.ts:410) uses fs.lstat and skips symlinks, and the write path (writeContextFile.ts:333 assertNotSymlink) refuses them. A symlinked .qwen/QWEN.local.md would appear healthy here (reporting the target's stat.size) while being silently absent from the prompt and rejected on write — three different behaviors for the same file.

Suggested change
const stat = await fs.stat(localPath);
const stat = await fs.lstat(localPath);
if (stat.isSymbolicLink()) {
// Skip — consistent with memoryDiscovery.ts and writeContextFile.ts
} else if (stat.isFile()) {

— qwen3.7-max via Qwen Code /review

if (stat.isFile()) {
files.push({
absolutePath: localPath,
scope: 'local',

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.

[Critical] [typecheck] TS2322: Type '"local"' is not assignable to type 'ServeContextFileScope'. The object literal here uses absolutePath but ServeWorkspaceMemoryFile expects path, and is missing the required kind: 'memory_file' property. TypeScript cannot unify the literal 'local' with the union type when the enclosing object shape diverges from the interface.

Suggested change
scope: 'local',
files.push({
kind: 'memory_file' as const,
path: localPath,
scope: 'local' as const,
bytes: stat.size,
});

— qwen3.7-max via Qwen Code /review

bytes: stat.size,
});
}
} catch (err) {
if (!isEnoent(err)) {
errors.push({
kind: 'memory_file',
status: 'error',
error: err instanceof Error ? err.message : String(err),
errorKind: 'stat_failed',
hint: localPath,
});
}
}

const globalDir = Storage.getGlobalQwenDir();
for (const filename of filenames) {
const candidate = path.join(globalDir, filename);
Expand Down
14 changes: 14 additions & 0 deletions packages/core/src/memory/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,24 @@
* SPDX-License-Identifier: Apache-2.0
*/

import * as path from 'node:path';
import { QWEN_DIR } from '../utils/paths.js';

export const DEFAULT_CONTEXT_FILENAME = 'QWEN.md';
export const AGENT_CONTEXT_FILENAME = 'AGENTS.md';
export const LOCAL_CONTEXT_FILENAME = 'QWEN.local.md';

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.

[Suggestion] LOCAL_CONTEXT_FILENAME is duplicated verbatim in packages/core/src/tools/memory-config.ts:15. These two files are complete duplicates (identical constants, state, and functions). All runtime imports use memory/const.ts; memory-config.ts has zero importers (only used in a vi.mock() call). Adding another constant to both files means another line that must be kept in sync manually — risk of silent drift.

Fix: Consolidate — have memory-config.ts re-export from const.ts, or remove the duplicate entirely.

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

export const MEMORY_SECTION_HEADER = '## Qwen Added Memories';

/**
* Returns the absolute path to `.qwen/QWEN.local.md` for a given
* project root. Centralized here so all call sites use a single
* source of truth — if the directory name or filename changes, only
* this function needs updating.
*/
export function getLocalContextFilePath(projectRoot: string): string {
return path.join(projectRoot, QWEN_DIR, LOCAL_CONTEXT_FILENAME);
}

// This variable will hold the currently configured filename for context files.
// It defaults to include both QWEN.md and AGENTS.md but can be overridden by setGeminiMdFilename.
// QWEN.md is first to maintain backward compatibility (used by /init command tool).
Expand Down
Loading