Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 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
39 changes: 36 additions & 3 deletions packages/cli/src/serve/workspaceMemory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import {
WorkspaceMemoryFileTooLargeError,
WorkspaceMemoryWriteTimeoutError,
getAllGeminiMdFilenames,
LOCAL_CONTEXT_FILENAME,
QWEN_DIR,
writeWorkspaceContextFile,
type WriteContextFileScope,
} from '@qwen-code/qwen-code-core';
import { writeStderrLine } from '../utils/stdioHelpers.js';
import { isServeDebugMode } from './debugMode.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 Down Expand Up @@ -337,6 +347,29 @@ 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 workspace root.
const localPath = path.join(boundWorkspace, QWEN_DIR, LOCAL_CONTEXT_FILENAME);

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] Write path uses boundWorkspace (daemon's raw bound directory) while the read path in loadServerHierarchicalMemory uses findProjectRoot(resolvedCwd) (git root). In a monorepo where the daemon is bound to a subdirectory (e.g. /repo/packages/foo but git root is /repo), the POST route writes .qwen/QWEN.local.md to the subdirectory while the discovery path probes the git root's .qwen/ directory. Memory writes silently disappear from the prompt — the POST reports success with a filePath, GET shows the file exists, but the LLM never sees it.

Suggested change
const localPath = path.join(boundWorkspace, QWEN_DIR, LOCAL_CONTEXT_FILENAME);
// Resolve project root via findProjectRoot to match the discovery path.
const resolvedCwd = path.resolve(boundWorkspace);
const foundRoot = await findProjectRoot(resolvedCwd);
const effectiveRoot = foundRoot ?? resolvedCwd;
const localPath = path.join(effectiveRoot, QWEN_DIR, LOCAL_CONTEXT_FILENAME);

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

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
1 change: 1 addition & 0 deletions packages/core/src/memory/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

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';

// This variable will hold the currently configured filename for context files.
Expand Down
204 changes: 204 additions & 0 deletions packages/core/src/memory/writeContextFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import * as os from 'node:os';
import * as path from 'node:path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { Storage } from '../config/storage.js';
import { QWEN_DIR } from '../utils/paths.js';
import {
AGENT_CONTEXT_FILENAME,
DEFAULT_CONTEXT_FILENAME,
LOCAL_CONTEXT_FILENAME,
MEMORY_SECTION_HEADER,
setGeminiMdFilename,
} from './const.js';
Expand Down Expand Up @@ -391,4 +393,206 @@ describe('writeWorkspaceContextFile', () => {
setGeminiMdFilename(DEFAULT_CONTEXT_FILENAME);
}
});

describe('auto scope', () => {
let savedFilenames: string | string[];

beforeEach(() => {
// Capture whatever the filename list is before we touch it.
savedFilenames = [DEFAULT_CONTEXT_FILENAME, AGENT_CONTEXT_FILENAME];
setGeminiMdFilename(savedFilenames);
});

afterEach(() => {
setGeminiMdFilename(savedFilenames);
});

it('writes to project QWEN.md when one exists at the project root', async () => {
const projectQwenPath = path.join(workspace, DEFAULT_CONTEXT_FILENAME);
await fs.writeFile(projectQwenPath, '# project\n', 'utf8');

const result = await writeWorkspaceContextFile({
scope: 'auto',
mode: 'append',
content: '- memory entry',
projectRoot: workspace,
});

expect(result.filePath).toBe(projectQwenPath);
const written = await fs.readFile(projectQwenPath, 'utf8');
expect(written).toContain('- memory entry');
// Global dir must not have been touched.
const globalPath = path.join(globalDir, DEFAULT_CONTEXT_FILENAME);
await expect(fs.access(globalPath)).rejects.toMatchObject({
code: 'ENOENT',
});
});

it('writes to project AGENTS.md when that exists instead of QWEN.md', async () => {
const agentsPath = path.join(workspace, AGENT_CONTEXT_FILENAME);
await fs.writeFile(agentsPath, '# agents\n', 'utf8');

const result = await writeWorkspaceContextFile({
scope: 'auto',
mode: 'append',
content: '- agents memory',
projectRoot: workspace,
});

expect(result.filePath).toBe(agentsPath);
const written = await fs.readFile(agentsPath, 'utf8');
expect(written).toContain('- agents memory');
// QWEN.md must not be created.
await expect(
fs.access(path.join(workspace, DEFAULT_CONTEXT_FILENAME)),
).rejects.toMatchObject({ code: 'ENOENT' });
});

it('falls back to global when no project context file exists', async () => {
const result = await writeWorkspaceContextFile({
scope: 'auto',
mode: 'append',
content: '- fallback entry',
projectRoot: workspace,
});

expect(result.filePath).toBe(
path.join(globalDir, DEFAULT_CONTEXT_FILENAME),
);
expect(getGlobalQwenDirSpy).toHaveBeenCalled();
const written = await fs.readFile(result.filePath, 'utf8');
expect(written).toBe(MEMORY_SECTION_HEADER + '\n- fallback entry\n');
});

it('prefers QWEN.md over AGENTS.md when both exist (QWEN.md first in filenames list)', async () => {
const qwenPath = path.join(workspace, DEFAULT_CONTEXT_FILENAME);
const agentsPath = path.join(workspace, AGENT_CONTEXT_FILENAME);
await fs.writeFile(qwenPath, '# qwen\n', 'utf8');
await fs.writeFile(agentsPath, '# agents\n', 'utf8');

const result = await writeWorkspaceContextFile({
scope: 'auto',
mode: 'append',
content: '- which one?',
projectRoot: workspace,
});

// Should prefer QWEN.md since getAllGeminiMdFilenames returns it first.
expect(result.filePath).toBe(qwenPath);
const qwenContent = await fs.readFile(qwenPath, 'utf8');
expect(qwenContent).toContain('- which one?');
});

it('falls back to local file when no committed context exists', async () => {

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] This test verifies result.filePath and file content but never asserts that .gitignore was updated with the local context file entry. The production code at writeContextFile.ts:162-166 has a path-based guard to trigger ensureGitignoreEntry when auto resolves to local — this is entirely untested. A regression in the path comparison would silently skip the gitignore update.

Suggested change
it('falls back to local file when no committed context exists', async () => {
it('falls back to local file when no committed context exists', async () => {
const localPath = path.join(workspace, QWEN_DIR, LOCAL_CONTEXT_FILENAME);
await fs.mkdir(path.dirname(localPath), { recursive: true });
await fs.writeFile(localPath, '# local\n', 'utf8');
const result = await writeWorkspaceContextFile({
scope: 'auto',
mode: 'append',
content: '- auto local entry',
projectRoot: workspace,
});
expect(result.filePath).toBe(localPath);
const written = await fs.readFile(localPath, 'utf8');
expect(written).toContain('- auto local entry');
const gitignore = await fs.readFile(path.join(workspace, '.gitignore'), 'utf8');
expect(gitignore).toContain(`${QWEN_DIR}/${LOCAL_CONTEXT_FILENAME}`);
});

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

const localPath = path.join(workspace, QWEN_DIR, LOCAL_CONTEXT_FILENAME);
await fs.mkdir(path.dirname(localPath), { recursive: true });
await fs.writeFile(localPath, '# local\n', 'utf8');

const result = await writeWorkspaceContextFile({
scope: 'auto',
mode: 'append',
content: '- auto local entry',
projectRoot: workspace,
});

expect(result.filePath).toBe(localPath);
const written = await fs.readFile(localPath, 'utf8');
expect(written).toContain('- auto local entry');
});

it('prefers committed QWEN.md over local file in auto scope', async () => {
const qwenPath = path.join(workspace, DEFAULT_CONTEXT_FILENAME);
const localPath = path.join(workspace, QWEN_DIR, LOCAL_CONTEXT_FILENAME);
await fs.writeFile(qwenPath, '# qwen\n', 'utf8');
await fs.mkdir(path.dirname(localPath), { recursive: true });
await fs.writeFile(localPath, '# local\n', 'utf8');

const result = await writeWorkspaceContextFile({
scope: 'auto',
mode: 'append',
content: '- which scope?',
projectRoot: workspace,
});

expect(result.filePath).toBe(qwenPath);
});
});

describe('local scope', () => {
it('writes to .qwen/QWEN.local.md', async () => {
const result = await writeWorkspaceContextFile({
scope: 'local',
mode: 'append',
content: '- local entry',
projectRoot: workspace,
});

const expectedPath = path.join(
workspace,
QWEN_DIR,
LOCAL_CONTEXT_FILENAME,
);
expect(result.filePath).toBe(expectedPath);
const written = await fs.readFile(expectedPath, 'utf8');
expect(written).toBe(`${MEMORY_SECTION_HEADER}\n- local entry\n`);
});

it('creates the .qwen directory if missing', async () => {
const qwenDir = path.join(workspace, QWEN_DIR);
await expect(fs.access(qwenDir)).rejects.toMatchObject({
code: 'ENOENT',
});

await writeWorkspaceContextFile({
scope: 'local',
mode: 'append',
content: '- created',
projectRoot: workspace,
});

const stat = await fs.stat(qwenDir);
expect(stat.isDirectory()).toBe(true);
});

it('adds .qwen/QWEN.local.md to .gitignore on first write', async () => {
await writeWorkspaceContextFile({
scope: 'local',
mode: 'append',
content: '- local entry',
projectRoot: workspace,
});

const gitignore = await fs.readFile(
path.join(workspace, '.gitignore'),
'utf8',
);
expect(gitignore).toContain(`${QWEN_DIR}/${LOCAL_CONTEXT_FILENAME}`);
});

it('does not duplicate the gitignore entry on subsequent writes', async () => {
await writeWorkspaceContextFile({
scope: 'local',
mode: 'append',
content: '- first',
projectRoot: workspace,
});
await writeWorkspaceContextFile({
scope: 'local',
mode: 'append',
content: '- second',
projectRoot: workspace,
});

const gitignore = await fs.readFile(
path.join(workspace, '.gitignore'),
'utf8',
);
const count = gitignore
.split('\n')
.filter(
(l) => l.trim() === `${QWEN_DIR}/${LOCAL_CONTEXT_FILENAME}`,
).length;
expect(count).toBe(1);
});
});
});
Loading
Loading