diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index c0b0cb14f1d..f125aac7847 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -35,6 +35,9 @@ import { CommandKind } from '../../ui/commands/types.js'; import { MessageType } from '../../ui/types.js'; const debugLoggerWarnSpy = vi.hoisted(() => vi.fn()); +// Records every LoopTickResolver construction's deps so a test can assert what +// Session computed (e.g. the home confinement root) without a private-field peek. +const loopTickResolverDepsSpy = vi.hoisted(() => vi.fn()); vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { const actual = @@ -49,6 +52,16 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { }), generatePromptSuggestion: vi.fn(), logPromptSuggestion: vi.fn(), + // Transparent recording wrapper: records the constructor deps, then behaves + // exactly like the real resolver (subclass → instanceof + methods preserved). + LoopTickResolver: class extends actual.LoopTickResolver { + constructor( + ...args: ConstructorParameters + ) { + loopTickResolverDepsSpy(args[0]); + super(...args); + } + }, }; }); @@ -158,6 +171,26 @@ function createEmptyStream() { return (async function* () {})(); } +/** + * Points os.homedir() at `home` for a test by overriding the env vars libuv + * reads (HOME on POSIX, USERPROFILE on Windows) — the module export itself can't + * be spied under ESM. Returns a restore function. + */ +function setFakeHome(home: string): () => void { + const prev = { + HOME: process.env['HOME'], + USERPROFILE: process.env['USERPROFILE'], + }; + process.env['HOME'] = home; + process.env['USERPROFILE'] = home; + return () => { + for (const key of ['HOME', 'USERPROFILE'] as const) { + if (prev[key] === undefined) delete process.env[key]; + else process.env[key] = prev[key]; + } + }; +} + // Helper to create async generator with chunks (avoids memory leak) function createStreamWithChunks( chunks: Array<{ type: unknown; value: unknown }>, @@ -335,6 +368,9 @@ describe('Session', () => { getModel: vi.fn().mockImplementation(() => currentModel), getSessionId: vi.fn().mockReturnValue('test-session-id'), getWorkingDir: vi.fn().mockReturnValue(process.cwd()), + // Folder trust gates the project `.qwen/loop.md`; default trusted (the + // production default). Untrusted-folder tests override to false. + isTrustedFolder: vi.fn().mockReturnValue(true), getTelemetryLogPromptsEnabled: vi.fn().mockReturnValue(false), getUsageStatisticsEnabled: vi.fn().mockReturnValue(false), getContentGeneratorConfig: vi.fn().mockReturnValue(undefined), @@ -4420,6 +4456,1589 @@ describe('Session', () => { }); }); + it('expands a loop.md sentinel into the task block and echoes a clean label', async () => { + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-session-'), + ); + const loopMdPath = path.join(tmpDir, '.qwen', 'loop.md'); + await fs.mkdir(path.dirname(loopMdPath), { recursive: true }); + await fs.writeFile(loopMdPath, '- finish the migration'); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ + prompt: '<>', + cronExpr: '@wakeup', + }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(createEmptyStream()) + .mockResolvedValueOnce(createEmptyStream()); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // The client sees a stable RELATIVE label, never the raw sentinel or + // the absolute path (which would leak the OS username / dir layout). + await vi.waitFor(() => { + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'user_message_chunk', + content: { + type: 'text', + text: 'Loop tick — tasks from project loop.md', + }, + _meta: { source: 'loop' }, + }, + }); + }); + // The absolute loop.md path must not appear in any client echo. + const echoedTexts = ( + mockClient.sessionUpdate as ReturnType + ).mock.calls + .map((call) => call[0]?.update?.content?.text) + .filter((text): text is string => typeof text === 'string'); + for (const text of echoedTexts) { + expect(text).not.toContain(loopMdPath); + } + + // The model receives the expanded full task block, not the sentinel. + let block = ''; + await vi.waitFor(() => { + const cronCall = ( + mockChat.sendMessageStream as ReturnType + ).mock.calls.find( + (c) => + Array.isArray(c[1]?.message) && + c[1].message.some((p: { text?: string }) => + p.text?.includes('finish the migration'), + ), + ); + expect(cronCall).toBeDefined(); + block = (cronCall![1].message as Array<{ text?: string }>) + .map((p) => p.text ?? '') + .join(''); + }); + expect(block).toContain('# /loop tick — loop.md tasks from'); + expect(block).toContain('- finish the migration'); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); + + it('delivers the full block then a SHORT REMINDER on an unchanged second tick', async () => { + // Two ticks of the same sentinel over unchanged loop.md: tick1 delivers + // the FULL block (INTRO + task body) and commits it; tick2 sees the + // unchanged content and delivers the one-line SHORT REMINDER (full:false) + // — a pure pointer with neither the INTRO nor the body. The client echo + // still names the source on the reminder (sourceLabel set), so this pins + // the full:false/labelled-reminder path through BOTH the echo and the + // model-message paths. + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-reminder-'), + ); + const loopMdPath = path.join(tmpDir, '.qwen', 'loop.md'); + await fs.mkdir(path.dirname(loopMdPath), { recursive: true }); + await fs.writeFile(loopMdPath, '- finish the migration'); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + // Drained serially against the one persistent resolver, so tick2 + // sees tick1's committed content as unchanged. + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + const cronModelTexts = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .filter((c) => Array.isArray(c[1]?.message)) + .map((c) => + (c[1].message as Array<{ text?: string }>) + .map((p) => p.text ?? '') + .join(''), + ); + + await vi.waitFor(() => { + const texts = cronModelTexts(); + // Exactly one FULL delivery (INTRO) and one SHORT REMINDER (preamble). + const full = texts.filter((t) => + t.includes('The user configured a loop-tasks file.'), + ); + const reminder = texts.filter((t) => + t.includes( + 'Work the tasks from the loop.md contents established earlier', + ), + ); + expect(full).toHaveLength(1); + expect(reminder).toHaveLength(1); + // The reminder is a pointer only: no INTRO and no task body (which + // the full block already paid into the cached prefix). + expect(reminder[0]).not.toContain( + 'The user configured a loop-tasks file.', + ); + expect(reminder[0]).not.toContain('- finish the migration'); + }); + + // full:false reminder still resolves a sourceLabel, so its client echo + // names the source — identical to the full tick's echo (both ticks). + const labelledEchoes = ( + mockClient.sessionUpdate as ReturnType + ).mock.calls.filter( + (c) => + c[0]?.update?.sessionUpdate === 'user_message_chunk' && + c[0]?.update?.content?.text === + 'Loop tick — tasks from project loop.md', + ).length; + expect(labelledEchoes).toBe(2); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); + + it('rebuilds the loop.md resolver when the working dir changes between ticks', async () => { + // /cd mid-session: the resolver is cached per project root, so a working- + // dir change must rebuild it for the NEW root. Two ticks of the same + // sentinel — the first resolves the OLD root's loop.md; getWorkingDir then + // flips and the second must resolve the NEW root's loop.md (a fresh + // resolver → full delivery), never re-serving the OLD root's content. + // Mutation check: drop the `loopTickResolverRoot !== root` rebuild guard + // and tick2 reuses the OLD resolver — the NEW content never reaches the + // model (the unchanged OLD content is re-served as a short reminder). + const oldDir = await fs.mkdtemp(path.join(os.tmpdir(), 'loop-md-old-')); + const newDir = await fs.mkdtemp(path.join(os.tmpdir(), 'loop-md-new-')); + await fs.mkdir(path.join(oldDir, '.qwen'), { recursive: true }); + await fs.mkdir(path.join(newDir, '.qwen'), { recursive: true }); + await fs.writeFile( + path.join(oldDir, '.qwen', 'loop.md'), + '- task from OLD root', + ); + await fs.writeFile( + path.join(newDir, '.qwen', 'loop.md'), + '- task from NEW root', + ); + + let currentRoot = oldDir; + mockConfig.getWorkingDir = vi.fn(() => currentRoot); + + let fire: + | ((job: { prompt: string; cronExpr?: string }) => void) + | undefined; + const scheduler = { + size: 1, + hasPendingWork: true, + enableDurable: vi.fn().mockResolvedValue(undefined), + // Capture the fire callback so the test can drive ticks one at a time, + // flipping the working dir in between. + start: vi.fn( + (cb: (job: { prompt: string; cronExpr?: string }) => void) => { + fire = cb; + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + // Bootstraps the scheduler and captures `fire`; no tick fires yet. + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + await vi.waitFor(() => expect(fire).toBeDefined()); + + const cronModelTexts = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .filter((c) => Array.isArray(c[1]?.message)) + .map((c) => + (c[1].message as Array<{ text?: string }>) + .map((p) => p.text ?? '') + .join(''), + ); + + // Tick 1 resolves against the OLD root. Waiting for its content in the + // model proves the resolve consumed oldDir before we flip (race-free: + // the model send is downstream of the resolve). + fire!({ prompt: '<>', cronExpr: '*/5 * * * *' }); + await vi.waitFor(() => { + expect( + cronModelTexts().some((t) => t.includes('task from OLD root')), + ).toBe(true); + }); + + // /cd: the resolver must rebuild for the new root on the next tick. + currentRoot = newDir; + + // Tick 2 must resolve the NEW root's loop.md (fresh resolver → full). + fire!({ prompt: '<>', cronExpr: '*/5 * * * *' }); + await vi.waitFor(() => { + expect( + cronModelTexts().some((t) => t.includes('task from NEW root')), + ).toBe(true); + }); + + // The NEW-root tick carries ONLY the new root's tasks — the old root's + // content is not re-resolved after the dir change. + const newMsg = cronModelTexts().find((t) => + t.includes('task from NEW root'), + )!; + expect(newMsg).not.toContain('task from OLD root'); + } finally { + await fs.rm(oldDir, { recursive: true, force: true }); + await fs.rm(newDir, { recursive: true, force: true }); + } + }); + + it('does not expand the project loop.md sentinel in an untrusted folder', async () => { + // An untrusted folder's repo-controlled .qwen/loop.md must not be read + // and fed to the model. With no user-owned ~/.qwen/loop.md, the tick is + // a labelled no-op — and the repo task block never reaches the model. + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-untrusted-'), + ); + const fakeHome = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-home-'), + ); + const loopMdPath = path.join(tmpDir, '.qwen', 'loop.md'); + await fs.mkdir(path.dirname(loopMdPath), { recursive: true }); + await fs.writeFile(loopMdPath, '- finish the migration'); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + mockConfig.isTrustedFolder = vi.fn().mockReturnValue(false); + // Point os.homedir() at an empty fake home (libuv reads HOME/USERPROFILE) + // so there is no user-owned loop.md and the tick is deterministically + // absent — the module export can't be spied under ESM. + const restoreHome = setFakeHome(fakeHome); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ + prompt: '<>', + cronExpr: '@wakeup', + }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(createEmptyStream()) + .mockResolvedValueOnce(createEmptyStream()); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // The client sees the absent label, never the repo file's path. + await vi.waitFor(() => { + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'user_message_chunk', + content: { + type: 'text', + text: 'Loop tick — loop.md not present', + }, + _meta: { source: 'loop' }, + }, + }); + }); + + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => + Array.isArray(c[1]?.message) ? c[1].message : [], + ) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + await vi.waitFor(() => { + expect(sentToModel()).toContain('# /loop tick — loop.md absent'); + }); + // The repo-controlled task block never reaches the model. + expect(sentToModel()).not.toContain('finish the migration'); + } finally { + restoreHome(); + await fs.rm(tmpDir, { recursive: true, force: true }); + await fs.rm(fakeHome, { recursive: true, force: true }); + } + }); + + it('keeps the home confinement root non-empty when os.homedir() is empty (no QWEN_HOME)', async () => { + // Minimal containers with no HOME make os.homedir() === ''. With QWEN_HOME + // unset the home confinement root must NOT collapse to '': isWithin('', + // anyPath) is trivially true, so an empty root lets a home + // `~/.qwen/loop.md` symlink resolve anywhere and bypass the confinement. + // The guard falls back to the parent of the global qwen dir + // (Storage.getGlobalQwenDir(), itself empty-home-safe), which is the + // homeQwenDir Session passes to the resolver. + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-nohome-'), + ); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + // HOME='' makes libuv's os.homedir() return '' on every platform — it + // null-checks HOME, never its emptiness. + const restoreHome = setFakeHome(''); + const prevQwenHome = process.env['QWEN_HOME']; + delete process.env['QWEN_HOME']; + loopTickResolverDepsSpy.mockClear(); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '@wakeup' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => { + expect(loopTickResolverDepsSpy).toHaveBeenCalled(); + }); + + const deps = loopTickResolverDepsSpy.mock.calls.at(-1)![0] as { + homeDir: string; + homeQwenDir?: string; + }; + // Without the `|| path.dirname(homeQwenDir)` guard this would be '' + // (os.homedir()); the guard makes it the non-empty parent of the + // empty-home-safe global qwen dir. + expect(deps.homeDir).not.toBe(''); + expect(deps.homeDir).toBe(path.dirname(deps.homeQwenDir!)); + } finally { + if (prevQwenHome === undefined) delete process.env['QWEN_HOME']; + else process.env['QWEN_HOME'] = prevQwenHome; + restoreHome(); + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); + + it('reads the home loop.md from QWEN_HOME, not the real ~/.qwen', async () => { + // The home/global candidate must honor QWEN_HOME (the relocated global + // dir) instead of always reading the real OS home. Point QWEN_HOME at a + // dir holding loop.md, leave the project dir and fake $HOME empty, and + // confirm the relocated file's block reaches the model. + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-qwenhome-proj-'), + ); + const fakeHome = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-qwenhome-home-'), + ); + const qwenHome = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-qwenhome-dir-'), + ); + await fs.writeFile( + path.join(qwenHome, 'loop.md'), + '- relocated home task', + ); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + const restoreHome = setFakeHome(fakeHome); + const prevQwenHome = process.env['QWEN_HOME']; + process.env['QWEN_HOME'] = qwenHome; + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // Echo names the home source (sourceLabel='home loop.md'), proving the + // home candidate resolved from QWEN_HOME rather than the empty $HOME. + await vi.waitFor(() => { + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'user_message_chunk', + content: { + type: 'text', + text: 'Loop tick — tasks from home loop.md', + }, + // `*/5 * * * *` is a recurring cron (not an @wakeup), so the + // echo carries source 'cron' (see job.cronExpr mapping). + _meta: { source: 'cron' }, + }, + }); + }); + + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => + Array.isArray(c[1]?.message) ? c[1].message : [], + ) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + await vi.waitFor(() => { + expect(sentToModel()).toContain('- relocated home task'); + }); + } finally { + restoreHome(); + if (prevQwenHome === undefined) delete process.env['QWEN_HOME']; + else process.env['QWEN_HOME'] = prevQwenHome; + await fs.rm(tmpDir, { recursive: true, force: true }); + await fs.rm(fakeHome, { recursive: true, force: true }); + await fs.rm(qwenHome, { recursive: true, force: true }); + } + }); + + it('propagates a sentinel resolve() error (EACCES) without leaking the absolute path to the client', async () => { + // #executeCronPrompt: when resolve() throws (e.g. EACCES on + // .qwen/loop.md) it logs a loop.md-specific warn and RE-THROWS into the + // cron catch. Regression guard: the failure must PROPAGATE (surface as a + // cron error, never degrade to a default/normal tick sent to the model) + // and the loop.md-tagged warn must fire so a resolution failure stays + // distinguishable from a model-call failure in logs. + // + // Security guard: the raw fs error message embeds the ABSOLUTE loop.md + // path (OS username + dir layout). The cron catch forwards error.message + // verbatim to the client via emitAgentMessage, so the re-thrown error's + // message must be SANITIZED — relative label + errno code only, never the + // absolute path. The full detail stays in the LOCAL debug warn. + debugLoggerWarnSpy.mockClear(); + const absoluteLoopMdPath = '/home/alice/project/.qwen/loop.md'; + const eacces = Object.assign( + new Error(`EACCES: permission denied, open '${absoluteLoopMdPath}'`), + { code: 'EACCES' }, + ); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eacces); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // The loop.md-specific warn fired, tagged with the sentinel mode and + // the EACCES code (proving the failure was logged as a resolution + // failure, not a generic model error). The raw error — whose message + // carries the absolute path — is passed as the second arg so the full + // detail is kept in this LOCAL log (debug logs are never sent to the + // client). + await vi.waitFor(() => { + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + 'loop.md sentinel resolution failed (mode=cron, code=EACCES) — check .qwen/loop.md permissions/IO', + eacces, + ); + }); + + // The error PROPAGATED to the cron catch and surfaced to the client, + // but SANITIZED: the emitted message names the relative candidate + // labels + errno code and NEVER the raw absolute loop.md path. + const sessionUpdateMock = mockClient.sessionUpdate as ReturnType< + typeof vi.fn + >; + const cronErrorTexts = () => + sessionUpdateMock.mock.calls + .map( + (call) => + ( + call[0] as { + update?: { + sessionUpdate?: string; + content?: { text?: string }; + }; + } + ).update, + ) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text) => text.includes('[cron error]')); + await vi.waitFor(() => + expect(cronErrorTexts().length).toBeGreaterThan(0), + ); + for (const text of cronErrorTexts()) { + // Relative label + errno code present... + expect(text).toContain('EACCES'); + expect(text).toContain('.qwen/loop.md (project)'); + // ...and NO absolute path leaked to the client/API. + expect(text).not.toContain(absoluteLoopMdPath); + expect(text).not.toContain('/home/alice'); + } + + // It was NOT swallowed into a normal tick: resolve() threw before any + // model send, so neither an expanded `# /loop tick` block nor the raw + // sentinel ever reached the model (the model is only sent the user + // prompt, never a degraded default tick). + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => + Array.isArray(c[1]?.message) ? c[1].message : [], + ) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + expect(sentToModel()).not.toContain('# /loop tick'); + expect(sentToModel()).not.toContain('<>'); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('names the QWEN_HOME-aware home path in the sanitized resolve error, not a hardcoded ~/.qwen', async () => { + // Regression: the sanitized resolve-error hardcoded `~/.qwen/loop.md + // (home)`, but the resolver's home candidate is QWEN_HOME-aware. With + // QWEN_HOME relocated OUTSIDE $HOME, the error reuses homeLoopLabel(), + // which names it via the literal `$QWEN_HOME/loop.md` — leak-safe (never + // the resolved absolute global dir, nor the absolute project path). + debugLoggerWarnSpy.mockClear(); + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-err-proj-'), + ); + const fakeHome = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-err-home-'), + ); + const qwenHome = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-err-qwenhome-'), + ); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + const restoreHome = setFakeHome(fakeHome); + const prevQwenHome = process.env['QWEN_HOME']; + process.env['QWEN_HOME'] = qwenHome; + // qwenHome is under os.tmpdir() (not the OS home), so tildeifyPath is a + // no-op there. The label is MODEL/client-facing, so it must read as the + // literal `$QWEN_HOME/loop.md`, never the resolved absolute path. + const expectedHomeLabel = `$QWEN_HOME/loop.md (home)`; + + const eacces = Object.assign( + new Error( + `EACCES: permission denied, open '${path.join(tmpDir, '.qwen', 'loop.md')}'`, + ), + { code: 'EACCES' }, + ); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eacces); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + const sessionUpdateMock = mockClient.sessionUpdate as ReturnType< + typeof vi.fn + >; + const cronErrorTexts = () => + sessionUpdateMock.mock.calls + .map( + (call) => + ( + call[0] as { + update?: { + sessionUpdate?: string; + content?: { text?: string }; + }; + } + ).update, + ) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text) => text.includes('[cron error]')); + await vi.waitFor(() => + expect(cronErrorTexts().length).toBeGreaterThan(0), + ); + for (const text of cronErrorTexts()) { + // The QWEN_HOME-aware home path is named... + expect(text).toContain(expectedHomeLabel); + expect(text).toContain('.qwen/loop.md (project)'); + // ...and the old hardcoded label is gone. + expect(text).not.toContain('~/.qwen/loop.md'); + // Still leak-safe: neither the absolute project path nor the + // resolved $QWEN_HOME global dir reaches the client/API. + expect(text).not.toContain(path.join(tmpDir, '.qwen', 'loop.md')); + expect(text).not.toContain(path.join(qwenHome, 'loop.md')); + } + } finally { + resolveSpy.mockRestore(); + restoreHome(); + if (prevQwenHome === undefined) delete process.env['QWEN_HOME']; + else process.env['QWEN_HOME'] = prevQwenHome; + await fs.rm(tmpDir, { recursive: true, force: true }); + await fs.rm(fakeHome, { recursive: true, force: true }); + await fs.rm(qwenHome, { recursive: true, force: true }); + } + }); + + it('omits the project candidate from the sanitized resolve error in an untrusted folder', async () => { + // An untrusted folder never reads `.qwen/loop.md` (the resolver gets + // allowProjectFile=false), so the sanitized error must NOT claim the + // project candidate was checked — it would be a lie. It still names the + // QWEN_HOME-aware home candidate (the only one actually probed) and the + // errno code, and stays leak-safe. Mutation guard: hardcoding + // `.qwen/loop.md (project)` back into the throw re-introduces the false + // claim and fails this test. + debugLoggerWarnSpy.mockClear(); + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-untrusted-err-'), + ); + const fakeHome = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-untrusted-home-'), + ); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + mockConfig.isTrustedFolder = vi.fn().mockReturnValue(false); + const restoreHome = setFakeHome(fakeHome); + + const absoluteLoopMdPath = path.join(tmpDir, '.qwen', 'loop.md'); + const eacces = Object.assign( + new Error(`EACCES: permission denied, open '${absoluteLoopMdPath}'`), + { code: 'EACCES' }, + ); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eacces); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + const sessionUpdateMock = mockClient.sessionUpdate as ReturnType< + typeof vi.fn + >; + const cronErrorTexts = () => + sessionUpdateMock.mock.calls + .map( + (call) => + ( + call[0] as { + update?: { + sessionUpdate?: string; + content?: { text?: string }; + }; + } + ).update, + ) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text) => text.includes('[cron error]')); + await vi.waitFor(() => + expect(cronErrorTexts().length).toBeGreaterThan(0), + ); + for (const text of cronErrorTexts()) { + // The home candidate and errno code are named... + expect(text).toContain('EACCES'); + expect(text).toContain('(home)'); + // ...but the never-read project candidate is omitted entirely. + expect(text).not.toContain('(project)'); + // ...and the absolute path is still never leaked to the client/API. + expect(text).not.toContain(absoluteLoopMdPath); + } + } finally { + resolveSpy.mockRestore(); + restoreHome(); + await fs.rm(tmpDir, { recursive: true, force: true }); + await fs.rm(fakeHome, { recursive: true, force: true }); + } + }); + + it('threads one captured folder-trust into both the resolve probe and the sanitized error', async () => { + // FIX 3: isTrustedFolder() can flip mid-tick (IDE workspace-trust + // update). Capturing it ONCE and threading it to BOTH resolve() and the + // error's absentLocations() keeps the sanitized error naming the SAME + // candidate set that was probed. Assert the trust handed to resolve() is + // identical to the one handed to absentLocations(). Mutation guard: + // reverting to two separate isTrustedFolder() reads drops the resolve() + // trust arg (undefined), so the two no longer match. + debugLoggerWarnSpy.mockClear(); + const eacces = Object.assign( + new Error("EACCES: permission denied, open '/home/x/.qwen/loop.md'"), + { code: 'EACCES' }, + ); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eacces); + const absentSpy = vi.spyOn( + core.LoopTickResolver.prototype, + 'absentLocations', + ); + mockConfig.isTrustedFolder = vi.fn().mockReturnValue(true); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => expect(resolveSpy).toHaveBeenCalled()); + await vi.waitFor(() => expect(absentSpy).toHaveBeenCalled()); + // resolve() was probed with the captured trust as its 2nd arg, and the + // error's absentLocations() got the SAME value — one capture, both + // paths agree. + const probedTrust = resolveSpy.mock.calls[0][1]; + const erroredTrust = absentSpy.mock.calls[0][0]; + expect(probedTrust).toBe(true); + expect(erroredTrust).toBe(true); + expect(probedTrust).toBe(erroredTrust); + } finally { + resolveSpy.mockRestore(); + absentSpy.mockRestore(); + } + }); + + it('keeps a dynamic loop alive on a transient resolve error (no throw, re-arm tick)', async () => { + // FIX 4: a `dynamic` loop is re-armed only by the model at end-of-turn, + // and the firing wakeup was already consumed. A transient, non-whitelisted + // resolve error (EIO) must NOT throw (no turn → no re-arm → silent death) + // — it degrades to a no-op tick that mirrors the absent path AND carries + // the dynamic re-arm instruction, so the model re-arms and the loop + // survives. Mutation guard: drop the `dynamic` branch (always throw) and a + // `[loop error]` surfaces while no tick reaches the model. + debugLoggerWarnSpy.mockClear(); + const eio = Object.assign(new Error('EIO: i/o error, read'), { + code: 'EIO', + }); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eio); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '@wakeup' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => (Array.isArray(c[1]?.message) ? c[1].message : [])) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + const errorEchoes = () => + (mockClient.sessionUpdate as ReturnType).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text: string) => text.includes('error]')); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // The degraded no-op tick reached the model (the turn ran → no throw). + await vi.waitFor(() => { + expect(sentToModel()).toContain( + '# /loop tick — loop.md unavailable (dynamic pacing)', + ); + }); + // It carries the dynamic re-arm instruction (the literal sentinel) and + // the errno note, so the loop continues. + expect(sentToModel()).toContain('<>'); + expect(sentToModel()).toContain('could not be read this tick (EIO)'); + // The CLIENT echo distinguishes a transient read failure (file present, + // unreadable this tick) from a genuinely-absent file: it must say + // "temporarily unavailable", never the misleading "not present". + // Mutation guard: drop the transientError flag/echo branch and the echo + // regresses to "not present", failing both assertions below. + const loopEchoes = ( + mockClient.sessionUpdate as ReturnType + ).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'user_message_chunk') + .map((u) => u?.content?.text ?? ''); + expect(loopEchoes).toContain( + 'Loop tick — loop.md temporarily unavailable', + ); + expect(loopEchoes).not.toContain('Loop tick — loop.md not present'); + // It did NOT surface as a loop/cron error (the loop did not die). + expect(errorEchoes()).toHaveLength(0); + // The real errno is still recorded in the LOCAL debug warn. + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + 'loop.md sentinel resolution failed (mode=dynamic, code=EIO) — check .qwen/loop.md permissions/IO', + eio, + ); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('still throws on a transient resolve error for a cron loop (no degraded tick)', async () => { + // The cron counterpart to the dynamic-survival path: cron re-fires on its + // own next interval, so a transient resolve error STILL propagates + // (sanitized) rather than degrading to a model tick. Mutation guard: + // widening the dynamic no-throw branch to cron would send a `# /loop tick` + // block instead of surfacing the error. + debugLoggerWarnSpy.mockClear(); + const eio = Object.assign(new Error('EIO: i/o error, read'), { + code: 'EIO', + }); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eio); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + const cronErrorTexts = () => + (mockClient.sessionUpdate as ReturnType).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text: string) => text.includes('[cron error]')); + await vi.waitFor(() => + expect(cronErrorTexts().length).toBeGreaterThan(0), + ); + // Sanitized error carries the errno; no degraded loop tick was sent. + for (const text of cronErrorTexts()) { + expect(text).toContain('EIO'); + } + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => + Array.isArray(c[1]?.message) ? c[1].message : [], + ) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + expect(sentToModel()).not.toContain('# /loop tick'); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('keeps a dynamic loop alive on a transient EACCES resolve error', async () => { + // EACCES is in TRANSIENT_FS_CODES, so a `dynamic` loop degrades to a + // no-op re-arm tick (same survival as the EIO case) rather than dying. + debugLoggerWarnSpy.mockClear(); + const eacces = Object.assign(new Error('EACCES: permission denied'), { + code: 'EACCES', + }); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eacces); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '@wakeup' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => (Array.isArray(c[1]?.message) ? c[1].message : [])) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + const errorEchoes = () => + (mockClient.sessionUpdate as ReturnType).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text: string) => text.includes('error]')); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // The degraded no-op tick reached the model (the turn ran → no throw), + // carrying the dynamic re-arm sentinel and the EACCES errno note. + await vi.waitFor(() => { + expect(sentToModel()).toContain( + '# /loop tick — loop.md unavailable (dynamic pacing)', + ); + }); + expect(sentToModel()).toContain('<>'); + expect(sentToModel()).toContain( + 'could not be read this tick (EACCES)', + ); + // The loop did NOT surface an error (it survived). + expect(errorEchoes()).toHaveLength(0); + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + 'loop.md sentinel resolution failed (mode=dynamic, code=EACCES) — check .qwen/loop.md permissions/IO', + eacces, + ); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('keeps a dynamic loop alive on a transient EISDIR resolve error', async () => { + // EISDIR is in TRANSIENT_FS_CODES (the lstat→open TOCTOU race: the path is + // swapped to a directory between the pre-open lstat and fs.open). A + // `dynamic` loop must degrade to a no-op re-arm tick — same survival as the + // EACCES/EIO cases — instead of dying. Mutation guard: drop EISDIR from the + // set and this throw falls through to the sanitized `[loop error]` re-throw. + debugLoggerWarnSpy.mockClear(); + const eisdir = Object.assign( + new Error('EISDIR: illegal operation on a directory, read'), + { code: 'EISDIR' }, + ); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eisdir); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '@wakeup' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => (Array.isArray(c[1]?.message) ? c[1].message : [])) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + const errorEchoes = () => + (mockClient.sessionUpdate as ReturnType).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text: string) => text.includes('error]')); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // The degraded no-op tick reached the model (the turn ran → no throw), + // carrying the dynamic re-arm sentinel and the EISDIR errno note. + await vi.waitFor(() => { + expect(sentToModel()).toContain( + '# /loop tick — loop.md unavailable (dynamic pacing)', + ); + }); + expect(sentToModel()).toContain('<>'); + expect(sentToModel()).toContain( + 'could not be read this tick (EISDIR)', + ); + // The loop did NOT surface an error (it survived). + expect(errorEchoes()).toHaveLength(0); + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + 'loop.md sentinel resolution failed (mode=dynamic, code=EISDIR) — check .qwen/loop.md permissions/IO', + eisdir, + ); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('keeps a dynamic loop alive on a transient ENOTDIR resolve error', async () => { + // ENOTDIR is the sibling TOCTOU code (a path component swapped to a + // non-directory between the lstat and fs.open). Like EISDIR it must degrade + // a `dynamic` loop to a no-op re-arm tick rather than killing it. + debugLoggerWarnSpy.mockClear(); + const enotdir = Object.assign( + new Error('ENOTDIR: not a directory, open'), + { code: 'ENOTDIR' }, + ); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(enotdir); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '@wakeup' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => (Array.isArray(c[1]?.message) ? c[1].message : [])) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + const errorEchoes = () => + (mockClient.sessionUpdate as ReturnType).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text: string) => text.includes('error]')); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => { + expect(sentToModel()).toContain( + '# /loop tick — loop.md unavailable (dynamic pacing)', + ); + }); + expect(sentToModel()).toContain('<>'); + expect(sentToModel()).toContain( + 'could not be read this tick (ENOTDIR)', + ); + expect(errorEchoes()).toHaveLength(0); + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + 'loop.md sentinel resolution failed (mode=dynamic, code=ENOTDIR) — check .qwen/loop.md permissions/IO', + enotdir, + ); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('re-throws (does NOT degrade) a dynamic loop on a NON-fs resolve error', async () => { + // The gate's reason for existing: a non-transient error (a TypeError / + // programming bug → code 'unknown') is NOT in TRANSIENT_FS_CODES, so the + // `dynamic` branch must NOT degrade to an infinite silent no-op cycle. It + // falls through to the sanitized throw so the real bug surfaces. + // Mutation guard: drop the `&& TRANSIENT_FS_CODES.includes(code)` gate and + // 'unknown' degrades — a `# /loop tick` reaches the model and no + // `[loop error]` surfaces, failing both assertions below. + debugLoggerWarnSpy.mockClear(); + const bug = new TypeError( + "Cannot read properties of undefined (reading 'x')", + ); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(bug); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '@wakeup' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + const loopErrorTexts = () => + (mockClient.sessionUpdate as ReturnType).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text: string) => text.includes('[loop error]')); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + // The unexpected error surfaced (the loop did NOT silently degrade). + await vi.waitFor(() => + expect(loopErrorTexts().length).toBeGreaterThan(0), + ); + for (const text of loopErrorTexts()) { + // Sanitized: carries the 'unknown' errno, not the raw TypeError text. + expect(text).toContain('loop.md resolution failed (unknown)'); + expect(text).not.toContain('Cannot read properties'); + } + // No degraded tick was ever sent to the model. + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => + Array.isArray(c[1]?.message) ? c[1].message : [], + ) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + expect(sentToModel()).not.toContain('# /loop tick'); + // The real (unsanitized) bug is still recorded in the LOCAL debug warn. + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + 'loop.md sentinel resolution failed (mode=dynamic, code=unknown) — check .qwen/loop.md permissions/IO', + bug, + ); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('still throws on a transient EACCES resolve error for a cron loop', async () => { + // The cron counterpart: cron re-fires on its own next interval, so even a + // known-transient EACCES STILL propagates (sanitized) rather than degrading. + debugLoggerWarnSpy.mockClear(); + const eacces = Object.assign(new Error('EACCES: permission denied'), { + code: 'EACCES', + }); + const resolveSpy = vi + .spyOn(core.LoopTickResolver.prototype, 'resolve') + .mockRejectedValue(eacces); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + const cronErrorTexts = () => + (mockClient.sessionUpdate as ReturnType).mock.calls + .map((call) => call[0]?.update) + .filter((u) => u?.sessionUpdate === 'agent_message_chunk') + .map((u) => u?.content?.text ?? '') + .filter((text: string) => text.includes('[cron error]')); + await vi.waitFor(() => + expect(cronErrorTexts().length).toBeGreaterThan(0), + ); + for (const text of cronErrorTexts()) { + expect(text).toContain('EACCES'); + } + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => + Array.isArray(c[1]?.message) ? c[1].message : [], + ) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + expect(sentToModel()).not.toContain('# /loop tick'); + } finally { + resolveSpy.mockRestore(); + } + }); + + it('echoes the absent label when a sentinel fires with no loop.md present', async () => { + // The `loopTick && !loopTick.sourceLabel` branch: a sentinel fires but no + // project or home loop.md exists, so the tick is a labelled no-op. + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-absent-'), + ); + const fakeHome = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-home-'), + ); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + const restoreHome = setFakeHome(fakeHome); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(createEmptyStream()) + .mockResolvedValueOnce(createEmptyStream()); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => { + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'user_message_chunk', + content: { + type: 'text', + text: 'Loop tick — loop.md not present', + }, + _meta: { source: 'cron' }, + }, + }); + }); + } finally { + restoreHome(); + await fs.rm(tmpDir, { recursive: true, force: true }); + await fs.rm(fakeHome, { recursive: true, force: true }); + } + }); + + it('leaves a non-sentinel cron prompt untouched (no loop.md expansion)', async () => { + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + callback({ + prompt: 'do the normal cron thing', + cronExpr: '0 * * * *', + }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(createEmptyStream()) + .mockResolvedValueOnce(createEmptyStream()); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => { + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'user_message_chunk', + content: { type: 'text', text: 'do the normal cron thing' }, + _meta: { source: 'cron' }, + }, + }); + }); + + const sentToModel = () => + (mockChat.sendMessageStream as ReturnType).mock.calls + .flatMap((c) => (Array.isArray(c[1]?.message) ? c[1].message : [])) + .map((p: { text?: string }) => p.text ?? '') + .join(''); + await vi.waitFor(() => { + expect(sentToModel()).toContain('do the normal cron thing'); + }); + expect(sentToModel()).not.toContain('# /loop tick'); + }); + + it('re-expands the full loop.md block after an auto-compaction resets the resolver cache', async () => { + // LoopTickResolver.resetCache() is unit-tested in isolation; this pins + // the Session-level wiring: an auto-compaction in the send path + // (#sendMessageStreamWithAutoCompression) must reset the resolver so the + // next unchanged tick re-delivers the FULL block (a short reminder would + // point back to a task block compaction just evicted from context). + // + // Three unchanged ticks: tick1 full (committed), tick2 would normally be + // a short reminder but COMPACTS mid-send, tick3 re-expands FULL purely + // because tick2's compaction reset the cache. The INTRO line therefore + // appears in exactly the two full deliveries (tick1 + tick3); without + // the reset it would appear only once. + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'loop-md-compact-'), + ); + const loopMdPath = path.join(tmpDir, '.qwen', 'loop.md'); + await fs.mkdir(path.dirname(loopMdPath), { recursive: true }); + await fs.writeFile(loopMdPath, '- stable task list'); + mockConfig.getWorkingDir = vi.fn().mockReturnValue(tmpDir); + + // Compress on the SECOND cron tick only — keyed on the cron promptId so + // the user 'hello' prompt's compression check stays a no-op. + let cronCompressions = 0; + mockGeminiClient.tryCompressChat = vi + .fn() + .mockImplementation(async (promptId: string) => { + const isCron = String(promptId).includes('cron'); + if (isCron) cronCompressions++; + const compressed = isCron && cronCompressions === 2; + return { + originalTokenCount: 100, + newTokenCount: 50, + compressionStatus: compressed + ? core.CompressionStatus.COMPRESSED + : core.CompressionStatus.NOOP, + }; + }); + + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn( + ( + callback: (job: { prompt: string; cronExpr?: string }) => void, + ) => { + // Three ticks of the same sentinel; the cron queue drains them + // serially against the one persistent resolver. + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + callback({ prompt: '<>', cronExpr: '*/5 * * * *' }); + }, + ), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockChat.sendMessageStream = vi + .fn() + .mockImplementation(() => Promise.resolve(createEmptyStream())); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + const fullDeliveries = () => + ( + mockChat.sendMessageStream as ReturnType + ).mock.calls.filter((c) => + (Array.isArray(c[1]?.message) ? c[1].message : []) + .map((p: { text?: string }) => p.text ?? '') + .join('') + .includes('The user configured a loop-tasks file.'), + ).length; + + // tick1 + tick3 re-expand; tick2 is the (compacting) short reminder. + await vi.waitFor(() => { + expect(fullDeliveries()).toBe(2); + }); + // The compaction actually fired on a cron tick (sanity-check the setup). + expect(cronCompressions).toBeGreaterThanOrEqual(2); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); + it('stops cron-fired ACP prompt before sending when the session token limit is exceeded', async () => { let cronCallback: ((job: { prompt: string }) => void) | undefined; const scheduler = { diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 74bfc0a3666..dd4d4917793 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -4,6 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as os from 'node:os'; +import * as path from 'node:path'; import type { Content, FunctionCall, @@ -28,11 +30,14 @@ import type { GoalTerminalEvent, ToolCallRequestInfo, ToolCallResponseInfo, + LoopTickResult, } from '@qwen-code/qwen-code-core'; import { AuthType, ApprovalMode, CompressionStatus, + detectLoopSentinel, + LoopTickResolver, convertToFunctionResponse, createDuplicateProviderToolCallResponse, findRepeatedDuplicateProviderToolCall, @@ -212,6 +217,23 @@ const MAX_MID_TURN_RESOURCE_TEXT_LENGTH = 100_000; // conforming-but-busy client, while a client that never answers stops // costing a stall per tool batch after a few batches. const MID_TURN_QUEUE_DRAIN_MAX_TIMEOUT_STRIKES = 3; +// fs codes that let a `dynamic` (self-paced) loop treat a THROWN loop.md +// sentinel-resolution as transient — degrade to a no-op re-arm tick so the loop +// survives — instead of re-throwing (which ends it: the firing wakeup is already +// consumed, so only an end-of-turn re-arm keeps it alive). readLoopTaskFile only +// re-throws EACCES/EIO/EBUSY/EPERM (it skips ENOENT/EISDIR/ENOTDIR/ELOOP/… to its +// own `missing` → no-op path); EISDIR/ENOTDIR stay here as defense-in-depth for +// the lstat→open TOCTOU race (path swapped to a dir/non-dir mid-read) should that +// internal skip ever narrow. ENOENT is omitted on purpose: "absent" is not a +// transient read failure and can never reach this catch. +const TRANSIENT_FS_CODES: readonly string[] = [ + 'EACCES', + 'EIO', + 'EBUSY', + 'EPERM', + 'EISDIR', + 'ENOTDIR', +]; type DrainedMidTurnMessage = | { kind: 'text'; message: string } @@ -667,6 +689,13 @@ export class Session implements SessionContext { private cronQueue: CronQueueItem[] = []; private cronProcessing = false; private cronAbortController: AbortController | null = null; + // Resolves the `<>` / `<>` sentinels at fire time. + // Lazily created on the first loop tick; its content cache is reset on + // compaction (see #sendMessageStreamWithAutoCompression) and it is rebuilt if + // the working dir changes (e.g. /cd) so it always reads the current project's + // loop.md. + private loopTickResolver: LoopTickResolver | null = null; + private loopTickResolverRoot: string | null = null; private cronCompletion: Promise | null = null; private cronDisabledByTokenLimit = false; private lastPromptTokenCount = 0; @@ -1927,6 +1956,10 @@ export class Session implements SessionContext { compressionInfo = compressed; this.#recordCompressionTokenCount(compressed); if (compressed.compressionStatus === CompressionStatus.COMPRESSED) { + // Context was just compacted; a loop.md tick must re-deliver the full + // task block (a short reminder refers back to a message that is no + // longer in context). + this.loopTickResolver?.resetCache(); const reasonClause = compressed.triggerReason === 'image_overflow' ? `accumulated enough tool screenshots to trigger compaction for ${this.config.getModel()}` @@ -2445,6 +2478,44 @@ export class Session implements SessionContext { } } + #getLoopTickResolver(): LoopTickResolver { + const root = this.config.getWorkingDir(); + // Rebuild if the working dir changed (e.g. /cd) so loop.md resolves against + // the current project; a fresh resolver also correctly re-delivers full. + if (!this.loopTickResolver || this.loopTickResolverRoot !== root) { + // Resolve the home/global loop.md from the QWEN_HOME-aware global dir (the + // rest of Qwen honors QWEN_HOME for `.qwen`); reading raw os.homedir() here + // would always hit the real `~/.qwen` and ignore a relocated config home. + const homeQwenDir = Storage.getGlobalQwenDir(); + // Confinement root for the home candidate's resolved target: $QWEN_HOME + // when set (it IS the global dir), else $HOME — keeps the earlier + // confinement (an in-root dotfile symlink resolves; an escape is refused). + // The `|| path.dirname(homeQwenDir)` guards an empty os.homedir() (minimal + // containers with no HOME): an empty root makes isWithin('', target) always + // true, trivially bypassing the symlink confinement. homeQwenDir + // (Storage.getGlobalQwenDir()) is always non-empty, so its parent is a + // sound non-empty fallback root. + const homeConfineRoot = + (process.env['QWEN_HOME'] ? homeQwenDir : os.homedir()) || + path.dirname(homeQwenDir); + this.loopTickResolver = new LoopTickResolver({ + projectRoot: root, + homeDir: homeConfineRoot, + homeQwenDir, + // The project `.qwen/loop.md` is repo-controlled, so an untrusted folder + // must not read it and feed it to the model (mirrors getProjectHooks()'s + // trust gate). The home/global `~/.qwen/loop.md` is user-owned and stays + // allowed. Pass a getter, not a snapshot: isTrustedFolder() can flip + // mid-session on an IDE workspace-trust update, and the resolver outlives + // a single tick — re-read it on every resolve() so a trusted→untrusted + // flip stops reading the project file immediately. + allowProjectFile: () => this.config.isTrustedFolder(), + }); + this.loopTickResolverRoot = root; + } + return this.loopTickResolver; + } + /** * Executes a single cron-fired prompt: echoes it as a user message with * `_meta.source='cron'`, streams the model response, and handles tool calls. @@ -2478,10 +2549,114 @@ export class Session implements SessionContext { async () => { let turnCount = 0; try { + // A `<>` / `<>` sentinel is expanded at + // fire time into the loop.md task block — full on the first or a + // changed fire, a short reminder when unchanged. Non-sentinel + // prompts pass through untouched. + const loopMode = detectLoopSentinel(prompt); + let loopTick: LoopTickResult | null = null; + if (loopMode) { + const resolver = this.#getLoopTickResolver(); + // Capture folder-trust ONCE for this tick and thread it through + // both the resolve probe and the error path. isTrustedFolder() + // can flip mid-tick (an IDE workspace-trust update), so two + // separate reads could let the sanitized error name a different + // candidate set than resolve() actually probed. + const trustedAtResolve = this.config.isTrustedFolder(); + try { + loopTick = await resolver.resolve(loopMode, trustedAtResolve); + } catch (resolveErr) { + // resolve() reads .qwen/loop.md (project or home/global); an + // EACCES/EIO here is a sentinel-RESOLUTION failure, not a + // model-call failure — tag it so the two are distinguishable + // in logs. + const code = + (resolveErr as NodeJS.ErrnoException).code ?? 'unknown'; + // Full detail — including the raw fs error's ABSOLUTE loop.md + // path (OS username + dir layout) — stays in this LOCAL debug + // log only; debug logs are never sent to the ACP client. + debugLogger.warn( + `loop.md sentinel resolution failed (mode=${loopMode}, code=${code}) — check .qwen/loop.md permissions/IO`, + resolveErr, + ); + if ( + loopMode === 'dynamic' && + TRANSIENT_FS_CODES.includes(code) + ) { + // A `dynamic` (self-paced) loop is kept alive ONLY by the + // model re-arming LoopWakeup at the end of each turn; the + // firing wakeup was already consumed, so throwing here (no + // turn → no re-arm) would silently kill the loop forever on a + // transient hiccup (EACCES/EIO, or a Windows editor/AV briefly + // locking the file). Degrade to a no-op tick mirroring the + // absent path so the model still re-arms and the loop survives. + // (`cron` re-fires on its own next interval, so it still + // throws below.) The captured trust names the SAME candidate + // set the probe used; the errno (no absolute path) is noted. + // Only KNOWN-transient codes degrade: an unexpected error + // (TypeError / assertion → code 'unknown') falls through to the + // throw so the real bug surfaces instead of an infinite no-op + // cycle. + loopTick = resolver.buildTransientErrorTick( + loopMode, + trustedAtResolve, + code, + ); + } else { + // Reached by `cron` (re-fires on its own next interval) and by + // `dynamic` with an UNEXPECTED (non-transient) error — both + // surface rather than silently degrade. Re-throw a SANITIZED + // error: the outer catch forwards error.message verbatim to the + // client via emitAgentMessage, + // so re-throwing the raw fs error would leak that absolute + // path. Surface only the candidate labels + errno code via the + // shared absentLocations() — reusing the QWEN_HOME-aware home + // label (never a hardcoded `~/.qwen`) and naming the project + // candidate only when it was actually read (the captured trust + // matches the resolve() probe, so an untrusted folder can't + // falsely claim `(project)`). + throw new Error( + `loop.md resolution failed (${code}) for ${resolver.absentLocations( + trustedAtResolve, + )}`, + ); + } + } + } + const modelText = loopTick ? loopTick.modelText : prompt; + if (loopTick) { + debugLogger.debug( + `loop tick: mode=${loopMode} delivery=${ + loopTick.full + ? 'full' + : loopTick.sourceLabel + ? 'reminder' + : 'absent' + } source=${loopTick.sourceLabel ?? 'none'} transient=${ + loopTick.transientError ?? false + }`, + ); + } + // For a loop tick echo a stable, relative label — never the bare + // sentinel or the full task dump (and the resolver never hands back + // the absolute path, which would leak the OS username / dir layout + // into the ACP client UI); otherwise echo the prompt verbatim. + const echoText = loopTick + ? loopTick.sourceLabel + ? `Loop tick — tasks from ${loopTick.sourceLabel}` + : // A transient-error tick (buildTransientErrorTick) resolved a + // file but couldn't read it this tick; it deliberately omits + // sourceLabel, so don't conflate it with a genuinely-absent + // loop.md. No errno/path here — those stay in the model text. + loopTick.transientError + ? 'Loop tick — loop.md temporarily unavailable' + : 'Loop tick — loop.md not present' + : prompt; + // Echo the cron prompt as a user message so the client sees it await this.sendUpdate({ sessionUpdate: 'user_message_chunk', - content: { type: 'text', text: prompt }, + content: { type: 'text', text: echoText }, _meta: { source: item.source }, }); @@ -2490,7 +2665,7 @@ export class Session implements SessionContext { const cronReminders = await this.#buildInitialSystemReminders(); let nextMessage: Content | null = { role: 'user', - parts: [...cronReminders, { text: prompt }], + parts: [...cronReminders, { text: modelText }], }; while (nextMessage !== null) { @@ -2519,6 +2694,13 @@ export class Session implements SessionContext { return; } const responseStream = sendResult.responseStream; + if (loopTick && turnCount === 1) { + // The block reached the model (the send started); commit it so + // the next tick can detect "unchanged". Deferring the commit + // to here keeps an abort before delivery from poisoning the + // cache into a dangling short reminder. + this.loopTickResolver?.markDelivered(); + } nextMessage = null; for await (const resp of responseStream) { diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index 0f9dab4330f..a709991d1af 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -6,6 +6,7 @@ import type { Config, + CronJob, ToolRegistry, ServerGeminiStreamEvent, SessionMetrics, @@ -22,9 +23,15 @@ import { ApprovalMode, SendMessageType, LoopType, + CronScheduler, + LOOP_SENTINEL_CRON, + LOOP_SENTINEL_DYNAMIC, } from '@qwen-code/qwen-code-core'; import type { Part } from '@google/genai'; -import { runNonInteractive } from './nonInteractiveCli.js'; +import { + runNonInteractive, + skipHeadlessLoopSentinel, +} from './nonInteractiveCli.js'; import { vi, type Mock, type MockInstance } from 'vitest'; import * as fs from 'node:fs/promises'; import * as os from 'node:os'; @@ -73,6 +80,73 @@ vi.mock('./services/CommandService.js', () => ({ }, })); +describe('skipHeadlessLoopSentinel', () => { + it('deletes a recurring session loop.md sentinel job so sessionSize reaches 0', () => { + // A recurring SESSION (non-durable) loop.md job left in the scheduler keeps + // sessionSize > 0, so the headless hold-open never resolves and the run + // hangs. Skipping the sentinel must delete the job, not just no-op the tick. + const scheduler = new CronScheduler(); + const job = scheduler.create('*/5 * * * *', LOOP_SENTINEL_CRON, true); + expect(scheduler.sessionSize).toBe(1); + + expect(skipHeadlessLoopSentinel(scheduler, job)).toBe(true); + + expect(scheduler.sessionSize).toBe(0); + expect(scheduler.list()).toHaveLength(0); + }); + + it('also cleans up a recurring session job for the dynamic sentinel', () => { + // Mirror of the cron case for `<>`. skipHeadlessLoopSentinel + // must route through detectLoopSentinel (which matches BOTH sentinels), not a + // `=== LOOP_SENTINEL_CRON` comparison — otherwise a dynamic loop.md job would + // pin sessionSize > 0 and hang the headless run. + const scheduler = new CronScheduler(); + const job = scheduler.create('*/5 * * * *', LOOP_SENTINEL_DYNAMIC, true); + expect(scheduler.sessionSize).toBe(1); + + expect(skipHeadlessLoopSentinel(scheduler, job)).toBe(true); + + expect(scheduler.sessionSize).toBe(0); + expect(scheduler.list()).toHaveLength(0); + }); + + it('returns false and keeps a non-sentinel job', () => { + const scheduler = new CronScheduler(); + scheduler.create('*/5 * * * *', 'do real work', true); + const job = scheduler.list()[0] as CronJob; + + expect(skipHeadlessLoopSentinel(scheduler, job)).toBe(false); + + expect(scheduler.sessionSize).toBe(1); + }); + + it('does not delete a durable sentinel job (it persists for a future session)', () => { + // Durable jobs live under ~/.qwen and never count toward sessionSize, so + // they don't pin the run; deleting one would wrongly remove it from disk. + const scheduler = new CronScheduler(); + const job = scheduler.create('*/5 * * * *', LOOP_SENTINEL_CRON, true); + job.durable = true; + const deleteSpy = vi.spyOn(scheduler, 'delete'); + + expect(skipHeadlessLoopSentinel(scheduler, job)).toBe(true); + + expect(deleteSpy).not.toHaveBeenCalled(); + }); + + it('does not delete a non-recurring sentinel job (one-shot stays in the scheduler)', () => { + // The deletion branch requires BOTH `recurring && !durable`. A one-shot + // sentinel job is already removed by the scheduler before it fires, so this + // guard must NOT delete it — a `!durable`-only guard would wrongly evict it. + const scheduler = new CronScheduler(); + const job = scheduler.create('*/5 * * * *', LOOP_SENTINEL_CRON, false); + const deleteSpy = vi.spyOn(scheduler, 'delete'); + + expect(skipHeadlessLoopSentinel(scheduler, job)).toBe(true); + + expect(deleteSpy).not.toHaveBeenCalled(); + }); +}); + describe('runNonInteractive', () => { let mockConfig: Config; let mockSettings: LoadedSettings; @@ -3137,6 +3211,42 @@ describe('runNonInteractive', () => { ); }); + it('installs a skipDurableFire predicate that classifies loop.md sentinels in headless mode', async () => { + // Locks the wiring at the scheduler-enable site: runNonInteractive must + // hand the scheduler a predicate that skips durable loop.md sentinels + // (which a headless run can't expand), while still letting non-sentinel + // durable jobs fire. Both halves are covered alone — detectLoopSentinel via + // skipHeadlessLoopSentinel above, the filter via cronScheduler tests — but + // nothing pins that runNonInteractive actually connects them. A refactor + // dropping or rewriting this call would otherwise silently fire raw + // `<>` sentinels at the model (or skip real durable jobs), uncaught. + setupMetricsMock(); + // Real scheduler with no projectRoot: enableDurable() short-circuits (no + // filesystem/lock work) and, with no jobs, the headless cron hold-open + // resolves immediately, so runNonInteractive returns without hanging. + const scheduler = new CronScheduler(); + const skipSpy = vi.spyOn(scheduler, 'setSkipDurableFire'); + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockGeminiClient.sendMessageStream.mockReturnValue( + createStreamFromEvents([ + { type: GeminiEventType.Content, value: 'ok' }, + { + type: GeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 1 } }, + }, + ]), + ); + + await runNonInteractive(mockConfig, mockSettings, 'test', 'p-cron-wiring'); + + expect(skipSpy).toHaveBeenCalledOnce(); + const predicate = skipSpy.mock.calls[0][0]; + expect(predicate({ prompt: LOOP_SENTINEL_CRON } as CronJob)).toBe(true); + expect(predicate({ prompt: LOOP_SENTINEL_DYNAMIC } as CronJob)).toBe(true); + expect(predicate({ prompt: 'regular cron job' } as CronJob)).toBe(false); + }); + describe('--json-schema structured output', () => { // Helper: walk an emitted event and extract the first tool_use_id when // it represents a tool_result block. Returns undefined for any other diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 3108cc87875..b85ffc53f43 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -7,6 +7,8 @@ import type { BackgroundTaskStatus, Config, + CronJob, + CronScheduler, ToolCallRequestInfo, } from '@qwen-code/qwen-code-core'; import { isSlashCommand } from './ui/utils/commandUtils.js'; @@ -25,6 +27,7 @@ import { uiTelemetryService, parseAndFormatApiError, createDebugLogger, + detectLoopSentinel, SendMessageType, restoreWorktreeContext, TeamEventType, @@ -136,6 +139,52 @@ function formatLoopDetectedMessage(loopType: LoopType | undefined): string { return `Loop detection halted the run${detail}.${hint}`; } +/** + * Headless handling for a fired `.qwen/loop.md` cron sentinel. loop.md + * expansion is interactive-only for now, so a bare sentinel can't be turned + * into a real prompt here — the tick is skipped (no-op) rather than sent to the + * model as empty content. Returns true when `job` was a sentinel so the caller + * skips enqueuing it. + * + * A recurring SESSION (non-durable) loop.md job would otherwise stay in + * `scheduler.sessionSize` and re-fire every interval, pinning the headless run + * open forever (the hold-open resolves only when sessionSize hits zero); delete + * it so the run can terminate. Durable jobs are left untouched here — they + * persist for a future owning session and never count toward sessionSize — and + * a one-shot job is already removed before it fires. + * + * Note: a DURABLE loop.md sentinel never even reaches this callback in headless, + * because `setSkipDurableFire` filters it at the scheduler before any fire or + * lastFiredAt persist (otherwise the tick would be marked fired while the work + * is skipped — silent loss). This guard's durable branch is kept defensive. + */ +export function skipHeadlessLoopSentinel( + scheduler: CronScheduler, + job: CronJob, +): boolean { + if (!detectLoopSentinel(job.prompt)) { + return false; + } + if (job.recurring && !job.durable) { + // A user created this recurring loop.md cron via /loop in interactive mode; + // deleting it here is otherwise silent, so leave a trace of why it vanished + // from `cron list` when the same workspace is later run headless. + debugLogger.debug( + 'skipHeadlessLoopSentinel: cleaning up recurring session loop.md cron in headless mode', + { jobId: job.id }, + ); + // delete() removes the in-memory job synchronously before any await, so the + // sessionSize check that follows this call sees it gone; the returned promise + // has no on-disk work for a session job. Fire-and-forget, but swallow a + // rejection so a future async delete() can't surface as an unhandled + // rejection (fatal under Node's --unhandled-rejections=throw). + void scheduler.delete(job.id).catch(() => { + /* session job: nothing to clean up on a delete failure */ + }); + } + return true; +} + function emitLoopDetectedMessage( config: Config, loopType: LoopType | undefined, @@ -1537,6 +1586,15 @@ export async function runNonInteractive( : config.getCronScheduler(); if (scheduler) { + // A headless run can't expand a `<>` sentinel, so durable + // loop.md jobs must be skipped at the scheduler level — firing one + // here would stamp+persist its lastFiredAt while the work is skipped + // (see skipHeadlessLoopSentinel), silently consuming a tick the + // owning interactive session should run. Set BEFORE enableDurable so + // a buffered catch-up flush at start() honors it too. + scheduler.setSkipDurableFire( + (job) => detectLoopSentinel(job.prompt) !== null, + ); // Durable tasks live under ~/.qwen (user-owned, not in the // working tree), so no folder-trust gate is needed here. await scheduler @@ -1596,7 +1654,16 @@ export async function runNonInteractive( reject(err); }; - scheduler.start((job: { prompt: string; cronExpr?: string }) => { + scheduler.start((job: CronJob) => { + // A bare loop.md sentinel can't expand in a headless run, so the + // tick is skipped. skipHeadlessLoopSentinel also deletes a + // recurring session job so it stops re-firing and sessionSize + // can fall to zero — otherwise checkCronDone never resolves and + // the run hangs. Full headless loop.md support is a follow-up. + if (skipHeadlessLoopSentinel(scheduler, job)) { + checkCronDone(); + return; + } const label = job.prompt.slice(0, 40); localQueue.push({ displayText: `${job.cronExpr === '@wakeup' ? 'Loop' : 'Cron'}: ${label}`, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0b5bcad30a9..e3ea173d513 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -372,6 +372,8 @@ export { export * from './extension/index.js'; export * from './prompts/mcp-prompts.js'; export * from './skills/index.js'; +export * from './skills/bundled/loop/loop-task-file.js'; +export * from './skills/bundled/loop/loop-tick-resolver.js'; export * from './subagents/index.js'; export * from './agents/index.js'; diff --git a/packages/core/src/services/cronScheduler.test.ts b/packages/core/src/services/cronScheduler.test.ts index ca7a0b44937..7987b1db3c9 100644 --- a/packages/core/src/services/cronScheduler.test.ts +++ b/packages/core/src/services/cronScheduler.test.ts @@ -932,6 +932,229 @@ describe('CronScheduler', () => { }); }); + it('skips a durable job the consumer cannot run: no fire, lastFiredAt left untouched', async () => { + // A headless run can't expand a `<>` sentinel. Firing it would + // stamp + persist lastFiredAt while the work is skipped downstream, + // silently consuming the tick; setSkipDurableFire must leave such a job's + // schedule intact for the owning interactive session. A co-scheduled + // non-sentinel durable job proves the skip is selective AND lands its + // persist in the SAME tick write — so checking the sentinel stayed null + // once the sibling shows its stamp is race-free, not a timing gap. + await writeCronTasks(tmpDir, [ + { ...diskTask('loopmd'), prompt: '<>' }, + { ...diskTask('normal'), prompt: 'normal task' }, + ]); + await scheduler.enableDurable('session-1'); + scheduler.setSkipDurableFire((job) => job.prompt === '<>'); + + const fired: CronJob[] = []; + scheduler.start((job) => fired.push(job)); + scheduler.tick(new Date(2025, 0, 15, 10, 30, 59)); + + expect(fired.map((j) => j.prompt)).toEqual(['normal task']); + + const minuteMs = new Date(2025, 0, 15, 10, 30, 0).getTime(); + await vi.waitFor(async () => { + const byId = Object.fromEntries( + (await readCronTasks(tmpDir)).map((t) => [t.id, t]), + ); + expect(byId['normal']!.lastFiredAt).toBe(minuteMs); // fired → persisted + expect(byId['loopmd']!.lastFiredAt ?? null).toBeNull(); // skipped → untouched + }); + }); + + it('deliverPending missed branch: skips a sentinel one-shot (no fire, left on disk), fires a sibling', async () => { + // CRITICAL regression lock. A missed durable <> sentinel a + // headless consumer can't run must NOT be fired NOR removed from disk — + // deleting it would lose the task forever though no consumer ran the + // loop.md work. The skip is selective: a co-missed non-sentinel one-shot + // in the SAME batch is still fired (batched notice) and removed. + // Mutation check: revert the missed-branch partition and this fails + // (sentinel gets batched into the notice AND deleted from disk). + // Past createdAt so each one-shot's single fire already elapsed (missed). + const past = Date.now() - 10 * 60_000; + await writeCronTasks(tmpDir, [ + { + id: 'loopmd', + cron: '* * * * *', + prompt: '<>', + recurring: false, + createdAt: past, + lastFiredAt: null, + }, + { + id: 'normal', + cron: '* * * * *', + prompt: 'normal one-shot', + recurring: false, + createdAt: past, + lastFiredAt: null, + }, + ]); + + const fired: CronJob[] = []; + scheduler.start((job) => fired.push(job)); + scheduler.setSkipDurableFire((job) => job.prompt === '<>'); + await scheduler.enableDurable('session-1'); + + // Only the runnable sibling is notified; the sentinel is partitioned out. + expect(fired).toHaveLength(1); + expect(fired[0]!.missed).toBe(true); + expect(fired[0]!.prompt).toContain('normal one-shot'); + expect(fired[0]!.prompt).not.toContain('<>'); + + // The skipped sentinel must NOT linger in pendingRemoval: it stays on disk + // (not removed), so a stuck guard would keep it out of both the job map and + // disk reconciliation forever. Delivery is synchronous within enableDurable, + // so this is race-free. Mutation check: drop the pendingRemoval.delete and + // this fails (the sentinel is stranded in pendingRemoval). + const pendingRemoval = ( + scheduler as unknown as { pendingRemoval: Set } + ).pendingRemoval; + expect(pendingRemoval.has('loopmd')).toBe(false); + + // The sentinel survives on disk; only the fired sibling is removed. + await vi.waitFor(async () => { + expect((await readCronTasks(tmpDir)).map((t) => t.id)).toEqual([ + 'loopmd', + ]); + }); + }); + + it('deliverPending missed branch: an ALL-sentinel batch fires nothing and leaves every task on disk', async () => { + // All-filtered companion to the mixed-batch lock above. When a headless + // load misses ONLY <> sentinels it can't run, the + // runnable.length > 0 guard must fire NOTHING (no empty carrier notice) + // AND never call removeMissedFromDisk, so every sentinel is preserved for + // its owning interactive session. Mutation check: drop the guard and the + // empty batch fires a bogus missed notification (durableTaskToJob over an + // undefined runnable[0]). + const past = Date.now() - 10 * 60_000; + await writeCronTasks(tmpDir, [ + { + id: 'loopmd-a', + cron: '* * * * *', + prompt: '<>', + recurring: false, + createdAt: past, + lastFiredAt: null, + }, + { + id: 'loopmd-b', + cron: '* * * * *', + prompt: '<>', + recurring: false, + createdAt: past, + lastFiredAt: null, + }, + ]); + + const fired: CronJob[] = []; + scheduler.start((job) => fired.push(job)); + scheduler.setSkipDurableFire((job) => job.prompt === '<>'); + await scheduler.enableDurable('session-1'); + + // Nothing in the batch is runnable → no fire at all (delivery is + // synchronous within enableDurable, so this is race-free). + expect(fired).toEqual([]); + + // Both sentinels survive — removeMissedFromDisk was never reached. + expect((await readCronTasks(tmpDir)).map((t) => t.id).sort()).toEqual([ + 'loopmd-a', + 'loopmd-b', + ]); + }); + + it('deliverPending catch-up branch: skips a sentinel overdue-recurring (stamp left on disk), fires a sibling', async () => { + // 3h overdue, past any jitter window. The sentinel must not be fired and + // must keep its on-disk lastFiredAt (left out of persistCatchUpStamps) so + // the owning session re-detects the catch-up; the sibling fires raw and + // its advanced stamp persists. + const createdAt = Date.now() - 3 * 60 * 60_000; + await writeCronTasks(tmpDir, [ + { + id: 'loopmd-c', + cron: '0 * * * *', + prompt: '<>', + recurring: true, + createdAt, + lastFiredAt: createdAt, + }, + { + id: 'normal-c', + cron: '0 * * * *', + prompt: 'overdue recurring', + recurring: true, + createdAt, + lastFiredAt: createdAt, + }, + ]); + + const fired: CronJob[] = []; + scheduler.start((job) => fired.push(job)); + scheduler.setSkipDurableFire((job) => job.prompt === '<>'); + await scheduler.enableDurable('session-1'); + + expect(fired.map((j) => j.prompt)).toEqual(['overdue recurring']); + + // Sibling's catch-up stamp lands; once it does, the sentinel's untouched + // disk stamp is race-free, not a timing gap. Both stay on disk. + await vi.waitFor(async () => { + const byId = Object.fromEntries( + (await readCronTasks(tmpDir)).map((t) => [t.id, t]), + ); + expect(byId['normal-c']!.lastFiredAt).toBeGreaterThan(createdAt); + expect(byId['loopmd-c']!.lastFiredAt).toBe(createdAt); + }); + }); + + it('deliverPending final branch: skips a sentinel aged-recurring (no final fire, left on disk), fires a sibling', async () => { + // Aged past the 7-day max age → final raw fire + delete. The sentinel is + // left on disk (not in removeMissedFromDisk) for the owning session; the + // sibling gets its one final fire and is deleted. + const createdAt = Date.now() - 8 * 24 * 60 * 60_000; + const lastFiredAt = Date.now() - 2 * 60 * 60_000; + await writeCronTasks(tmpDir, [ + { + id: 'loopmd-f', + cron: '0 * * * *', + prompt: '<>', + recurring: true, + createdAt, + lastFiredAt, + }, + { + id: 'normal-f', + cron: '0 * * * *', + prompt: 'aged recurring', + recurring: true, + createdAt, + lastFiredAt, + }, + ]); + + const fired: CronJob[] = []; + scheduler.start((job) => fired.push(job)); + scheduler.setSkipDurableFire((job) => job.prompt === '<>'); + await scheduler.enableDurable('session-1'); + + expect(fired.map((j) => j.prompt)).toEqual(['aged recurring']); + + // Same limbo guard as the missed branch: a skipped final task stays on + // disk, so it must not be stranded in pendingRemoval. + const pendingRemoval = ( + scheduler as unknown as { pendingRemoval: Set } + ).pendingRemoval; + expect(pendingRemoval.has('loopmd-f')).toBe(false); + + // The fired sibling is deleted; the skipped sentinel stays on disk. + await vi.waitFor(async () => { + expect((await readCronTasks(tmpDir)).map((t) => t.id)).toEqual([ + 'loopmd-f', + ]); + }); + }); + it('rolls back the in-memory job when the durable persist fails', async () => { // A corrupted tasks file makes updateCronTasks throw inside // addCronTask, after the job was provisionally installed in memory. diff --git a/packages/core/src/services/cronScheduler.ts b/packages/core/src/services/cronScheduler.ts index 48286fd058f..411886e1c9c 100644 --- a/packages/core/src/services/cronScheduler.ts +++ b/packages/core/src/services/cronScheduler.ts @@ -200,6 +200,14 @@ export class CronScheduler { private _disabled = false; private timer: ReturnType | null = null; private onFire: ((job: CronJob) => void) | null = null; + // Guard a consumer installs when it cannot execute certain durable jobs. A + // headless run can't expand a `.qwen/loop.md` sentinel, so it marks such + // durable jobs skippable here: they are then neither fired NOR have their + // persisted fired-state advanced (lastFiredAt stamp / one-shot removal), + // leaving the tick for the owning interactive session instead of silently + // consuming it for work the consumer never ran. Session-only jobs and durable + // jobs in a consumer that can run them are unaffected (predicate unset/false). + private skipDurableFire: ((job: CronJob) => boolean) | null = null; // --- Durable (file-backed) support --- private durableEnabled = false; @@ -742,12 +750,40 @@ export class CronScheduler { // load (claw-code parity) — one model turn and one confirmation // flow instead of N separate prompts. The carrier job exists to // satisfy the onFire shape; consumers only read prompt/missed. - onFire({ - ...durableTaskToJob(pending.tasks[0]!), - prompt: buildMissedCronNotification(pending.tasks), - missed: true, + // Same skip as catch-up/final: partition out durable one-shots + // this consumer can't run (e.g. a loop.md sentinel in a headless + // run). They are not notified and, critically, left on disk (not + // in removeMissedFromDisk) so the owning interactive session still + // surfaces and runs them instead of losing the task permanently. + const skipped: string[] = []; + const runnable = pending.tasks.filter((t) => { + const job = durableTaskToJob(t); + // `job.durable &&` mirrors catch-up/final/tick — durableTaskToJob always + // sets durable, so it's a no-op today, but keeps the four skip sites + // identical so a future non-durable carrier can't be silently dropped. + if (job.durable && this.skipDurableFire?.(job)) { + debugLogger.debug( + `Skipping durable job ${t.id} (missed): consumer cannot run it`, + ); + skipped.push(t.id); + return false; + } + return true; }); - this.removeMissedFromDisk(pending.tasks.map((t) => t.id)); + // A skipped sentinel stays on disk (not in removeMissedFromDisk) for its + // interactive owner — so drop its pendingRemoval guard too. Left set, it + // would sit out of BOTH the job map and disk reconciliation forever; + // cleared, the next loadFileTasks re-installs it (the intended + // "defer to the owning session" path). + for (const id of skipped) this.pendingRemoval.delete(id); + if (runnable.length > 0) { + onFire({ + ...durableTaskToJob(runnable[0]!), + prompt: buildMissedCronNotification(runnable), + missed: true, + }); + this.removeMissedFromDisk(runnable.map((t) => t.id)); + } break; } case 'catch-up': { @@ -755,6 +791,16 @@ export class CronScheduler { for (const id of pending.ids) { const job = this.jobs.get(id); if (!job) continue; // deleted while buffered + // Same skip as the tick loop (job.durable && …): a durable job this + // consumer can't run is not fired and not stamped (left out of + // persistCatchUpStamps), so its overdue schedule survives for the + // owning session. + if (job.durable && this.skipDurableFire?.(job)) { + debugLogger.debug( + `Skipping durable job ${job.id} (catch-up): consumer cannot run it`, + ); + continue; + } onFire(job); fired.push(id); } @@ -762,10 +808,24 @@ export class CronScheduler { break; } case 'final': { + const fired: string[] = []; for (const job of pending.jobs) { + // Same skip as the tick loop (job.durable && …): a skipped durable + // job is left on disk (not in removeMissedFromDisk) so the owning + // session still gets its one final fire + delete. + if (job.durable && this.skipDurableFire?.(job)) { + debugLogger.debug( + `Skipping durable job ${job.id} (final): consumer cannot run it`, + ); + // Same limbo as the missed branch: a skipped final task stays on + // disk, so clear its pendingRemoval guard rather than strand it. + this.pendingRemoval.delete(job.id); + continue; + } onFire(job); + fired.push(job.id); } - this.removeMissedFromDisk(pending.jobs.map((j) => j.id)); + this.removeMissedFromDisk(fired); break; } default: { @@ -860,6 +920,17 @@ export class CronScheduler { } } + /** + * Installs a predicate marking durable jobs the active consumer cannot run + * (see the `skipDurableFire` field). Such jobs are skipped before any fire or + * persist, so their durable schedule is left intact for an owning session that + * can run them. Set before `start()` so a buffered catch-up flush also honors + * it. A no-op for session-only jobs. + */ + setSkipDurableFire(predicate: (job: CronJob) => boolean): void { + this.skipDurableFire = predicate; + } + /** * Starts the scheduler tick. Calls `onFire` when a job is due. * Only fires when called — does not auto-fire missed intervals. @@ -1005,6 +1076,16 @@ export class CronScheduler { // in non-owner sessions, where a persisted job would otherwise fire // uncoordinated alongside the real owner's copy. if (job.durable && !this.isOwner) continue; + // A durable job this consumer can't run (e.g. a loop.md sentinel in a + // headless run) is skipped BEFORE processJob stamps lastFiredAt — firing + // it here would persist the stamp while the work is skipped downstream, + // silently consuming the tick. Leave it for the owning session. + if (job.durable && this.skipDurableFire?.(job)) { + debugLogger.debug( + `Skipping durable job ${job.id} (tick): consumer cannot run it`, + ); + continue; + } const result = this.processJob(job, currentDate, currentMs); if (!job.durable || result === 'none') continue; diff --git a/packages/core/src/skills/bundled/loop/SKILL.md b/packages/core/src/skills/bundled/loop/SKILL.md index 26a04be7fca..ac9e7de5b6e 100644 --- a/packages/core/src/skills/bundled/loop/SKILL.md +++ b/packages/core/src/skills/bundled/loop/SKILL.md @@ -92,4 +92,13 @@ If the interval does not cleanly divide its unit (for example `7m` gives uneven - If it is a slash command, invoke it via the Skill tool. - Otherwise, act on it directly. +## loop.md task-file mode + +Use this when the user wants the loop to work a task list kept in a file (they say "work through my loop.md", "loop over the tasks in .qwen/loop.md", or point at such a file). Tasks live in `.qwen/loop.md` (project) or `~/.qwen/loop.md` (home; project wins). Instead of a natural-language prompt, set the loop's `prompt` to a sentinel so each fire re-reads the file: + +- Self-paced (no interval) → LoopWakeup `prompt`: `<>` +- Fixed interval → CronCreate `prompt`: `<>` (with `recurring: true`, and `durable: true` if persistence is implied) + +At each fire you receive either the full task list (first delivery, after the file changes, or after a compaction) or a short reminder to keep working the list established earlier. Work the tasks; in self-paced mode re-arm LoopWakeup with `<>` only when continued follow-up is useful (same "don't re-arm if complete/blocked" rules as the prompt-only path). If `.qwen/loop.md` is absent at fire time, treat the tick as a no-op. Confirm to the user in plain language ("looping over your `.qwen/loop.md` task list…"), not the raw sentinel. + ## Input diff --git a/packages/core/src/skills/bundled/loop/SKILL.test.ts b/packages/core/src/skills/bundled/loop/SKILL.test.ts index feafafc0539..a4d0e6488bd 100644 --- a/packages/core/src/skills/bundled/loop/SKILL.test.ts +++ b/packages/core/src/skills/bundled/loop/SKILL.test.ts @@ -96,4 +96,13 @@ describe('bundled loop skill', () => { expect(body).toContain('**`clear`** — call CronList'); expect(body).toContain('call CronDelete for every job returned'); }); + + it('documents loop.md task-file mode and the two sentinels', () => { + const { body } = loadLoopSkill(); + + expect(body).toContain('## loop.md task-file mode'); + expect(body).toContain('.qwen/loop.md'); + expect(body).toContain('`<>`'); + expect(body).toContain('`<>`'); + }); }); diff --git a/packages/core/src/skills/bundled/loop/loop-task-file.test.ts b/packages/core/src/skills/bundled/loop/loop-task-file.test.ts new file mode 100644 index 00000000000..86b8420c96f --- /dev/null +++ b/packages/core/src/skills/bundled/loop/loop-task-file.test.ts @@ -0,0 +1,1064 @@ +/** + * @license + * Copyright 2026 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + LOOP_TASK_FILE_MAX_BYTES, + readLoopTaskFile, +} from './loop-task-file.js'; + +// Make only open controllable; every other fs call stays real so the temp-dir +// fixtures keep working. The reader is bounded via fs.open + filehandle.read, +// so open is the injection point. The default impl calls through to actual. +vi.mock('node:fs/promises', async (importActual) => { + const actual = await importActual(); + return { ...actual, open: vi.fn(actual.open) }; +}); + +// Capture the module's debug calls so a test can assert WHY a candidate was +// skipped (the whitespace-only branch is the load-bearing case). Other tests +// don't read it; production debug() no-ops without an active session anyway. +const debugSpy = vi.hoisted(() => vi.fn()); +vi.mock('../../../utils/debugLogger.js', () => ({ + createDebugLogger: () => ({ + isEnabled: () => true, + debug: debugSpy, + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})); + +describe('readLoopTaskFile', () => { + let tempDir: string; + let projectRoot: string; + let homeDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'loop-task-file-')); + projectRoot = path.join(tempDir, 'project'); + homeDir = path.join(tempDir, 'home'); + await fs.mkdir(projectRoot, { recursive: true }); + await fs.mkdir(homeDir, { recursive: true }); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + const writeProject = (content: string) => + fs + .mkdir(path.join(projectRoot, '.qwen'), { recursive: true }) + .then(() => + fs.writeFile(path.join(projectRoot, '.qwen', 'loop.md'), content), + ); + const writeHome = (content: string) => + fs + .mkdir(path.join(homeDir, '.qwen'), { recursive: true }) + .then(() => + fs.writeFile(path.join(homeDir, '.qwen', 'loop.md'), content), + ); + + // Wrap the next fs.open so each handle.read() length is recorded against a real + // handle. Lets a test prove the reader stays bounded: a "read the whole file, + // then slice" regression pulls the full file through these reads and trips the + // per-read / cumulative cap assertions. Returns the array, filled by reference. + const recordHandleReadLengths = async (): Promise => { + const lengths: number[] = []; + const actual = + await vi.importActual( + 'node:fs/promises', + ); + vi.mocked(fs.open).mockImplementationOnce(async (p) => { + const handle = await actual.open( + p as Parameters[0], + 'r', + ); + const realRead = handle.read.bind(handle); + handle.read = ((...readArgs: Parameters) => { + // Impl calls read(buffer, offset, length, position); record length. + lengths.push((readArgs as unknown[])[2] as number); + return realRead(...(readArgs as Parameters)); + }) as typeof handle.read; + return handle; + }); + return lengths; + }; + + it('reads the project loop task file first', async () => { + await writeProject('project tasks'); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(projectRoot, '.qwen', 'loop.md'), + source: 'project', + content: 'project tasks', + truncated: false, + }); + }); + + it('falls back to the user loop task file', async () => { + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('does not follow symlinked project loop task files', async () => { + await fs.mkdir(path.join(projectRoot, '.qwen'), { recursive: true }); + const outside = path.join(tempDir, 'secret.txt'); + await fs.writeFile(outside, 'secret tasks'); + await fs.symlink(outside, path.join(projectRoot, '.qwen', 'loop.md')); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('refuses a project loop.md whose .qwen ancestor symlinks outside the workspace', async () => { + // `.qwen -> ` makes a final-component lstat pass while the file + // resolves outside the project; realpath must catch the ancestor symlink. + const outside = path.join(tempDir, 'outside'); + await fs.mkdir(outside, { recursive: true }); + await fs.writeFile(path.join(outside, 'loop.md'), 'escaped tasks'); + await fs.symlink(outside, path.join(projectRoot, '.qwen')); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('refuses a project loop.md resolving to a SIBLING dir that shares a name prefix', async () => { + // isWithin appends path.sep before startsWith, so root `/foo` must NOT + // accept a candidate under the sibling `/foobar`. Make projectRoot + // `/foo` and symlink its `.qwen` to `/foobar/.qwen`; realpath then + // resolves loop.md into `foobar`, whose canonical path bare-startsWith + // `/foo` yet is NOT a descendant. A regression to a bare + // `real.startsWith(root)` (no separator) would wave this cross-workspace + // read through — this test fails the moment that separator is dropped. + const fooRoot = path.join(tempDir, 'foo'); + const siblingQwen = path.join(tempDir, 'foobar', '.qwen'); + await fs.mkdir(fooRoot, { recursive: true }); + await fs.mkdir(siblingQwen, { recursive: true }); + await fs.writeFile(path.join(siblingQwen, 'loop.md'), 'sibling tasks'); + await fs.symlink(siblingQwen, path.join(fooRoot, '.qwen')); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot: fooRoot, + homeDir, + allowProjectFile: true, + }); + + // Refused → falls through to home; the sibling content is never returned. + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('does not read a project loop.md symlinked to an in-workspace file (exfiltration guard)', async () => { + // The dangerous case confinement alone misses: a repo-committed + // `.qwen/loop.md -> ../.env` resolves INSIDE the workspace, so the realpath + // confinement passes — yet it must NOT be read. A symlinked project loop.md + // is refused outright; only a real regular file at the literal path is read. + await fs.mkdir(path.join(projectRoot, '.qwen'), { recursive: true }); + const secret = path.join(projectRoot, '.env'); + await fs.writeFile(secret, 'SECRET=should-not-be-read'); + await fs.symlink( + path.join('..', '.env'), + path.join(projectRoot, '.qwen', 'loop.md'), + ); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('does not read a HARD-LINKED project loop.md (exfiltration guard)', async () => { + // The case the symlink guard misses: `ln .qwen/loop.md` makes + // loop.md an ordinary regular file (lstat sees no symlink, isFile() true) + // that SHARES the secret's inode (nlink === 2). It resolves to itself inside + // the workspace, so confinement passes too — only the `nlink > 1` guard + // refuses it. Mutation check: drop that guard and the secret is returned as + // the project source instead of falling through to home. + await fs.mkdir(path.join(projectRoot, '.qwen'), { recursive: true }); + const secret = path.join(tempDir, 'secret-env'); + await fs.writeFile(secret, 'SECRET=should-not-be-read'); + const projectLoop = path.join(projectRoot, '.qwen', 'loop.md'); + await fs.link(secret, projectLoop); // hard link → nlink 2, same inode + // Precondition: the link really is a hard link to the secret, not a symlink. + const linkStat = await fs.lstat(projectLoop); + expect(linkStat.isSymbolicLink()).toBe(false); + expect(linkStat.nlink).toBeGreaterThan(1); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + // The hard-linked project file is skipped → home is read; the secret content + // is never returned from any candidate. + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(result.content).not.toContain('SECRET'); + }); + + it('does not read a HARD-LINKED home loop.md (exfiltration guard)', async () => { + // Same hard-link vector on the home candidate: `ln ~/.qwen/loop.md`. + // fs.stat follows to a regular file with nlink 2, so isFile()/confinement + // pass — only the `nlink > 1` guard refuses it. No project file here, so the + // result is `missing`; the secret content is never returned. + await fs.mkdir(path.join(homeDir, '.qwen'), { recursive: true }); + const secret = path.join(tempDir, 'home-secret'); + await fs.writeFile(secret, 'SECRET=should-not-be-read'); + const homeLoop = path.join(homeDir, '.qwen', 'loop.md'); + await fs.link(secret, homeLoop); + const linkStat = await fs.lstat(homeLoop); + expect(linkStat.nlink).toBeGreaterThan(1); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'missing', + checkedPaths: [ + path.join(projectRoot, '.qwen', 'loop.md'), + path.join(homeDir, '.qwen', 'loop.md'), + ], + }); + }); + + it('does not falsely refuse a project loop.md when the workspace root is a filesystem root', async () => { + // When the CLI runs from a filesystem root, realRoot is `/` (or `C:\`), so the + // old `realRoot + path.sep` prefix became `//` (`C:\\`) — which no descendant + // startsWith, wrongly refusing every project loop.md. Drive realRoot to the + // filesystem root via a realpath mock; the real loop.md still resolves to a + // normal absolute path (a descendant of the root) and must be read, not refused. + await writeProject('- root-level tasks'); + const root = path.parse(projectRoot).root; // '/' on POSIX, e.g. 'C:\\' on Windows + const actual = + await vi.importActual( + 'node:fs/promises', + ); + vi.spyOn(fs, 'realpath').mockImplementation((p) => + String(p) === projectRoot + ? Promise.resolve(root) + : actual.realpath(p as string), + ); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toMatchObject({ + status: 'found', + source: 'project', + content: '- root-level tasks', + }); + }); + + it('skips a FIFO/non-regular project loop.md before opening it (does not hang)', async () => { + // A FIFO at the project path must be rejected BEFORE the blocking fs.open: + // open() on a FIFO blocks until a writer appears, wedging the tick forever. + // Drive a FIFO-typed node via a mocked lstat (a real mkfifo is platform- + // fragile); the load-bearing proof is that fs.open is never called on the + // project path, so no blocking open() can happen. + await writeHome('user tasks'); + const projectLoop = path.join(projectRoot, '.qwen', 'loop.md'); + const actual = + await vi.importActual( + 'node:fs/promises', + ); + const fifoStat = { + isSymbolicLink: () => false, + isFile: () => false, + isFIFO: () => true, + } as unknown as Awaited>; + vi.spyOn(fs, 'lstat').mockImplementation(async (p) => + String(p) === projectLoop ? fifoStat : actual.lstat(p as string), + ); + const openSpy = vi.mocked(fs.open); + openSpy.mockClear(); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toMatchObject({ source: 'home', content: 'user tasks' }); + // The project FIFO path is never opened — proof there is no blocking open(). + for (const call of openSpy.mock.calls) { + expect(String(call[0])).not.toBe(projectLoop); + } + }); + + it('reads a home loop.md that is a symlink to a real regular file inside $HOME', async () => { + // The user's own dotfile may legitimately be a symlink (e.g. into a synced + // dotfiles repo). Follow it, as long as the target is a real regular file + // that resolves WITHIN $HOME (the confinement added for escapes). + await fs.mkdir(path.join(homeDir, '.qwen'), { recursive: true }); + const target = path.join(homeDir, 'dotfiles', 'loop.md'); + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, 'symlinked user tasks'); + await fs.symlink(target, path.join(homeDir, '.qwen', 'loop.md')); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'symlinked user tasks', + truncated: false, + }); + }); + + it('skips a home loop.md whose symlink target escapes $HOME', async () => { + // Home symlinks are allowed (dotfiles repos), but only if they resolve + // WITHIN $HOME. A `~/.qwen/loop.md -> /etc/passwd`-style escape (here a + // sibling outside homeDir) must be skipped, not read and fed to the model. + await fs.mkdir(path.join(homeDir, '.qwen'), { recursive: true }); + const outside = path.join(tempDir, 'outside-secret'); + await fs.writeFile(outside, 'SECRET=should-not-be-read'); + await fs.symlink(outside, path.join(homeDir, '.qwen', 'loop.md')); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'missing', + checkedPaths: [ + path.join(projectRoot, '.qwen', 'loop.md'), + path.join(homeDir, '.qwen', 'loop.md'), + ], + }); + }); + + it('reads the home loop.md from a relocated homeQwenDir (QWEN_HOME)', async () => { + // The home candidate lives in the QWEN_HOME-aware global dir, not always + // /.qwen — write loop.md into a relocated global dir and confirm it + // is read as the `home` source from /loop.md. + const relocated = path.join(tempDir, 'relocated-qwen'); + await fs.mkdir(relocated, { recursive: true }); + await fs.writeFile(path.join(relocated, 'loop.md'), 'relocated user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + // Caller passes the global dir as both candidate dir and confinement root + // when QWEN_HOME is set (see Session.#getLoopTickResolver). + homeDir: relocated, + homeQwenDir: relocated, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(relocated, 'loop.md'), + source: 'home', + content: 'relocated user tasks', + truncated: false, + }); + }); + + it('keeps confinement for a relocated homeQwenDir (escaping symlink refused)', async () => { + // Relocation must not loosen the earlier confinement: a symlink whose target + // escapes the home confinement root is still refused, not read. + const relocated = path.join(tempDir, 'relocated-qwen'); + await fs.mkdir(relocated, { recursive: true }); + const outside = path.join(tempDir, 'outside-secret'); + await fs.writeFile(outside, 'SECRET=should-not-be-read'); + await fs.symlink(outside, path.join(relocated, 'loop.md')); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir: relocated, + homeQwenDir: relocated, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'missing', + checkedPaths: [ + path.join(projectRoot, '.qwen', 'loop.md'), + path.join(relocated, 'loop.md'), + ], + }); + }); + + it('skips a home loop.md that is a self-referential symlink (ELOOP) instead of throwing', async () => { + // fs.stat follows the home symlink; a self-referential link raises ELOOP. + // That must be treated as a skippable candidate (→ missing), not crash the + // tick — without ELOOP in the skip whitelist this rethrows and aborts. + await fs.mkdir(path.join(homeDir, '.qwen'), { recursive: true }); + const loop = path.join(homeDir, '.qwen', 'loop.md'); + await fs.symlink(loop, loop); // points at itself → ELOOP on stat + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'missing', + checkedPaths: [path.join(projectRoot, '.qwen', 'loop.md'), loop], + }); + }); + + it('skips a home loop.md that resolves to a non-regular file (directory/FIFO)', async () => { + // The home candidate follows symlinks via fs.stat; if the (possibly + // symlinked) target is a directory/FIFO it must be skipped — the project + // path proves this via lstat, but the home path's fs.stat needs its own + // coverage so a blocking open / directory read never happens. + const homeLoop = path.join(homeDir, '.qwen', 'loop.md'); + await fs.mkdir(path.dirname(homeLoop), { recursive: true }); + await fs.writeFile(homeLoop, '- user tasks'); // real file so realpath resolves + const actual = + await vi.importActual( + 'node:fs/promises', + ); + const dirStat = { + isFile: () => false, + isDirectory: () => true, + } as unknown as Awaited>; + vi.spyOn(fs, 'stat').mockImplementation(async (p) => + String(p) === homeLoop ? dirStat : actual.stat(p as string), + ); + const openSpy = vi.mocked(fs.open); + openSpy.mockClear(); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('missing'); + // The non-regular guard fired before any open() on the home path. + for (const call of openSpy.mock.calls) { + expect(String(call[0])).not.toBe(homeLoop); + } + }); + + it('defaults to fail-secure: omitting allowProjectFile skips the project file', async () => { + // This function is re-exported from the core barrel; an external caller that + // forgets the option must NOT read the repo-controlled project loop.md from + // an untrusted workspace. The default is false — callers opt IN to trust. + await writeProject('repo-controlled tasks'); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ projectRoot, homeDir }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('skips the project candidate entirely when allowProjectFile is false', async () => { + // Untrusted folder: the repo-controlled project loop.md is not read even + // when present; the user-owned home loop.md still is. + await writeProject('repo-controlled tasks'); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: false, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('reports only the home path as missing when allowProjectFile is false', async () => { + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: false, + }); + + expect(result).toEqual({ + status: 'missing', + checkedPaths: [path.join(homeDir, '.qwen', 'loop.md')], + }); + }); + + it('skips a non-directory component at .qwen (ENOTDIR) and falls through', async () => { + // A regular file where the `.qwen` dir should be → reading .qwen/loop.md + // raises ENOTDIR; skip to home rather than throwing. + await fs.writeFile(path.join(projectRoot, '.qwen'), 'not a dir'); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('skips a directory at the loop.md path and falls through', async () => { + // A directory at the project path yields EISDIR on read — skip it, not throw. + await fs.mkdir(path.join(projectRoot, '.qwen', 'loop.md'), { + recursive: true, + }); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('rethrows non-whitelisted fs errors (e.g. EACCES)', async () => { + // Only ENOENT/EISDIR/ENOTDIR fall through to the next candidate; a real + // error such as a permission denial must surface, not be swallowed. + await writeProject('project tasks'); + const eacces = Object.assign(new Error('EACCES: permission denied'), { + code: 'EACCES', + }); + vi.mocked(fs.open).mockRejectedValueOnce(eacces); + + await expect( + readLoopTaskFile({ projectRoot, homeDir, allowProjectFile: true }), + ).rejects.toThrow(/EACCES/); + }); + + it('evicts the cached project-root realpath after a transient failure and retries on the next tick', async () => { + // The project-root realpath is cached per process. A TRANSIENT failure + // (EACCES/ENOENT) must NOT be pinned: the entry is evicted on rejection so + // the next tick re-resolves instead of replaying a permanently-cached + // rejection. Drop that eviction and one transient error would break loop.md + // resolution for this root forever. Drive it purely via the realpath mock. + await writeProject('project tasks'); + + const eacces = Object.assign(new Error('EACCES: permission denied'), { + code: 'EACCES', + }); + const actual = + await vi.importActual( + 'node:fs/promises', + ); + const realpathSpy = vi.spyOn(fs, 'realpath'); + // Fail the first project-root resolution, then resolve normally. + realpathSpy.mockRejectedValueOnce(eacces); + realpathSpy.mockImplementation((p) => actual.realpath(p as string)); + + // First tick: the transient error surfaces (current per-tick semantics). + await expect( + readLoopTaskFile({ projectRoot, homeDir, allowProjectFile: true }), + ).rejects.toThrow(/EACCES/); + + // Second tick: the poisoned entry was evicted, so realpath is retried and + // the project loop.md resolves — proving the rejection was not cached. + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(projectRoot, '.qwen', 'loop.md'), + source: 'project', + content: 'project tasks', + truncated: false, + }); + // The root was re-resolved on the retry (call #2), not served from a + // poisoned cache entry; #3 is the loop.md realpath on the successful tick. + expect(realpathSpy.mock.calls.length).toBeGreaterThanOrEqual(2); + }); + + it('skips an empty or whitespace-only file and falls through', async () => { + await writeProject(' \n\t \n'); + await writeHome('user tasks'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'found', + path: path.join(homeDir, '.qwen', 'loop.md'), + source: 'home', + content: 'user tasks', + truncated: false, + }); + }); + + it('logs a debug line when it skips a whitespace-only loop.md', async () => { + // The whitespace-only skip was the ONLY skip branch with no debug log, so a + // present-but-empty file was indistinguishable from an absent one in logs. + // Assert the labelled skip line fires for the project candidate before the + // fall-through to home. + await writeProject(' \n\t \n'); + await writeHome('user tasks'); + debugSpy.mockClear(); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toMatchObject({ source: 'home', content: 'user tasks' }); + expect(debugSpy).toHaveBeenCalledWith('skipping whitespace-only loop.md', { + source: 'project', + filePath: path.join(projectRoot, '.qwen', 'loop.md'), + }); + }); + + it('returns missing when every candidate is empty', async () => { + await writeProject(''); + await writeHome('\n \n'); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toEqual({ + status: 'missing', + checkedPaths: [ + path.join(projectRoot, '.qwen', 'loop.md'), + path.join(homeDir, '.qwen', 'loop.md'), + ], + }); + }); + + it('returns a missing result when no task file exists', async () => { + await expect( + readLoopTaskFile({ projectRoot, homeDir, allowProjectFile: true }), + ).resolves.toEqual({ + status: 'missing', + checkedPaths: [ + path.join(projectRoot, '.qwen', 'loop.md'), + path.join(homeDir, '.qwen', 'loop.md'), + ], + }); + }); + + it('byte-caps task files above the cap and flags them truncated', async () => { + await writeProject('x'.repeat(LOOP_TASK_FILE_MAX_BYTES + 5)); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(Buffer.byteLength(result.content, 'utf8')).toBe( + LOOP_TASK_FILE_MAX_BYTES, + ); + expect(result.truncated).toBe(true); + }); + + it('bounds the read for a very large file (never reads past the cap)', async () => { + // A multi-MB file must not be fully read/decoded every tick. Observe the + // actual handle.read() calls: neither any single read nor their sum may + // exceed the cap budget — so a "read the whole file, then slice" regression + // (which would pull all 2 MB through these reads) fails this test. + await writeProject('x'.repeat(2_000_000)); + const cap = LOOP_TASK_FILE_MAX_BYTES + 1; + const openSpy = vi.mocked(fs.open); + openSpy.mockClear(); + const readLengths = await recordHandleReadLengths(); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(result.truncated).toBe(true); + expect(Buffer.byteLength(result.content, 'utf8')).toBe( + LOOP_TASK_FILE_MAX_BYTES, + ); + // A single bounded fs.open handle, not fs.readFile of the whole. + expect(openSpy).toHaveBeenCalledTimes(1); + // Load-bearing: every read, and the total bytes requested, stay within cap. + expect(readLengths.length).toBeGreaterThan(0); + for (const length of readLengths) { + expect(length).toBeLessThanOrEqual(cap); + } + expect(readLengths.reduce((a, b) => a + b, 0)).toBeLessThanOrEqual(cap); + }); + + it('reads a short file fully via bounded reads that never exceed the cap', async () => { + // The EOF path: a sub-cap file is returned whole (not truncated), and the + // bounded reader still never requests past the cap on any read. + const body = 'short tasks\n'; + await writeProject(body); + const cap = LOOP_TASK_FILE_MAX_BYTES + 1; + const readLengths = await recordHandleReadLengths(); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toMatchObject({ + status: 'found', + content: body, + truncated: false, + }); + expect(readLengths.length).toBeGreaterThan(0); + for (const length of readLengths) { + expect(length).toBeLessThanOrEqual(cap); + } + // Load-bearing: the buffer is sized to the file (+1 for truncation + // detection), NOT the 25 KB cap — so a tiny loop.md doesn't zero-fill 25 KB + // every tick. The first read requests exactly that bounded length. + expect(readLengths[0]).toBe(body.length + 1); + }); + + it('does not truncate task files at exactly the byte cap', async () => { + await writeProject('x'.repeat(LOOP_TASK_FILE_MAX_BYTES)); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(Buffer.byteLength(result.content, 'utf8')).toBe( + LOOP_TASK_FILE_MAX_BYTES, + ); + expect(result.truncated).toBe(false); + }); + + it('truncates on a UTF-8 boundary without exceeding the cap or inserting a replacement char', async () => { + // 3-byte chars make the raw byte cap land mid-character. + await writeProject('一'.repeat(LOOP_TASK_FILE_MAX_BYTES)); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(result.truncated).toBe(true); + expect(Buffer.byteLength(result.content, 'utf8')).toBeLessThanOrEqual( + LOOP_TASK_FILE_MAX_BYTES, + ); + expect(result.content).not.toContain('�'); + }); + + it('drops an INCOMPLETE trailing multi-byte sequence at the cap (no orphan lead / U+FFFD)', async () => { + // A buffer cut mid-4-byte-sequence whose final byte is NOT a continuation + // (the sequence is incomplete) defeats the continuation-only back-off: it + // would keep `f0 9f a6` and decode a trailing U+FFFD. Sized so the orphan + // lands exactly on the cap, so the byte-length re-clamp can't mask it — only + // dropping the whole incomplete lead keeps the tail clean. + const head = Buffer.alloc(LOOP_TASK_FILE_MAX_BYTES - 3, 0x61); // 'a' * (cap-3) + const partial = Buffer.from([0xf0, 0x9f, 0xa6]); // 3 of a 4-byte char... + const tail = Buffer.from([0x62]); // ...then 'b' (non-continuation) → incomplete + const raw = Buffer.concat([head, partial, tail]); // cap + 1 bytes → truncated + await fs.mkdir(path.join(projectRoot, '.qwen'), { recursive: true }); + await fs.writeFile(path.join(projectRoot, '.qwen', 'loop.md'), raw); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(result.truncated).toBe(true); + expect(Buffer.byteLength(result.content, 'utf8')).toBeLessThanOrEqual( + LOOP_TASK_FILE_MAX_BYTES, + ); + // The incomplete sequence is gone entirely — no replacement char, and the + // body ends on the last complete ('a') char. + expect(result.content).not.toContain('�'); + expect(result.content.endsWith('a')).toBe(true); + }); + + it('drops an INCOMPLETE trailing 2-byte lead at the cap (covers the 2-byte width branch)', async () => { + // A lone 2-byte lead (0xc3, its continuation replaced by a non-continuation) + // must be dropped by the width branch ((b & 0xe0) === 0xc0 → width 2), not + // kept as an orphan decoding to U+FFFD. The two trailing continuation bytes + // are sized so a width-table regression (treating 0xc3 as width 1) leaves + // the orphan's U+FFFD at exactly the cap, where the byte-length re-clamp + // can't mask it — catching a regression the re-clamp alone would hide. + const N = LOOP_TASK_FILE_MAX_BYTES; + const head = Buffer.alloc(N - 3, 0x61); // 'a' * (N-3) + const tail = Buffer.from([0xc3, 0x41, 0x80, 0x80]); // 2-byte lead, 'A', 2 conts + const raw = Buffer.concat([head, tail]); // N + 1 bytes → truncated + await fs.mkdir(path.join(projectRoot, '.qwen'), { recursive: true }); + await fs.writeFile(path.join(projectRoot, '.qwen', 'loop.md'), raw); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(result.truncated).toBe(true); + expect(result.content).not.toContain('�'); + expect(result.content).toBe('a'.repeat(N - 3)); + }); + + it('drops an INCOMPLETE trailing 3-byte lead at the cap (covers the 3-byte width branch)', async () => { + // A 3-byte lead with only ONE of its two continuations (0xe4 0xb8) followed + // by a non-continuation must be dropped by the width branch + // ((b & 0xf0) === 0xe0 → width 3). Sized so a width-table regression (0xe4 + // treated as width 1 or 2) leaves the orphan's U+FFFD below the cap, where + // it survives the re-clamp — so the regression is observable. + const N = LOOP_TASK_FILE_MAX_BYTES; + const head = Buffer.alloc(N - 4, 0x61); // 'a' * (N-4) + const tail = Buffer.from([0xe4, 0xb8, 0x41, 0x80, 0x80]); // lead+1 cont, 'A', 2 conts + const raw = Buffer.concat([head, tail]); // N + 1 bytes → truncated + await fs.mkdir(path.join(projectRoot, '.qwen'), { recursive: true }); + await fs.writeFile(path.join(projectRoot, '.qwen', 'loop.md'), raw); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(result.truncated).toBe(true); + expect(result.content).not.toContain('�'); + expect(result.content).toBe('a'.repeat(N - 4)); + }); + + it('drops an ORPHAN lead followed by an ASCII byte and stray continuations (no U+FFFD)', async () => { + // The continuation back-off walks `lead` to the LAST non-continuation byte, + // so a `> end` width check alone stops at the ASCII `0x41` (a complete 1-byte + // char) and keeps the orphan `0xc3` before it plus the three stray `0x80` + // continuations after it — all of which decode to trailing U+FFFD. Re-checking + // the boundary against the EXACT char width (and re-running after each trim) + // is what strips the whole malformed tail. The trailing `0x61` defeats the + // initial continuation back-off, so the stray `0x80` bytes are not at the very + // end and only the boundary loop removes them. + const N = LOOP_TASK_FILE_MAX_BYTES; + const head = Buffer.alloc(N - 5, 0x61); // 'a' * (N-5) + const tail = Buffer.from([0xc3, 0x41, 0x80, 0x80, 0x80, 0x61]); // orphan lead, 'A', 3 conts, 'a' + const raw = Buffer.concat([head, tail]); // N + 1 bytes → truncated + await fs.mkdir(path.join(projectRoot, '.qwen'), { recursive: true }); + await fs.writeFile(path.join(projectRoot, '.qwen', 'loop.md'), raw); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result.status).toBe('found'); + if (result.status !== 'found') { + throw new Error('expected loop.md to be found'); + } + expect(result.truncated).toBe(true); + expect(Buffer.byteLength(result.content, 'utf8')).toBeLessThanOrEqual( + LOOP_TASK_FILE_MAX_BYTES, + ); + // The whole malformed tail is gone: no replacement char, and the body ends + // on the last complete ('a') char — a clean UTF-8 boundary. + expect(result.content).not.toContain('�'); + expect(result.content).toBe('a'.repeat(N - 5)); + }); + + it('skips a candidate that raises ENAMETOOLONG and falls through instead of throwing', async () => { + // The over-long-path code is in the skip whitelist but otherwise untested; a + // typo'd entry would start throwing on a real ENAMETOOLONG instead of falling + // through. Drive it via a mocked lstat on the project path; home still reads. + await writeHome('user tasks'); + const projectLoop = path.join(projectRoot, '.qwen', 'loop.md'); + const actual = + await vi.importActual( + 'node:fs/promises', + ); + const enametoolong = Object.assign(new Error('ENAMETOOLONG'), { + code: 'ENAMETOOLONG', + }); + vi.spyOn(fs, 'lstat').mockImplementation(async (p) => + String(p) === projectLoop + ? Promise.reject(enametoolong) + : actual.lstat(p as string), + ); + + const result = await readLoopTaskFile({ + projectRoot, + homeDir, + allowProjectFile: true, + }); + + expect(result).toMatchObject({ + status: 'found', + source: 'home', + content: 'user tasks', + }); + }); + + it('lets the original read error propagate when handle.close() also throws', async () => { + // readBoundedTaskFile closes the handle in a `finally`. If read() throws + // (e.g. EIO) AND close() also throws (e.g. EBADF), an unguarded `finally` + // would replace the original I/O error with the close error, masking the + // real cause. The close is guarded, so the ORIGINAL read error must survive. + await writeProject('project tasks'); // real file so lstat/realpath/confine pass + const eio = Object.assign(new Error('EIO: i/o error, read'), { + code: 'EIO', + }); + const ebadf = Object.assign( + new Error('EBADF: bad file descriptor, close'), + { code: 'EBADF' }, + ); + const close = vi.fn().mockRejectedValue(ebadf); + const fakeHandle = { + stat: async () => ({ isFile: () => true, size: 100 }), + read: vi.fn().mockRejectedValue(eio), + close, + } as unknown as Awaited>; + // The first fs.open is the project candidate (read first); hand it the + // fake handle. lstat/realpath above this still run against the real file. + vi.mocked(fs.open).mockImplementationOnce(async () => fakeHandle); + + await expect( + readLoopTaskFile({ projectRoot, homeDir, allowProjectFile: true }), + ).rejects.toBe(eio); + // The close was still attempted (we swallow its failure, not skip it). + expect(close).toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/skills/bundled/loop/loop-task-file.ts b/packages/core/src/skills/bundled/loop/loop-task-file.ts new file mode 100644 index 00000000000..749d221e227 --- /dev/null +++ b/packages/core/src/skills/bundled/loop/loop-task-file.ts @@ -0,0 +1,404 @@ +/** + * @license + * Copyright 2026 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { createDebugLogger } from '../../../utils/debugLogger.js'; + +const debugLogger = createDebugLogger('LOOP_TASK_FILE'); + +export const LOOP_TASK_FILE_MAX_BYTES = 25_000; + +/** Which candidate a found loop.md came from. The caller maps this to a label + * (an exhaustive map fails closed if a new candidate is added). */ +export type LoopTaskFileSource = 'project' | 'home'; + +export type LoopTaskFileResult = + | { + status: 'found'; + path: string; + source: LoopTaskFileSource; + content: string; + truncated: boolean; + } + | { + status: 'missing'; + checkedPaths: string[]; + }; + +export interface ReadLoopTaskFileOptions { + projectRoot: string; + /** + * Confinement root for the home candidate's resolved (symlink-followed) + * target — a target escaping this dir (e.g. `-> /etc/passwd`) is refused while + * an in-root dotfile symlink is followed. Pass `$QWEN_HOME` when set, else + * `$HOME` (see `homeQwenDir`). + */ + homeDir: string; + /** + * Directory holding the home/global `loop.md` candidate (`/loop.md`). + * Pass the QWEN_HOME-aware global dir (`Storage.getGlobalQwenDir()`) so a + * relocated config home is honored instead of always reading the real OS home. + * Defaults to `/.qwen` so a direct barrel caller keeps the `~/.qwen` + * layout. + */ + homeQwenDir?: string; + /** + * When false, the project `.qwen/loop.md` candidate is skipped entirely — it + * is repo-controlled, so an untrusted workspace must not read it and feed it + * to the model (mirrors the folder-trust gate on project hooks). The + * home/global `~/.qwen/loop.md` is user-owned and always allowed. + * + * Defaults to false (fail-secure): this function is re-exported from the core + * barrel, so a caller that omits the option must NOT silently read an + * untrusted workspace's repo-controlled file — callers opt IN by passing the + * trust-derived value explicitly. + */ + allowProjectFile?: boolean; + /** + * Per-resolver cache for the boundary `fs.realpath()` results. LoopTickResolver + * passes its own instance-scoped Map so the cache lifetime is tied to the + * resolver (rebuilt on `/cd`, cleared by `resetCache()`) instead of living + * forever at module scope. Omitted by direct barrel callers, who fall back to a + * process-lifetime cache. Eviction-on-failure is preserved either way. + */ + realDirCache?: Map>; +} + +/** + * Process-lifetime fallback `fs.realpath(dir)` cache for the two confinement + * boundaries — the workspace root and the home dir. Used only by direct callers + * of this re-exported function that don't supply their own cache; resolver-driven + * ticks pass an instance-scoped cache (see `ReadLoopTaskFileOptions.realDirCache`) + * so the boundary realpath stays invalidatable and a long-lived process can't pin + * a stale boundary after a `/cd` or symlink re-point. Keyed by the TRUSTED dir the + * caller passes (never a path derived from file contents), so a caller can't widen + * a boundary with a stale/broader path. + */ +const moduleRealDirCache = new Map>(); + +function resolveRealDir( + dir: string, + cache: Map>, +): Promise { + let real = cache.get(dir); + if (real === undefined) { + real = fs.realpath(dir); + // Don't pin a rejection: a transient failure (EACCES, ENOENT) must be + // retried next tick rather than cached, preserving per-tick error semantics. + real.catch(() => cache.delete(dir)); + cache.set(dir, real); + } + return real; +} + +/** + * True when `real` is `root` itself or a descendant of it — the prefix + * confinement shared by the project and home candidates. The separator isn't + * double-appended: at a filesystem root `root` is already `/` (or `C:\`), so + * `root + path.sep` would be `//` / `C:\\`, which no descendant startsWith, + * wrongly refusing everything — so `real === root` is allowed too. + */ +function isWithin(root: string, real: string): boolean { + if (real === root) { + return true; + } + const prefix = root.endsWith(path.sep) ? root : root + path.sep; + return real.startsWith(prefix); +} + +/** + * Read at most `LOOP_TASK_FILE_MAX_BYTES + 1` bytes — the one extra byte is the + * truncation signal and the only thing we need past the cap, so a huge/malicious + * loop.md is never fully read or decoded. Returns `null` for a non-regular node + * (e.g. a directory at the loop.md path) so the caller skips to the next + * candidate. Symlink/escape filtering is the caller's job and already done. + */ +async function readBoundedTaskFile(filePath: string): Promise { + const handle = await fs.open(filePath, 'r'); + try { + const stat = await handle.stat(); + if (!stat.isFile()) { + return null; + } + const cap = LOOP_TASK_FILE_MAX_BYTES + 1; + // Size the buffer to the file (+1 to still detect a file that exceeds the + // cap), never above cap — so a small loop.md doesn't zero-fill 25 KB every + // tick. `read` below is bounded by this length too, so a file that grows + // past `stat.size` between stat and read is still read safely (its tail just + // isn't seen this tick). + const allocSize = Math.min(cap, stat.size + 1); + const buffer = Buffer.alloc(allocSize); + let total = 0; + // A single read() may return short even before EOF; loop until full or EOF. + while (total < allocSize) { + const { bytesRead } = await handle.read( + buffer, + total, + allocSize - total, + total, + ); + if (bytesRead === 0) { + break; + } + total += bytesRead; + } + return buffer.subarray(0, total); + } finally { + // Guard the close so a close failure (e.g. EBADF) can't replace an in-flight + // read/stat error (e.g. EIO) — JS would otherwise surface the close error and + // mask the original. Swallow it (debug-log only) and let the original throw. + try { + await handle.close(); + } catch (closeErr) { + debugLogger.debug('failed to close loop.md handle', { closeErr }); + } + } +} + +/** + * Reads `.qwen/loop.md`, project before home, byte-capped at 25 KB. A missing, + * directory, non-regular, or empty (whitespace-only) path is skipped to the next + * candidate rather than treated as present; all candidates exhausted → missing. + * Only the byte cap lives here — the fire-time resolver owns the user-facing + * truncation notice so the byte-vs-line nuance stays in one place. + * + * Project candidate: must be a real regular file at the literal path, and is + * stat'd BEFORE the blocking open. A symlinked `.qwen/loop.md` is refused + * outright — a repo-controlled symlink such as `-> ../.env` resolves *inside* + * the workspace, so confinement alone would pass and exfiltrate that file to the + * model. A FIFO/socket/device/dir is refused too, so a named pipe can never + * wedge the tick (a blocking `open` on a FIFO waits for a writer) or be read as + * a task list. The canonical path is still confined to the workspace root to + * catch an *ancestor* symlink like a checked-in `.qwen -> /outside` that a + * final-component `lstat` cannot see. When `allowProjectFile` is false (untrusted + * folder) the candidate is dropped entirely. + * + * Home candidate: `/loop.md` (the QWEN_HOME-aware global dir, not + * always the real `~/.qwen`). It is the user's own dotfile, so a symlink IS + * followed (a common, legitimate setup — e.g. into a synced dotfiles repo), but + * the resolved target must be a regular file AND stay within the home + * confinement root (`homeDir`: `$QWEN_HOME` or `$HOME`) so a FIFO/device/dir + * can't hang the tick and an escaping symlink (e.g. `-> /etc/passwd`) can't be + * exfiltrated. + */ +export async function readLoopTaskFile({ + projectRoot, + homeDir, + homeQwenDir = path.join(homeDir, '.qwen'), + allowProjectFile = false, + realDirCache = moduleRealDirCache, +}: ReadLoopTaskFileOptions): Promise { + if (!allowProjectFile) { + // Repo-controlled file in an untrusted folder — never read it (the + // candidate is dropped below; this is the trace for why). + debugLogger.debug('skipping project loop.md: folder is untrusted'); + } + const candidates: ReadonlyArray<{ + source: LoopTaskFileSource; + path: string; + }> = [ + ...(allowProjectFile + ? [ + { + source: 'project' as const, + path: path.join(projectRoot, '.qwen', 'loop.md'), + }, + ] + : []), + { source: 'home', path: path.join(homeQwenDir, 'loop.md') }, + ]; + + for (const { source, path: filePath } of candidates) { + let buffer: Buffer | null; + try { + if (source === 'project') { + // lstat WITHOUT following the final component, BEFORE the blocking open. + // A symlinked loop.md is the exfiltration vector (it may point at an + // in-workspace `.env`, which confinement would wave through), so refuse + // it; a FIFO/socket/device/dir is refused too so open can never block. + const projectStat = await fs.lstat(filePath); + if (projectStat.isSymbolicLink()) { + debugLogger.debug('skipping symlinked project loop.md', { filePath }); + continue; + } + if (!projectStat.isFile()) { + debugLogger.debug('skipping non-regular project loop.md', { + filePath, + }); + continue; + } + // A hard-linked loop.md is an ordinary regular file (lstat sees no + // symlink) but shares a sensitive target's inode (e.g. `ln .env + // .qwen/loop.md`), so confinement passes on the same fs and the secret + // would be read every tick. `nlink > 1` is the only tell — refuse it, + // mirroring canonicalizeKeytermsFile. + if (projectStat.nlink > 1) { + debugLogger.debug('skipping hard-linked project loop.md', { + filePath, + }); + continue; + } + // A final-component lstat can't see an ANCESTOR symlink (e.g. a + // checked-in `.qwen -> /outside`); realpath resolves it, so confine the + // canonical path to the workspace root before reading. + const realRoot = await resolveRealDir(projectRoot, realDirCache); + const real = await fs.realpath(filePath); + if (!isWithin(realRoot, real)) { + debugLogger.debug( + 'skipping project loop.md that escapes the workspace', + { + filePath, + resolved: real, + }, + ); + continue; + } + buffer = await readBoundedTaskFile(real); + } else { + // Home loop.md is the user's own dotfile: a symlink is a legitimate, + // common setup, so follow it (stat, not lstat). But require the resolved + // target to be a regular file so a FIFO/device/dir can neither hang the + // tick on a blocking open nor be decoded as a task list. + const homeStat = await fs.stat(filePath); + if (!homeStat.isFile()) { + debugLogger.debug('skipping non-regular home loop.md', { filePath }); + continue; + } + // Same hard-link guard as the project candidate: a `nlink > 1` regular + // file shares another inode's content (e.g. `ln ~/.ssh/id_ed25519 + // ~/.qwen/loop.md`) and would otherwise be read and fed to the model. + if (homeStat.nlink > 1) { + debugLogger.debug('skipping hard-linked home loop.md', { filePath }); + continue; + } + // A home symlink IS followed, but its target must stay WITHIN $HOME: + // otherwise `~/.qwen/loop.md -> /etc/passwd` (or `-> /dev/...`) would be + // read and fed to the model every tick. In-home dotfile symlinks (e.g. + // `-> ~/dotfiles/loop.md`) still resolve inside $HOME and are allowed. + const realHome = await resolveRealDir(homeDir, realDirCache); + const real = await fs.realpath(filePath); + if (!isWithin(realHome, real)) { + debugLogger.debug( + 'skipping home loop.md that escapes the home directory', + { filePath, resolved: real }, + ); + continue; + } + buffer = await readBoundedTaskFile(real); + } + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + // None of these name a readable loop.md, so try the next candidate: + // absent (ENOENT), a directory (EISDIR), a non-directory path component + // (ENOTDIR, e.g. a stray file where `.qwen` should be), a symlink loop + // (ELOOP, e.g. a self-referential `~/.qwen/loop.md`), or an over-long path + // (ENAMETOOLONG). Anything else (EACCES permissions, real I/O) surfaces + // rather than being silently swallowed. + if ( + code === 'ENOENT' || + code === 'EISDIR' || + code === 'ENOTDIR' || + code === 'ELOOP' || + code === 'ENAMETOOLONG' + ) { + continue; + } + throw error; + } + + // A non-regular node (e.g. a directory where loop.md was expected) → skip. + if (buffer === null) { + continue; + } + + // A whitespace-only file is not a task list; fall through to the next path. + // Log it (like every other skip branch) so a present-but-empty loop.md is + // distinguishable from an absent one in debug logs. + if (buffer.toString('utf8').trim().length === 0) { + debugLogger.debug('skipping whitespace-only loop.md', { + source, + filePath, + }); + continue; + } + + const truncated = buffer.byteLength > LOOP_TASK_FILE_MAX_BYTES; + let content: string; + if (truncated) { + // Cap by bytes on a UTF-8 char boundary. First back off any trailing + // continuation bytes (10xxxxxx) left by a mid-character cut at the cap... + let end = LOOP_TASK_FILE_MAX_BYTES; + while (end > 0 && (buffer[end] & 0xc0) === 0x80) { + end--; + } + // ...then drop any malformed trailing unit. `lead` is the last + // non-continuation byte, and the back-off skipped exactly the continuation + // bytes after it, so the trailing character is well-formed iff its declared + // width reaches `end` exactly. A mismatch is either an INCOMPLETE lead (too + // few continuations, `lead + width > end`) or an ORPHAN lead whose stray + // continuations belong to nothing (`lead + width < end`) — e.g. a width + // check that only tests `> end` keeps `c3 41 80 80 80` (orphan `c3` plus + // stray continuations after the `41`). Drop the unit and re-check, since + // several malformed units can stack. Each surviving orphan decodes to a + // trailing U+FFFD the byte-length re-clamp below cannot remove, so this loop + // is load-bearing and the re-clamp is a pure safety net. + while (end > 0) { + let lead = end - 1; + while (lead >= 0 && (buffer[lead] & 0xc0) === 0x80) { + lead--; + } + if (lead < 0) { + break; + } + const b = buffer[lead]; + const width = + (b & 0x80) === 0x00 + ? 1 + : (b & 0xe0) === 0xc0 + ? 2 + : (b & 0xf0) === 0xe0 + ? 3 + : (b & 0xf8) === 0xf0 + ? 4 + : 1; // invalid lead (0xC0/0xC1/0xF8–0xFF): treat as a 1-byte unit + if (lead + width === end) { + break; // a complete, well-formed trailing character + } + end = lead; + } + content = buffer.subarray(0, end).toString('utf8'); + while (Buffer.byteLength(content, 'utf8') > LOOP_TASK_FILE_MAX_BYTES) { + content = content.slice(0, -1); + } + } else { + content = buffer.toString('utf8'); + } + + // The one happy-path trace (all other logs here are skip/failure) so oncall + // can confirm a tick actually picked up a file. Logs the relative source + // label and byte count, never the absolute path (which would leak the OS + // username / dir layout into debug logs). + debugLogger.debug('read loop.md', { + source, + bytes: buffer.byteLength, + truncated, + }); + + return { + status: 'found', + path: filePath, + source, + content, + truncated, + }; + } + + return { + status: 'missing', + checkedPaths: candidates.map((c) => c.path), + }; +} diff --git a/packages/core/src/skills/bundled/loop/loop-tick-resolver.test.ts b/packages/core/src/skills/bundled/loop/loop-tick-resolver.test.ts new file mode 100644 index 00000000000..9a3354d4947 --- /dev/null +++ b/packages/core/src/skills/bundled/loop/loop-tick-resolver.test.ts @@ -0,0 +1,665 @@ +/** + * @license + * Copyright 2026 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + LOOP_SENTINEL_CRON, + LOOP_SENTINEL_DYNAMIC, + LoopTickResolver, + detectLoopSentinel, +} from './loop-tick-resolver.js'; +import { LOOP_TASK_FILE_MAX_BYTES } from './loop-task-file.js'; + +// Make only realpath observable; every other fs call stays real so the temp-dir +// fixtures keep working. The default impl calls through, so behavior is unchanged +// — the spy just lets a test count how often a boundary is re-resolved. +vi.mock('node:fs/promises', async (importActual) => { + const actual = await importActual(); + return { ...actual, realpath: vi.fn(actual.realpath) }; +}); + +describe('detectLoopSentinel', () => { + it('recognizes the cron and dynamic sentinels exactly (after trim)', () => { + expect(detectLoopSentinel(LOOP_SENTINEL_CRON)).toBe('cron'); + expect(detectLoopSentinel(LOOP_SENTINEL_DYNAMIC)).toBe('dynamic'); + expect(detectLoopSentinel(` ${LOOP_SENTINEL_DYNAMIC}\n`)).toBe('dynamic'); + }); + + it('returns null for non-sentinel prompts', () => { + expect(detectLoopSentinel('/loop check the deploy')).toBeNull(); + expect(detectLoopSentinel('<> and more')).toBeNull(); + expect(detectLoopSentinel('')).toBeNull(); + }); +}); + +describe('LoopTickResolver', () => { + let tempDir: string; + let projectRoot: string; + let homeDir: string; + let resolver: LoopTickResolver; + + const projectFile = () => path.join(projectRoot, '.qwen', 'loop.md'); + const homeFile = () => path.join(homeDir, '.qwen', 'loop.md'); + const writeProject = (content: string) => + fs + .mkdir(path.join(projectRoot, '.qwen'), { recursive: true }) + .then(() => fs.writeFile(projectFile(), content)); + const writeHome = (content: string) => + fs + .mkdir(path.join(homeDir, '.qwen'), { recursive: true }) + .then(() => fs.writeFile(homeFile(), content)); + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'loop-tick-')); + projectRoot = path.join(tempDir, 'project'); + homeDir = path.join(tempDir, 'home'); + await fs.mkdir(projectRoot, { recursive: true }); + await fs.mkdir(homeDir, { recursive: true }); + resolver = new LoopTickResolver({ + projectRoot, + homeDir, + allowProjectFile: () => true, + }); + }); + + afterEach(async () => { + // Reset realpath call history (keep the call-through impl) between tests. + vi.mocked(fs.realpath).mockClear(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('ignores the project loop.md in an untrusted folder (allowProjectFile: false)', async () => { + // An untrusted folder's repo-controlled project loop.md must not be read, + // but the user-owned home loop.md still is. + await writeProject('- repo-controlled tasks'); + await writeHome('- user tasks'); + const untrusted = new LoopTickResolver({ + projectRoot, + homeDir, + allowProjectFile: () => false, + }); + + const tick = await untrusted.resolve('cron'); + + expect(tick.full).toBe(true); + expect(tick.sourceLabel).toBe('home loop.md'); + expect(tick.modelText).toContain('- user tasks'); + expect(tick.modelText).not.toContain('- repo-controlled tasks'); + }); + + it('treats a present project loop.md as absent when the folder is untrusted', async () => { + await writeProject('- repo-controlled tasks'); + const untrusted = new LoopTickResolver({ + projectRoot, + homeDir, + allowProjectFile: () => false, + }); + + const tick = await untrusted.resolve('cron'); + + expect(tick.full).toBe(false); + expect(tick.sourceLabel).toBeUndefined(); + // A genuinely-absent tick is NOT flagged transient, so the echo says "not + // present" rather than "temporarily unavailable". + expect(tick.transientError).toBe(false); + expect(tick.modelText).toContain('loop.md is not currently present'); + // The project candidate was never read (untrusted), so the absent message + // must not claim it was checked — only the home candidate is named, via a + // leak-safe label (this fixture's homeDir is a temp dir outside the real + // $HOME, so the absolute path must never reach the model text). + expect(tick.modelText).not.toContain('(project)'); + expect(tick.modelText).toContain('(home)'); + expect(tick.modelText).not.toContain(homeFile()); + }); + + it('re-reads folder trust per tick: a trusted→untrusted flip stops reading the project file', async () => { + // allowProjectFile is a getter, not a snapshot: isTrustedFolder() can flip + // mid-session (IDE workspace-trust update) and the resolver outlives a tick. + // A resolver built while trusted must skip the repo-controlled project + // loop.md on the very next tick once trust flips — not keep reading it. + await writeProject('- repo-controlled tasks'); + let trusted = true; + const flipping = new LoopTickResolver({ + projectRoot, + homeDir, + allowProjectFile: () => trusted, + }); + + const trustedTick = await flipping.resolve('cron'); + expect(trustedTick.full).toBe(true); + expect(trustedTick.sourceLabel).toBe('project loop.md'); + expect(trustedTick.modelText).toContain('- repo-controlled tasks'); + flipping.markDelivered(); + + // Trust revoked. With no user-owned home loop.md, the next tick must be a + // labelled no-op — the project file is no longer read by the SAME resolver. + trusted = false; + const untrustedTick = await flipping.resolve('cron'); + expect(untrustedTick.full).toBe(false); + expect(untrustedTick.sourceLabel).toBeUndefined(); + expect(untrustedTick.modelText).toContain( + 'loop.md is not currently present', + ); + expect(untrustedTick.modelText).not.toContain('- repo-controlled tasks'); + // Trust is revoked, so the project file was not read — don't claim it. + expect(untrustedTick.modelText).not.toContain('(project)'); + }); + + it('delivers the full task block on first fire', async () => { + await writeProject('- ship the thing'); + + const tick = await resolver.resolve('dynamic'); + + expect(tick.full).toBe(true); + // sourceLabel is the relative label, never the absolute path — the model + // text (and this label) must not leak projectFile(). + expect(tick.sourceLabel).toBe('project loop.md'); + expect(tick.modelText).toContain( + '# /loop tick — loop.md tasks from project loop.md', + ); + expect(tick.modelText).not.toContain(projectFile()); + expect(tick.modelText).toContain('The user configured a loop-tasks file.'); + expect(tick.modelText).toContain('- ship the thing'); + // The full block carries the mode-specific pacing suffix (dynamic re-arm)... + expect(tick.modelText).toContain('(dynamic pacing)'); + expect(tick.modelText).toContain('call LoopWakeup again'); + // ...but NOT the "established earlier" reminder: the block is right here in + // this message, so that phrasing would contradict the INTRO above it. + expect(tick.modelText).not.toContain('established earlier'); + // Exactly one H1 in the whole message (no duplicated tick heading). + expect(tick.modelText.match(/^# /gm)).toHaveLength(1); + }); + + it('delivers only the short reminder when content is unchanged', async () => { + await writeProject('- ship the thing'); + await resolver.resolve('dynamic'); + resolver.markDelivered(); + + const tick = await resolver.resolve('dynamic'); + + expect(tick.full).toBe(false); + // The unchanged branch still reports the resolved source so Session.ts can + // label it even when only the short reminder is sent. + expect(tick.sourceLabel).toBe('project loop.md'); + expect(tick.modelText).not.toContain( + 'The user configured a loop-tasks file.', + ); + // A subsequent tick DOES point back to the earlier full block — that + // reminder semantics is intact (only the first delivery omits it). + expect(tick.modelText).toContain('established earlier'); + expect(tick.modelText).toContain( + '# /loop tick — loop.md tasks (dynamic pacing)', + ); + }); + + it('commits content only on markDelivered, so an undelivered tick re-expands', async () => { + await writeProject('- tasks'); + expect((await resolver.resolve('dynamic')).full).toBe(true); + + // No markDelivered() — the block was never delivered (e.g. the tick was + // aborted before the send). The next tick must re-deliver the full block. + expect((await resolver.resolve('dynamic')).full).toBe(true); + + resolver.markDelivered(); + expect((await resolver.resolve('dynamic')).full).toBe(false); + }); + + it('re-delivers the full NEW block when an undelivered tick is followed by an edit', async () => { + // First tick resolved but ABORTED before delivery (no markDelivered), then the + // file is edited. Delivered content (#lastContent) is still null, so the second + // resolve must emit the FULL block with the NEW content — this is the + // #pendingContent-vs-#lastContent divergence path. If #pendingContent were + // committed eagerly on resolve(), the first tick would collapse to a short + // reminder (full=false), pointing the model at a block it never received. + await writeProject('- v1 tasks'); + expect((await resolver.resolve('dynamic')).full).toBe(true); + // No markDelivered() — the first tick never reached the model. + + await writeProject('- v2 edited tasks'); + const tick = await resolver.resolve('dynamic'); + + expect(tick.full).toBe(true); + expect(tick.modelText).toContain('The user configured a loop-tasks file.'); + expect(tick.modelText).toContain('- v2 edited tasks'); + }); + + it('re-delivers the full block when loop.md is edited', async () => { + await writeProject('- v1'); + await resolver.resolve('dynamic'); + resolver.markDelivered(); + + await writeProject('- v2 edited'); + const tick = await resolver.resolve('dynamic'); + + expect(tick.full).toBe(true); + expect(tick.modelText).toContain('- v2 edited'); + }); + + it('re-delivers the full block after resetCache (compaction)', async () => { + await writeProject('- stable'); + await resolver.resolve('dynamic'); + resolver.markDelivered(); + expect((await resolver.resolve('dynamic')).full).toBe(false); + + resolver.resetCache(); + const tick = await resolver.resolve('dynamic'); + + expect(tick.full).toBe(true); + expect(tick.modelText).toContain('- stable'); + }); + + it('clears the boundary realpath cache on resetCache so it is re-resolved', async () => { + // The fs.realpath of the confinement boundary (projectRoot) is cached per + // resolver for the per-tick perf win. resetCache must invalidate it too — + // otherwise a long-lived process keeps a stale boundary after a /cd or symlink + // re-point. Prove projectRoot is re-resolved only after a reset. + await writeProject('- tasks'); + const rootResolves = () => + vi + .mocked(fs.realpath) + .mock.calls.filter((c) => String(c[0]) === projectRoot).length; + + await resolver.resolve('cron'); + expect(rootResolves()).toBe(1); + await resolver.resolve('cron'); + expect(rootResolves()).toBe(1); // served from the instance cache, not re-resolved + + resolver.resetCache(); + await resolver.resolve('cron'); + expect(rootResolves()).toBe(2); // cache cleared → boundary re-resolved + }); + + it('emits the absent reminder without poisoning the cache, then re-expands on recreate', async () => { + const absent = await resolver.resolve('dynamic'); + expect(absent.full).toBe(false); + expect(absent.sourceLabel).toBeUndefined(); + expect(absent.modelText).toContain('loop.md is not currently present'); + + await writeProject('- recreated tasks'); + const tick = await resolver.resolve('dynamic'); + + expect(tick.full).toBe(true); + expect(tick.modelText).toContain('- recreated tasks'); + }); + + it('gives the absent tick the same shared heading style (and dynamic suffix)', async () => { + const cron = await resolver.resolve('cron'); + expect(cron.modelText).toContain('# /loop tick — loop.md absent\n'); + + const dyn = new LoopTickResolver({ + projectRoot, + homeDir, + allowProjectFile: () => true, + }); + const dynTick = await dyn.resolve('dynamic'); + expect(dynTick.modelText).toContain( + '# /loop tick — loop.md absent (dynamic pacing)\n', + ); + // The absent dynamic tail names the re-arm sentinel by interpolating the + // constant — asserting against LOOP_SENTINEL_DYNAMIC catches a future rename + // drift between the constant and the user-facing instruction. + expect(dynTick.modelText).toContain(LOOP_SENTINEL_DYNAMIC); + // Exactly one H1 — the heading isn't duplicated by the body. + expect(dynTick.modelText.match(/^# /gm)).toHaveLength(1); + }); + + it('resolve() honors an explicit allowProjectFile override over the getter', async () => { + // FIX 3: the caller captures folder-trust ONCE per tick and threads it in, + // so the per-tick getter is bypassed. Build a resolver whose getter would + // ALLOW the project file, but pass `false`: the repo-controlled project + // loop.md must be skipped on this tick, exactly as the getter-false path. + await writeProject('- repo-controlled tasks'); + const threaded = new LoopTickResolver({ + projectRoot, + homeDir, + allowProjectFile: () => true, // getter would allow... + }); + + const tick = await threaded.resolve('cron', false); // ...override forbids + + expect(tick.full).toBe(false); + expect(tick.modelText).not.toContain('- repo-controlled tasks'); + expect(tick.modelText).not.toContain('(project)'); + expect(tick.modelText).toContain('(home)'); + }); + + it('buildTransientErrorTick mirrors the absent tick with a re-arm and errno note', () => { + // FIX 4: a transient, non-whitelisted read error must NOT kill a dynamic + // loop. The degraded tick mirrors the absent path's re-arm + cache-clear, plus + // a note that the file was unreadable this tick, so the model still re-arms + // LoopWakeup and the loop survives. + const tick = resolver.buildTransientErrorTick('dynamic', true, 'EIO'); + + expect(tick.full).toBe(false); + // Flagged transient (file present, unreadable this tick) so the caller's echo + // can say "temporarily unavailable" rather than the genuinely-absent label. + expect(tick.transientError).toBe(true); + // The heading says "unavailable", NOT "absent"/"not present": the file exists, + // it just couldn't be read this tick, so the heading must mirror the body. + // Mutation guard: revert the heading to { absent: true } and these fail. + expect(tick.modelText).toContain( + '# /loop tick — loop.md unavailable (dynamic pacing)\n', + ); + expect(tick.modelText).not.toContain('absent'); + expect(tick.modelText).not.toContain('not present'); + expect(tick.modelText).toContain('could not be read this tick (EIO)'); + // The dynamic re-arm instruction (the literal sentinel) keeps the loop alive. + expect(tick.modelText).toContain(LOOP_SENTINEL_DYNAMIC); + // projectChecked=true names BOTH candidates (the set that was probed). + expect(tick.modelText).toContain('(project)'); + expect(tick.modelText).toContain('(home)'); + }); + + it('cron buildTransientErrorTick uses the cron tail and omits an unprobed project', () => { + // cron mode degrades only via its own next interval, but the tick text still + // uses the cron no-op tail (no LoopWakeup re-arm). With projectChecked=false + // (untrusted) the never-probed project candidate must NOT be named. + const tick = resolver.buildTransientErrorTick('cron', false, 'EACCES'); + + // Heading conveys "unavailable" (file exists, unreadable this tick), never + // the misleading "absent". + expect(tick.modelText).toContain('# /loop tick — loop.md unavailable'); + expect(tick.modelText).not.toContain('absent'); + expect(tick.modelText).toContain('could not be read this tick (EACCES)'); + expect(tick.modelText).toContain('the recurring cron fires the next tick'); + expect(tick.modelText).not.toContain(LOOP_SENTINEL_DYNAMIC); + expect(tick.modelText).not.toContain('(project)'); + expect(tick.modelText).toContain('(home)'); + }); + + it('a transient-error tick clears the change-detection cache so the next read re-delivers full', async () => { + // The degraded tick must behave like absent for caching: after it, a read of + // byte-identical content re-expands the FULL block rather than a dangling + // short reminder pointing at a block no longer guaranteed to be in context. + // Mutation guard: if buildTransientErrorTick doesn't clear the caches, the + // second resolve sees "unchanged" and returns a short reminder (full:false). + await writeProject('- tasks'); + const full = await resolver.resolve('dynamic'); + expect(full.full).toBe(true); + resolver.markDelivered(); + + resolver.buildTransientErrorTick('dynamic', true, 'EIO'); + + const next = await resolver.resolve('dynamic'); + expect(next.full).toBe(true); + }); + + it('names the real home loop.md in the absent reminder (QWEN_HOME-aware, not a hardcoded ~/.qwen)', async () => { + // Regression: the absent body hardcoded `~/.qwen/loop.md (home)`, which is + // wrong once the global dir is relocated (QWEN_HOME). The resolver checks + // `/loop.md`, but the label is MODEL-FACING, so a $QWEN_HOME + // outside $HOME (tildeifyPath no-op there) must read as the literal + // `$QWEN_HOME/loop.md`, never the raw absolute path it would otherwise leak. + const relocated = path.join(tempDir, 'relocated-qwen'); + const prevQwenHome = process.env['QWEN_HOME']; + process.env['QWEN_HOME'] = relocated; + try { + const relocatedTick = await new LoopTickResolver({ + projectRoot, + homeDir: relocated, + homeQwenDir: relocated, + allowProjectFile: () => true, + }).resolve('cron'); + + expect(relocatedTick.full).toBe(false); + expect(relocatedTick.modelText).toContain( + 'loop.md is not currently present', + ); + expect(relocatedTick.modelText).toContain('$QWEN_HOME/loop.md (home)'); + // The old hardcoded home location is gone; the project label stays relative. + expect(relocatedTick.modelText).not.toContain('~/.qwen/loop.md'); + expect(relocatedTick.modelText).toContain('.qwen/loop.md (project)'); + // Privacy: the raw absolute global dir never reaches the model text. + expect(relocatedTick.modelText).not.toContain(relocated); + } finally { + if (prevQwenHome === undefined) delete process.env['QWEN_HOME']; + else process.env['QWEN_HOME'] = prevQwenHome; + } + + // Under the real OS home (the QWEN_HOME-unset case) the home prefix tilde- + // abbreviates, so the message reads `~/…/loop.md`, never the absolute $HOME. + const underHome = path.join( + os.homedir(), + `.qwen-loop-absent-${process.pid}`, + ); + const homeTick = await new LoopTickResolver({ + projectRoot, + homeDir: os.homedir(), + homeQwenDir: underHome, + allowProjectFile: () => true, + }).resolve('dynamic'); + + expect(homeTick.modelText).toContain( + `~/${path.basename(underHome)}/loop.md (home)`, + ); + expect(homeTick.modelText).not.toContain(os.homedir()); + }); + + it('homeLoopLabel never leaks an absolute $QWEN_HOME path outside $HOME (privacy)', async () => { + // The label is sent to the model/API. $QWEN_HOME may point OUTSIDE $HOME + // (supported relocation; common in containers/CI), where tildeifyPath is a + // no-op — so the resolved absolute dir must be swapped for the literal + // `$QWEN_HOME`. Mutation guard: revert homeLoopLabel to + // `tildeifyPath(join(homeQwenDir,'loop.md'))` and `outside` (the absolute + // path) reappears in BOTH assertions below, failing this test. + const outside = path.join(tempDir, 'srv-qwen-home'); + const prevQwenHome = process.env['QWEN_HOME']; + process.env['QWEN_HOME'] = outside; + try { + const relocated = new LoopTickResolver({ + projectRoot, + homeDir: outside, + homeQwenDir: outside, + allowProjectFile: () => true, + }); + expect(relocated.homeLoopLabel()).toBe('$QWEN_HOME/loop.md'); + const tick = await relocated.resolve('cron'); + expect(tick.modelText).toContain('$QWEN_HOME/loop.md (home)'); + expect(tick.modelText).not.toContain(outside); + + // Defensive case: an out-of-$HOME global dir with $QWEN_HOME UNSET still + // never surfaces the absolute path — a generic placeholder is used. + delete process.env['QWEN_HOME']; + const generic = new LoopTickResolver({ + projectRoot, + homeDir: outside, + homeQwenDir: outside, + allowProjectFile: () => true, + }); + expect(generic.homeLoopLabel()).toBe('the configured global loop.md'); + expect(generic.homeLoopLabel()).not.toContain(outside); + } finally { + if (prevQwenHome === undefined) delete process.env['QWEN_HOME']; + else process.env['QWEN_HOME'] = prevQwenHome; + } + }); + + it('homeLoopLabel keeps the separator when $QWEN_HOME has a trailing slash', async () => { + // Storage.getGlobalQwenDir() does NOT strip a trailing slash, so a + // `QWEN_HOME=/srv/qwen/` reaches homeQwenDir as `/srv/qwen/`. Slicing the + // joined loop.md path by the raw homeQwenDir length over-counts the trailing + // separator and garbles the label into `$QWEN_HOMEloop.md`. Mutation guard: + // revert the slice base to `homeQwenDir.length` and the first assertion below + // fails with the separator-less `$QWEN_HOMEloop.md`. + const outsideTrailing = path.join(tempDir, 'srv-qwen-home') + path.sep; + const prevQwenHome = process.env['QWEN_HOME']; + process.env['QWEN_HOME'] = outsideTrailing; + try { + const trailing = new LoopTickResolver({ + projectRoot, + homeDir: outsideTrailing, + homeQwenDir: outsideTrailing, + allowProjectFile: () => true, + }); + expect(trailing.homeLoopLabel()).toBe('$QWEN_HOME/loop.md'); + // Never the raw absolute dir, and never the garbled separator-less form. + expect(trailing.homeLoopLabel()).not.toContain(outsideTrailing); + expect(trailing.homeLoopLabel()).not.toContain('$QWEN_HOMEloop.md'); + + // out-of-$HOME branch still behaves with QWEN_HOME UNSET: generic placeholder. + delete process.env['QWEN_HOME']; + const generic = new LoopTickResolver({ + projectRoot, + homeDir: outsideTrailing, + homeQwenDir: outsideTrailing, + allowProjectFile: () => true, + }); + expect(generic.homeLoopLabel()).toBe('the configured global loop.md'); + } finally { + if (prevQwenHome === undefined) delete process.env['QWEN_HOME']; + else process.env['QWEN_HOME'] = prevQwenHome; + } + + // under-$HOME branch still behaves with a trailing slash: tilde-abbreviated. + const underHomeTrailing = + path.join(os.homedir(), `.qwen-loop-trailing-${process.pid}`) + path.sep; + const underHome = new LoopTickResolver({ + projectRoot, + homeDir: os.homedir(), + homeQwenDir: underHomeTrailing, + allowProjectFile: () => true, + }); + expect(underHome.homeLoopLabel()).toBe( + `~/.qwen-loop-trailing-${process.pid}/loop.md`, + ); + expect(underHome.homeLoopLabel()).not.toContain(os.homedir()); + }); + + it('homeLoopLabel keeps the separator when $QWEN_HOME is the filesystem root', async () => { + // `QWEN_HOME=/` makes homeQwenDir the root, so homeLoopPath is + // path.join('/', 'loop.md') = '/loop.md', whose path.dirname is '/' (length 1). + // Slicing the joined path past that length drops the leading separator, + // garbling the label into the separator-less `$QWEN_HOMEloop.md`. Mutation + // guard: revert homeLoopLabel to the slice-by-dirname-length approach and the + // first assertion below fails with `$QWEN_HOMEloop.md`. + const root = path.sep; // the filesystem root ('/' on POSIX) + const prevQwenHome = process.env['QWEN_HOME']; + process.env['QWEN_HOME'] = root; + try { + const atRoot = new LoopTickResolver({ + projectRoot, + homeDir: root, + homeQwenDir: root, + allowProjectFile: () => true, + }); + expect(atRoot.homeLoopLabel()).toBe(`$QWEN_HOME${path.sep}loop.md`); + // The garbled, separator-less form must never appear. + expect(atRoot.homeLoopLabel()).not.toContain('$QWEN_HOMEloop.md'); + } finally { + if (prevQwenHome === undefined) delete process.env['QWEN_HOME']; + else process.env['QWEN_HOME'] = prevQwenHome; + } + }); + + it('re-expands after delete→recreate even when the recreated content is identical', async () => { + await writeProject('- same tasks'); + expect((await resolver.resolve('dynamic')).full).toBe(true); + resolver.markDelivered(); + // Unchanged content → short reminder, as expected. + expect((await resolver.resolve('dynamic')).full).toBe(false); + + // Delete → the absent tick clears the delivered-content memory. + await fs.rm(projectFile()); + const absent = await resolver.resolve('dynamic'); + expect(absent.full).toBe(false); + expect(absent.modelText).toContain('loop.md is not currently present'); + + // Recreate with byte-identical content. Absence was a state change, so the + // full block must re-expand rather than collapse to a dangling reminder. + await writeProject('- same tasks'); + const tick = await resolver.resolve('dynamic'); + expect(tick.full).toBe(true); + expect(tick.modelText).toContain('- same tasks'); + }); + + it('uses mode-specific reminders; dynamic names the re-arm sentinel', async () => { + await writeProject('- tasks'); + + const cron = await resolver.resolve('cron'); + expect(cron.modelText).toContain('do not call LoopWakeup from this tick'); + expect(cron.modelText).not.toContain('(dynamic pacing)'); + + // Fresh resolver so 'dynamic' is also a first (full) delivery. + const dyn = new LoopTickResolver({ + projectRoot, + homeDir, + allowProjectFile: () => true, + }); + const dynTick = await dyn.resolve('dynamic'); + expect(dynTick.modelText).toContain(LOOP_SENTINEL_DYNAMIC); + expect(dynTick.modelText).toContain('call LoopWakeup again'); + }); + + it('appends the truncation warning on a line boundary for oversized files', async () => { + const line = 'task line padding padding padding\n'; + const body = line.repeat(Math.ceil(LOOP_TASK_FILE_MAX_BYTES / line.length)); + await writeProject(body); + + const tick = await resolver.resolve('cron'); + + expect(tick.full).toBe(true); + const warning = `> WARNING: loop.md was truncated to ${LOOP_TASK_FILE_MAX_BYTES} bytes. Keep the task list concise.`; + expect(tick.modelText).toContain(`\n${warning}`); + // The body is trimmed back to a COMPLETE line — the warning never glues onto + // a half-line. Guards against cutToLastNewline regressing to a no-op (which + // would leave the body ending mid-line, e.g. "task line "). + const beforeWarning = tick.modelText.slice( + 0, + tick.modelText.indexOf(`\n${warning}`), + ); + expect(beforeWarning.endsWith('task line padding padding padding')).toBe( + true, + ); + }); + + it('keeps the body when the only newline is at index 0 (no empty truncated block)', async () => { + // A truncated file whose only newline is the leading byte: there is no + // complete line to keep, so cutting to the "last full line" would empty the + // body and leave the INTRO promising tasks that aren't there. The body must + // survive — guards cutToLastNewline against a `cut >= 0` regression that + // slices a position-0 newline down to "". + await writeProject('\n' + 'x'.repeat(LOOP_TASK_FILE_MAX_BYTES + 100)); + + const tick = await resolver.resolve('cron'); + + expect(tick.full).toBe(true); + const warning = `> WARNING: loop.md was truncated to ${LOOP_TASK_FILE_MAX_BYTES} bytes. Keep the task list concise.`; + expect(tick.modelText).toContain(`\n${warning}`); + // The x-run above the warning is non-empty; a `cut >= 0` regression would + // empty it, leaving only INTRO + warning. + const beforeWarning = tick.modelText.slice( + 0, + tick.modelText.indexOf(`\n${warning}`), + ); + expect(beforeWarning).toContain('xxxxxxxxxx'); + }); + + it('names the home loop.md in the header and re-expands when the source switches', async () => { + await writeProject('- project tasks'); + const first = await resolver.resolve('cron'); + resolver.markDelivered(); + expect(first.full).toBe(true); + expect(first.sourceLabel).toBe('project loop.md'); + + // Project gone, home has DIFFERENT content → re-expand (cache keys on + // content, not path) and the header now names the home file. + await fs.rm(projectFile()); + await writeHome('- home tasks'); + const second = await resolver.resolve('cron'); + + expect(second.full).toBe(true); + expect(second.sourceLabel).toBe('home loop.md'); + expect(second.modelText).toContain( + '# /loop tick — loop.md tasks from home loop.md', + ); + // The absolute home path must not leak into the model-facing text. + expect(second.modelText).not.toContain(homeFile()); + expect(second.modelText).toContain('- home tasks'); + }); +}); diff --git a/packages/core/src/skills/bundled/loop/loop-tick-resolver.ts b/packages/core/src/skills/bundled/loop/loop-tick-resolver.ts new file mode 100644 index 00000000000..28f64469236 --- /dev/null +++ b/packages/core/src/skills/bundled/loop/loop-tick-resolver.ts @@ -0,0 +1,347 @@ +/** + * @license + * Copyright 2026 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as path from 'node:path'; +import { tildeifyPath } from '../../../utils/paths.js'; +import { + LOOP_TASK_FILE_MAX_BYTES, + readLoopTaskFile, + type LoopTaskFileSource, +} from './loop-task-file.js'; + +/** + * Fire-time resolver for `.qwen/loop.md`-driven loops. + * + * A `/loop` whose scheduled prompt is one of these sentinels re-reads loop.md + * on every fire and gets either the FULL task block (first delivery, or whenever + * the file changed) or a one-line SHORT reminder (unchanged) — so the task list + * is paid for once into the cached message-prefix and later ticks stay cheap. + * + * Divergence from the upstream design this mirrors: the `lastContent` cache is + * held per Session instance (not a module singleton) so it scopes to one + * conversation and resets cleanly with that conversation's context (compaction). + * Change-detection is full content equality, not mtime/hash, so edit and + * delete→recreate both re-expand for free. + */ + +export const LOOP_SENTINEL_CRON = '<>'; +export const LOOP_SENTINEL_DYNAMIC = '<>'; + +export type LoopMode = 'cron' | 'dynamic'; + +export interface LoopTickResolverDeps { + /** Pass `config.getWorkingDir()` — loop.md is resolved against the cwd. */ + projectRoot: string; + /** Home-candidate confinement root: `$QWEN_HOME` when set, else `$HOME`. */ + homeDir: string; + /** + * QWEN_HOME-aware global dir holding the home `loop.md` (`Storage.getGlobalQwenDir()`). + * Omitted → defaults to `/.qwen` inside readLoopTaskFile. + */ + homeQwenDir?: string; + /** + * Pass `() => config.isTrustedFolder()`. Re-evaluated on every `resolve()`, + * never captured once: `isTrustedFolder()` is not process-stable in IDE + * sessions (a workspace-trust update can flip it), and a trusted→untrusted + * flip must immediately stop reading the repo-controlled project + * `.qwen/loop.md` (the user-owned `~/.qwen/loop.md` still is read). + */ + allowProjectFile: () => boolean; +} + +export interface LoopTickResult { + /** Text to deliver to the model in place of the sentinel prompt. */ + modelText: string; + /** True when the full task block was delivered (vs a short reminder). */ + full: boolean; + /** Non-absolute label for the matched candidate (e.g. "project loop.md"), + * when present — safe for logs/UI that must not leak the absolute path, and + * doubles as the "a loop.md was found" flag for callers. */ + sourceLabel?: string; + /** True ONLY for buildTransientErrorTick: a loop.md exists but could not be + * read THIS tick (a transient EACCES/EIO or editor/AV lock), as distinct from + * the genuinely-absent no-op (where this stays false). Lets the caller's echo + * say "temporarily unavailable" instead of "not present". Carries no errno or + * path — those stay in the modelText note and LOCAL debug logs only. */ + transientError?: boolean; +} + +const TRUNCATION_WARNING = `> WARNING: loop.md was truncated to ${LOOP_TASK_FILE_MAX_BYTES} bytes. Keep the task list concise.`; + +const INTRO = + 'The user configured a loop-tasks file. Work through the tasks defined below; these are the instructions for this tick and every subsequent tick (the reminder on later fires refers back to this message).'; + +// Mode-specific pacing guidance. Appended to BOTH the full block and the short +// reminder — the no-op/re-arm instruction applies on every tick. +const PACING_SUFFIX: Record = { + cron: 'The recurring cron fires the next tick automatically — do not call LoopWakeup from this tick.', + dynamic: `You scheduled this tick via LoopWakeup (not a recurring cron). To keep the loop alive, call LoopWakeup again at the end of this turn with prompt set to the literal sentinel \`${LOOP_SENTINEL_DYNAMIC}\` — otherwise the loop ends after this tick.`, +}; + +// Preamble for the UNCHANGED-tick reminder, which points back to the full block +// delivered on an earlier fire. NOT used on the first/changed full delivery, +// where the block is present in THIS message — there is no "earlier" to refer +// back to, so claiming the contents were established earlier would contradict +// the INTRO that sits right above them. +const SHORT_REMINDER_PREAMBLE = + 'Work the tasks from the loop.md contents established earlier in this conversation. If you cannot find them, treat this as a no-op tick.'; + +/** + * The single H1 for every tick variant (full block, short reminder, absent), so + * they share one heading style and the dynamic-pacing suffix lives in one place. + * `sourceLabel` (set only on a full-block delivery) is a relative label like + * "project loop.md", never the absolute path — so the resolved file location + * isn't leaked to the model/API provider. + */ +function tickHeading( + mode: LoopMode, + opts: { sourceLabel?: string; absent?: boolean; unavailable?: boolean } = {}, +): string { + // `unavailable` (transient read failure) is distinct from `absent`: the file + // exists but couldn't be read THIS tick, so the heading must not claim it's gone. + const subject = opts.unavailable + ? 'loop.md unavailable' + : opts.absent + ? 'loop.md absent' + : opts.sourceLabel + ? `loop.md tasks from ${opts.sourceLabel}` + : 'loop.md tasks'; + const base = `# /loop tick — ${subject}`; + return mode === 'dynamic' ? `${base} (dynamic pacing)` : base; +} + +/** Model-safe relative label per source — exhaustive, so a new loop.md + * candidate added to readLoopTaskFile won't compile until it gets a label + * (rather than silently mislabelling it). */ +const SOURCE_LABELS: Record = { + project: 'project loop.md', + home: 'home loop.md', +}; + +// Per-mode tail of the absent reminder. The shared prefix (built in absentBody) +// names the candidate location(s) actually checked; only this no-op/re-arm +// guidance differs by mode. +const ABSENT_TAIL: Record = { + cron: 'Treat this as a no-op tick; the recurring cron fires the next tick automatically.', + dynamic: `Treat this as a no-op tick. To pick it up if it is recreated, call LoopWakeup again with prompt set to the literal sentinel \`${LOOP_SENTINEL_DYNAMIC}\` — otherwise the loop ends after this tick.`, +}; + +// Body of the absent reminder — the H1 is supplied by tickHeading() so the +// absent tick shares the same heading style as the full block and reminder. +// `locations` is LoopTickResolver.absentLocations(): the candidate path(s) +// ACTUALLY checked this tick (the project candidate is omitted on an untrusted +// folder), with a QWEN_HOME-aware home label that is never a raw absolute path. +function absentBody(mode: LoopMode, locations: string): string { + return `loop.md is not currently present at ${locations}. ${ABSENT_TAIL[mode]}`; +} + +/** Detect whether a scheduled prompt is a loop.md sentinel, and which mode. */ +export function detectLoopSentinel(prompt: string): LoopMode | null { + const trimmed = prompt.trim(); + if (trimmed === LOOP_SENTINEL_DYNAMIC) { + return 'dynamic'; + } + if (trimmed === LOOP_SENTINEL_CRON) { + return 'cron'; + } + return null; +} + +/** Trim a truncated body back to its last full line before the warning tail. */ +function cutToLastNewline(content: string): string { + const cut = content.lastIndexOf('\n'); + // `> 0`, not `>= 0`: when the only newline is at index 0 (or there is none), + // there is no complete line to keep, so cutting would empty the body and leave + // the INTRO promising tasks that aren't there. Keep the (truncated) content + // instead — only a genuine trailing partial line (newline at index > 0) is + // dropped so the warning never glues onto a half-line. + return cut > 0 ? content.slice(0, cut) : content; +} + +export class LoopTickResolver { + // What the model has actually received. Drives full-vs-reminder detection. + #lastContent: string | null = null; + // The most recent resolve()'s content, committed to #lastContent only once + // the caller confirms it reached the model (markDelivered) — so a tick that + // is aborted between resolve() and delivery can't poison the cache into + // sending a dangling short reminder next time. + #pendingContent: string | null = null; + // Instance-scoped fs.realpath cache for the confinement boundaries, handed to + // readLoopTaskFile. Tying it to the resolver (a fresh Map per /cd rebuild, + // cleared by resetCache) keeps the per-tick perf win while staying + // invalidatable — a module-global cache would pin a stale boundary in a + // long-lived process after a /cd or symlink re-point. + readonly #realDirCache = new Map>(); + + constructor(private readonly deps: LoopTickResolverDeps) {} + + /** Forget the delivered content so the next fire re-delivers the full block + * — called when the conversation is compacted (fresh context). */ + resetCache(): void { + this.#lastContent = null; + this.#pendingContent = null; + // A reset may follow a /cd or symlink change, so drop the cached boundary + // realpaths too and re-resolve them on the next tick. + this.#realDirCache.clear(); + } + + /** Commit the last resolve()'s content once it has reached the model. */ + markDelivered(): void { + if (this.#pendingContent !== null) { + this.#lastContent = this.#pendingContent; + } + } + + /** MODEL-FACING label for the home loop.md location. Mirrors + * readLoopTaskFile's home candidate (`/loop.md`) so the absent + * reminder — and the caller's sanitized resolve-error — names the location + * actually checked (QWEN_HOME-aware), but must NEVER surface a raw absolute + * path: it flows into model/API text, leaking the host's filesystem layout. + * - under $HOME → tilde-abbreviated `~/.qwen/loop.md`; + * - relocated via $QWEN_HOME → the literal `$QWEN_HOME/loop.md`, not the + * resolved dir (`tildeifyPath` only abbreviates $HOME, so it's a no-op for + * a $QWEN_HOME outside $HOME and would otherwise pass the path through); + * - any other out-of-$HOME dir → a generic placeholder, never the path. + * The real absolute path stays in LOCAL debug logs only. */ + homeLoopLabel(): string { + const homeQwenDir = + this.deps.homeQwenDir ?? path.join(this.deps.homeDir, '.qwen'); + const homeLoopPath = path.join(homeQwenDir, 'loop.md'); + + const tildeified = tildeifyPath(homeLoopPath); + if (tildeified !== homeLoopPath) { + return tildeified; + } + // Outside $HOME: tildeifyPath was a no-op. When $QWEN_HOME relocated the + // global dir (homeQwenDir is its resolved value), report the literal env-var + // name — never the absolute path. The home candidate is always + // `/loop.md`, so swap the whole resolved dir for `$QWEN_HOME` and + // re-attach the separator + basename directly. Deriving the tail from the + // resolved path's length instead mishandles edge dirs: a trailing slash + // (`$QWEN_HOME=/x/.qwen/`) over-counts the separator, and a filesystem-root + // homeQwenDir (`$QWEN_HOME=/` → homeLoopPath `/loop.md`, dirname `/`) drops the + // leading separator — both garbling the tail into `$QWEN_HOMEloop.md`. + if (process.env['QWEN_HOME']) { + return `$QWEN_HOME${path.sep}loop.md`; + } + return 'the configured global loop.md'; + } + + /** The checked-candidate "where" string shared by the absent reminder and the + * caller's sanitized resolve-error. Names the project candidate ONLY when it + * was actually read (`projectChecked` — a trusted folder), so neither path can + * claim `.qwen/loop.md (project)` for an untrusted folder where the project + * file is skipped. The home label is the QWEN_HOME-aware, never-absolute + * homeLoopLabel(). Single source of truth so the two messages can't drift. */ + absentLocations(projectChecked: boolean): string { + const homeLabel = this.homeLoopLabel(); + return projectChecked + ? `.qwen/loop.md (project) or ${homeLabel} (home)` + : `${homeLabel} (home)`; + } + + /** A model-facing no-op tick (loop.md absent, or unreadable this tick). Clears + * the change-detection caches so a later successful tick re-delivers the FULL + * block instead of a dangling short reminder pointing at a block no longer + * guaranteed to be in context — absence (and a failed read) is itself a state + * change. */ + #noOpTick(modelText: string, transientError = false): LoopTickResult { + this.#pendingContent = null; + this.#lastContent = null; + return { modelText, full: false, transientError }; + } + + /** + * No-op tick for a transient, non-whitelisted read error (EACCES/EIO, or a + * Windows editor/AV briefly locking loop.md). Mirrors the absent tick — same + * heading + the mode's re-arm tail (ABSENT_TAIL) — so a `dynamic` loop still + * re-arms LoopWakeup and survives the hiccup instead of dying silently: its + * firing wakeup was already consumed by the scheduler, and only the + * end-of-turn re-arm keeps it alive, so a thrown turn ends the loop forever. + * `cron` callers don't use this (they re-fire on their own next interval). + * `projectChecked` is the trust captured for THIS tick (so the named candidate + * set matches what was probed); `code` is the errno only — never an absolute + * path — for a brief model-facing note. + */ + buildTransientErrorTick( + mode: LoopMode, + projectChecked: boolean, + code: string, + ): LoopTickResult { + return this.#noOpTick( + // `unavailable`, not `absent`: the file exists but was unreadable this tick, + // so the heading mirrors the body instead of contradicting it. + `${tickHeading(mode, { unavailable: true })}\nloop.md at ${this.absentLocations( + projectChecked, + )} could not be read this tick (${code}). ${ABSENT_TAIL[mode]}`, + // Flag the tick as a transient read failure (file exists, unreadable this + // tick) so the caller's echo distinguishes it from a genuinely-absent file. + true, + ); + } + + /** + * @param allowProjectFileOverride Trust captured once by the caller for this + * tick (see LoopTickResolverDeps.allowProjectFile). Threaded in — rather than + * re-reading the getter here — so the caller's error path can name the SAME + * candidate set that was probed even if `isTrustedFolder()` flips mid-tick. + * Omitted by direct callers, who fall back to the per-tick getter. + */ + async resolve( + mode: LoopMode, + allowProjectFileOverride?: boolean, + ): Promise { + // Re-read trust per tick (see LoopTickResolverDeps.allowProjectFile): a + // resolver built while trusted must skip the project file once trust flips. + // Captured so the absent reminder reflects what was ACTUALLY checked. + const allowProjectFile = + allowProjectFileOverride ?? this.deps.allowProjectFile(); + const result = await readLoopTaskFile({ + projectRoot: this.deps.projectRoot, + homeDir: this.deps.homeDir, + homeQwenDir: this.deps.homeQwenDir, + allowProjectFile, + realDirCache: this.#realDirCache, + }); + + if (result.status === 'missing') { + // Absence is itself a state change: #noOpTick clears both caches so a + // later recreate — even with byte-identical content — re-expands the full + // block rather than sending a dangling short reminder. + return this.#noOpTick( + `${tickHeading(mode, { absent: true })}\n${absentBody(mode, this.absentLocations(allowProjectFile))}`, + ); + } + + const content = result.truncated + ? `${cutToLastNewline(result.content)}\n${TRUNCATION_WARNING}` + : result.content; + this.#pendingContent = content; + + // Label by which candidate matched, never result.path (the absolute path), + // which would leak the OS username / dir layout to the API provider and to + // debug logs. The label alone is enough for the caller's UI and presence + // check, so the absolute path is not surfaced on the result at all. + const sourceLabel = SOURCE_LABELS[result.source]; + + if (this.#lastContent === content) { + return { + modelText: `${tickHeading(mode)}\n${SHORT_REMINDER_PREAMBLE} ${PACING_SUFFIX[mode]}`, + full: false, + sourceLabel, + }; + } + + // First/changed full delivery: INTRO + the block itself, then only the + // pacing suffix — no "established earlier" preamble, which would contradict + // the block sitting right here in this same message. + return { + modelText: `${tickHeading(mode, { sourceLabel })}\n${INTRO}\n${content}\n${PACING_SUFFIX[mode]}`, + full: true, + sourceLabel, + }; + } +}