diff --git a/.qwen/skills/autofix/SKILL.md b/.qwen/skills/autofix/SKILL.md index aed53067736..3477534e6f8 100644 --- a/.qwen/skills/autofix/SKILL.md +++ b/.qwen/skills/autofix/SKILL.md @@ -68,6 +68,7 @@ owns the model-driven decisions, code changes, and pre-commit verification. Keep `failure.md` and `handoff.md` English-only WITHOUT a details block: handoff comments embed a byte-truncated excerpt of them, and a severed `
` tag would swallow the rest of the comment when rendered. + - Never ask the user a question in this headless workflow. If blocked, write `/failure.md` with what you learned and stop. diff --git a/integration-tests/cli/qwen-serve-routes.test.ts b/integration-tests/cli/qwen-serve-routes.test.ts index 5acc973f34b..75c01bd46c7 100644 --- a/integration-tests/cli/qwen-serve-routes.test.ts +++ b/integration-tests/cli/qwen-serve-routes.test.ts @@ -743,6 +743,46 @@ describe('qwen serve — cancel + list', () => { }); }); +describe('qwen serve — GET /goals', () => { + const getGoals = async () => { + const res = await fetch(`${base}/goals`, { + headers: { Authorization: `Bearer ${TOKEN}` }, + }); + return { status: res.status, body: await res.json() }; + }; + + it('returns an empty, versioned list when no session has a goal', async () => { + const { status, body } = await getGoals(); + expect(status).toBe(200); + expect(body).toEqual({ v: 1, goals: [], droppedCount: 0 }); + }); + + it('probes each live session over the bridge without reporting a goal', async () => { + // The real round trip: serve -> bridge -> `sessionGoalGet` ext method in + // the `qwen --acp` child -> back. A live session with no `/goal` must come + // back as "no goal" rather than an error or a phantom entry. + const session = await client.createOrAttachSession({ + workspaceCwd: REPO_ROOT, + sessionScope: 'thread', + }); + try { + const { status, body } = await getGoals(); + expect(status).toBe(200); + // `droppedCount: 0` is the load-bearing half: it proves the ext-method + // probe actually reached the child. A dropped probe would also yield an + // empty `goals`, so that alone cannot tell success from a silent failure. + expect(body).toEqual({ v: 1, goals: [], droppedCount: 0 }); + } finally { + await client.closeSession(session.sessionId); + } + }); + + it('requires the bearer token', async () => { + const res = await fetch(`${base}/goals`); + expect(res.status).toBe(401); + }); +}); + describe('qwen serve — DELETE /session/:id', () => { it('204 on explicit close', async () => { const session = await client.createOrAttachSession({ diff --git a/package-lock.json b/package-lock.json index 6a9d16c902b..6fc4f1e734d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28100,7 +28100,7 @@ "fzf": "^0.5.2", "glob": "^10.5.0", "highlight.js": "^11.11.1", - "ink": "7.0.3", + "ink": "^7.0.3", "ink-gradient": "^3.0.0", "ink-link": "^4.1.0", "ink-spinner": "^5.0.0", @@ -28120,8 +28120,8 @@ "update-notifier": "^7.3.1", "wrap-ansi": "^10.0.0", "ws": "^8.18.0", - "yauzl": "^2.10.0", "yargs": "^17.7.2", + "yauzl": "^2.10.0", "zod": "^3.23.8" }, "bin": { @@ -28143,8 +28143,8 @@ "@types/shell-quote": "^1.7.5", "@types/supertest": "^6.0.3", "@types/ws": "^8.5.0", - "@types/yauzl": "^2.9.1", "@types/yargs": "^17.0.32", + "@types/yauzl": "^2.9.1", "archiver": "^7.0.1", "ink-testing-library": "^4.0.0", "jsdom": "^26.1.0", diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 9f8e89ae892..9829d140b70 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -96,6 +96,7 @@ import type { BridgeRestoreSessionRequest, BridgeSessionState, BridgeRestoredSession, + BridgeSessionGoal, BridgeSessionSummary, BridgePendingInteraction, BridgeClientRequestContext, @@ -6146,6 +6147,13 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ); }, + async getSessionGoal(sessionId) { + return requestSessionStatus( + sessionId, + SERVE_CONTROL_EXT_METHODS.sessionGoalGet, + ); + }, + async continueSession(sessionId, context) { // Validate the originator up-front, mirroring POST /session/:id/prompt, so // an unknown client id (or a session that vanished) surfaces as an error diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 284745a3e31..02d0f96a4f7 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -373,6 +373,24 @@ export interface BridgeSessionSummary { color?: SessionGroupPresetColor | null; } +/** + * A session's live `/goal` state, as reported by the `qwen --acp` child. + * + * Only the active goal crosses the bridge. The child also caches the most + * recent goal that ended on its own, but nothing on this side reads it, so it + * is not part of the wire shape — add it back alongside the first consumer. + */ +export interface BridgeSessionGoal { + active: { + condition: string; + /** Judge turns completed so far; 0 before the first stop-hook evaluation. */ + iterations: number; + setAt: number; + /** The judge's verdict on the most recent turn, when it has run. */ + lastReason?: string; + } | null; +} + export interface SessionMetadataUpdate { displayName?: string; } @@ -971,6 +989,13 @@ export interface AcpSessionBridge { sessionId: string, ): Promise<{ cleared: boolean; condition?: string }>; + /** + * Read a live session's goal state. Throws `SessionNotFoundError` when the + * session is not resident — goals live in the child's memory, so a + * non-resident session has no goal to report. + */ + getSessionGoal(sessionId: string): Promise; + /** * Resume a live session's unfinished previous turn — an interrupted prompt * (model never answered) or a turn left with dangling tool calls — without diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index 70a029052fb..214dfb2b673 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -157,6 +157,13 @@ export const SERVE_CONTROL_EXT_METHODS = { // Runtime MCP server mutation ext-methods sessionTaskCancel: 'qwen/control/session/task/cancel', sessionGoalClear: 'qwen/control/session/goal/clear', + /** + * Read a live session's `/goal` state. The active goal lives only in the + * child's in-memory store, so this is the sole authoritative source for the + * condition, its running turn count and the judge's last verdict. Params: + * `{ sessionId }`; result: `{ active: ActiveGoalView | null }`. + */ + sessionGoalGet: 'qwen/control/session/goal/get', workspaceMcpRuntimeAdd: 'qwen/control/workspace/mcp/runtime-add', workspaceMcpRuntimeRemove: 'qwen/control/workspace/mcp/runtime-remove', workspaceReload: 'qwen/control/workspace/reload', diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 0ecf3d96067..85cb02ed76a 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -406,6 +406,12 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ updatedModelProviders: {}, }), unregisterGoalHook: vi.fn(), + getActiveGoal: vi.fn(), + getLastGoalTerminal: vi.fn(), + // Reached through the real `ui/utils/restoreGoal.js` on the resume path. + registerGoalHook: vi.fn(), + setGoalTerminalObserver: vi.fn(), + setLastGoalTerminal: vi.fn(), uiTelemetryService: { removeSession: vi.fn(), }, @@ -748,6 +754,8 @@ import { SessionTranscriptPageTooLargeError, encodeSessionTranscriptCursor, unregisterGoalHook, + getActiveGoal, + registerGoalHook, startEventLoopLagMonitor, registerAcpEventLoopLagGauge, SESSION_ARTIFACT_PERSISTENCE_VERSION, @@ -1531,6 +1539,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }) as unknown as InstanceType, @@ -1789,6 +1798,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), }) as unknown as InstanceType, ); @@ -1943,6 +1953,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }) as unknown as InstanceType, @@ -2121,6 +2132,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), emitGoalStatus: vi.fn(), @@ -6073,6 +6085,128 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('reads a live session goal, including the judge verdict', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + await setupSessionMocks(sessionId); + vi.mocked(getActiveGoal).mockReturnValue({ + condition: 'ship it', + iterations: 2, + setAt: 123, + tokensAtStart: 456, + hookId: 'goal-hook', + lastReason: 'one test still fails', + }); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionGoalGet, { sessionId }), + ).resolves.toEqual({ + active: { + condition: 'ship it', + iterations: 2, + setAt: 123, + lastReason: 'one test still fails', + }, + }); + // tokensAtStart / hookId are internals and must not leak over the wire. + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('reports a null goal state when nothing is active', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + await setupSessionMocks(sessionId); + vi.mocked(getActiveGoal).mockReturnValue(undefined); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionGoalGet, { sessionId }), + ).resolves.toEqual({ active: null }); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('rejects a goal read with a missing, empty or non-string sessionId', async () => { + await setupSessionMocks('11111111-1111-1111-1111-111111111111'); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + for (const params of [{}, { sessionId: '' }, { sessionId: 42 }]) { + await expect( + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionGoalGet, params), + ).rejects.toThrow(/sessionId/i); + } + expect(getActiveGoal).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('rejects a goal read for a session that is not resident', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + await setupSessionMocks(sessionId); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionGoalGet, { + sessionId: 'not-a-live-session', + }), + ).rejects.toThrow(); + expect(getActiveGoal).not.toHaveBeenCalledWith('not-a-live-session'); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('newSession with SSE MCP server creates MCPServerConfig with url', async () => { await setupSessionMocks('session-sse'); @@ -9122,6 +9256,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), } as unknown as InstanceType; @@ -10067,6 +10202,7 @@ describe('QwenAgent extMethod renameSession routing', () => { sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }) as unknown as InstanceType, @@ -10627,6 +10763,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { apiTimeMs: number; }; installRewriter: ReturnType; + installGoalTerminalObserver: ReturnType; startCronScheduler: ReturnType; dispose: ReturnType; } @@ -10714,6 +10851,11 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { }), getFileSystemService: vi.fn().mockReturnValue(undefined), setFileSystemService: vi.fn(), + // `goalRestoreBlockedBy` reads trust FIRST. Without this, resume threw + // `config.isTrustedFolder is not a function`, and the goal-gate tests + // below passed through `#restoreGoalOnResume`'s catch rather than the + // branch each one names. + isTrustedFolder: vi.fn().mockReturnValue(true), getHookSystem: vi.fn().mockReturnValue(undefined), getDisableAllHooks: vi.fn().mockReturnValue(true), hasHooksForEvent: vi.fn().mockReturnValue(false), @@ -10770,6 +10912,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { apiTimeMs: 11, }, installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }; @@ -10813,6 +10956,382 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { await agentPromise; }); + /** + * A persisted `system` / `slash_command` record carrying goal cards — the only + * place a daemon transcript stores them. + */ + function goalRecord(...outputHistoryItems: Array>) { + return { + uuid: 'goal-rec', + parentUuid: null, + sessionId: 'persisted-1', + timestamp: new Date(0).toISOString(), + type: 'system', + subtype: 'slash_command', + cwd: '/tmp', + version: '1.0.0', + systemPayload: { + phase: 'result', + rawCommand: '/goal', + outputHistoryItems, + }, + }; + } + + /** Lets `restoreGoalFromHistory` past its trust / hook-policy gates. */ + function allowGoalRestore(innerConfig: Record) { + innerConfig['isTrustedFolder'] = vi.fn().mockReturnValue(true); + innerConfig['getDisableAllHooks'] = vi.fn().mockReturnValue(false); + innerConfig['getHookSystem'] = vi.fn().mockReturnValue({ + addFunctionHook: vi.fn().mockReturnValue('hook-1'), + removeFunctionHook: vi.fn().mockReturnValue(true), + }); + } + + it('loadSession re-registers the goal hook when the transcript ends on an unsatisfied goal', async () => { + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 5, + }), + goalRecord({ + type: 'goal_status', + kind: 'checking', + condition: 'ship it', + iterations: 4, + }), + ], + }, + }); + allowGoalRestore(innerConfig as unknown as Record); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + expect(registerGoalHook).toHaveBeenCalledWith({ + config: innerConfig, + sessionId: 'persisted-1', + condition: 'ship it', + tokensAtStart: 0, + // Carried across resume so MAX_GOAL_ITERATIONS stays a cross-resume cap. + initialIterations: 4, + // Taken from the `set` card two records back: the trailing `checking` + // card has no setAt, so without the back-scan the goal's elapsed time + // would restart on every load. + initialSetAt: 5, + }); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession does not revive a goal the transcript already recorded as achieved', async () => { + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 5, + }), + goalRecord({ + type: 'goal_status', + kind: 'achieved', + condition: 'ship it', + iterations: 4, + durationMs: 900, + }), + ], + }, + }); + allowGoalRestore(innerConfig as unknown as Record); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + expect(registerGoalHook).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('unstable_resumeSession also re-registers the goal hook', async () => { + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'keep going', + setAt: 5, + }), + ], + }, + }); + allowGoalRestore(innerConfig as unknown as Record); + const { agent, agentPromise } = await spawnAgent(); + + await agent.unstable_resumeSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + expect(registerGoalHook).toHaveBeenCalledWith( + expect.objectContaining({ + condition: 'keep going', + initialIterations: 0, + }), + ); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession reinstalls the goal terminal observer after a restore', async () => { + // `registerGoalHook` calls `unregisterGoalHook`, which clears the session's + // goal-terminal observer. The ACP path passes no `addItem`, so nothing in + // `restoreGoalFromHistory` puts it back: a restored goal would then achieve + // or fail with no wire update and no persisted terminal card, and the next + // reload would revive a goal that already finished. + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 5, + }), + ], + }, + }); + allowGoalRestore(innerConfig as unknown as Record); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + expect(registerGoalHook).toHaveBeenCalled(); + expect(lastSessionMock!.installGoalTerminalObserver).toHaveBeenCalled(); + // Order is the assertion: installing before the restore would be undone. + const installedAt = + lastSessionMock!.installGoalTerminalObserver.mock.invocationCallOrder.at( + -1, + )!; + const registeredAt = vi + .mocked(registerGoalHook) + .mock.invocationCallOrder.at(-1)!; + expect(installedAt).toBeGreaterThan(registeredAt); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession reinstalls the goal terminal observer even when there is no goal to restore', async () => { + // The no-goal branch still calls `unregisterGoalHook`, which clears the + // observer the Session constructor installed. A `/goal` set later in this + // session would otherwise have no terminal card path. + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ + type: 'goal_status', + kind: 'achieved', + condition: 'ship it', + iterations: 1, + durationMs: 10, + }), + ], + }, + }); + allowGoalRestore(innerConfig as unknown as Record); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + expect(unregisterGoalHook).toHaveBeenCalled(); + const installedAt = + lastSessionMock!.installGoalTerminalObserver.mock.invocationCallOrder.at( + -1, + )!; + const unregisteredAt = vi + .mocked(unregisterGoalHook) + .mock.invocationCallOrder.at(-1)!; + expect(installedAt).toBeGreaterThan(unregisteredAt); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession does not attempt a goal restore for an empty transcript', async () => { + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { messages: [] }, + }); + allowGoalRestore(innerConfig as unknown as Record); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + // A brand-new session must not pay for a restore scan, and must not have + // its (absent) hook torn down by the no-goal branch either. + expect(registerGoalHook).not.toHaveBeenCalled(); + expect(unregisterGoalHook).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession still completes when the goal restore throws', async () => { + // Restoring a goal is best-effort: it must never take the session down with + // it. `registerGoalHook` is the deepest thing #restoreGoalOnResume calls. + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 5, + }), + ], + }, + }); + allowGoalRestore(innerConfig as unknown as Record); + vi.mocked(registerGoalHook).mockImplementation(() => { + throw new Error('hook system exploded'); + }); + const { agent, agentPromise } = await spawnAgent(); + + const response = await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + expect(registerGoalHook).toHaveBeenCalled(); + expect(response).toMatchObject({ + modes: expect.anything(), + models: expect.anything(), + configOptions: expect.anything(), + }); + // The throw path is where the `finally` earns its keep: `registerGoalHook` + // clears the observer before exploding, so a session that survives the + // throw but loses its observer would go on to reach achieved/failed with + // nobody listening — no wire update, no persisted terminal card. + expect(lastSessionMock!.installGoalTerminalObserver).toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('reports a malformed condition exactly once on resume', async () => { + // Two producers could speak for this one event: `restoreGoalFromHistory` + // (which knows the condition is bad) and `#restoreGoalOnResume` (which + // knows the session). The env gates print one line; this must too. + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ type: 'goal_status', kind: 'set', condition: '' }), + ], + }, + }); + allowGoalRestore(innerConfig as unknown as Record); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockReturnValue(true) as unknown as MockInstance; + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + const lines = stderr.mock.calls + .map((c) => String(c[0])) + .filter((l) => /goal/i.test(l)); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('the condition is empty'); + expect(registerGoalHook).not.toHaveBeenCalled(); + stderr.mockRestore(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession leaves the goal hook alone when hooks are disabled by policy', async () => { + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 5, + }), + ], + }, + }); + // makeRestoreInnerConfig defaults to getDisableAllHooks() === true. + const stderr = vi + .spyOn(process.stderr, 'write') + .mockReturnValue(true) as unknown as MockInstance; + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + expect(registerGoalHook).not.toHaveBeenCalled(); + // `registerGoalHook` not being called is not enough on its own: anything + // that throws inside `#restoreGoalOnResume` skips it too, so a broken + // config mock would satisfy the assertion above while never reaching the + // hooks-disabled branch this test is named for. Pin the branch. + const written = stderr.mock.calls.map((c) => String(c[0])).join(''); + expect(written).toContain('hooks-disabled'); + expect(written).not.toContain('goal restore failed'); + stderr.mockRestore(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('loadSession returns LoadSessionResponse and replays history on the session', async () => { const messages = [{ role: 'user', parts: [{ text: 'hi' }] }]; bindRestoreMocks({ @@ -12214,6 +12733,7 @@ describe('sessionLanguage multi-session propagation', () => { getConfig: vi.fn().mockReturnValue(cfg), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }; @@ -12314,6 +12834,7 @@ describe('sessionLanguage multi-session propagation', () => { getConfig: vi.fn().mockReturnValue(cfg), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), } as unknown as InstanceType; @@ -12404,6 +12925,7 @@ describe('sessionLanguage multi-session propagation', () => { isIdle: vi.fn().mockReturnValue(true), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }) as unknown as InstanceType, @@ -12608,6 +13130,7 @@ describe('sessionLanguage multi-session propagation', () => { getConfig: vi.fn().mockReturnValue(cfg), sendAvailableCommandsUpdate, installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }) as unknown as InstanceType, @@ -12686,6 +13209,7 @@ describe('sessionLanguage multi-session propagation', () => { getConfig: vi.fn().mockReturnValue(cfg), sendAvailableCommandsUpdate, installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), }) as unknown as InstanceType, diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index af0dc7dcd04..a276bc17473 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -67,6 +67,7 @@ import { subagentGenerator, redactUrlCredentials, computeUniqueBranchTitle, + getActiveGoal, unregisterGoalHook, ToolNames, FORK_SUBAGENT_TYPE, @@ -300,6 +301,11 @@ import { formatContextUsageText, } from '../ui/commands/contextCommand.js'; import type { HistoryItemContextUsage } from '../ui/types.js'; +import { + collectGoalStatusItemsFromRecords, + restoreGoalFromHistory, +} from '../ui/utils/restoreGoal.js'; +import { writeStderrLineSafe } from '../utils/stdioHelpers.js'; import { executeGeneration, GENERATION_MAX_PROMPT_BYTES, @@ -3515,6 +3521,8 @@ class QwenAgent implements Agent { records: replayPage.records, gaps: sessionData?.historyGaps, cumulativeUsage: replayUsage, + // A resume: the goal restore runs right after this. + supersedeUnrestorableGoal: true, logger: debugLogger, }); replayUpdates = replay.updates; @@ -3547,6 +3555,7 @@ class QwenAgent implements Agent { } await this.#restoreWorktreeOnResume(config, session); + this.#restoreGoalOnResume(config, session); const modesData = this.buildModesData(config); const availableModels = this.buildAvailableModels(config); @@ -3607,6 +3616,7 @@ class QwenAgent implements Agent { ); await this.#restoreWorktreeOnResume(config, session); + this.#restoreGoalOnResume(config, session); const modesData = this.buildModesData(config); const availableModels = this.buildAvailableModels(config); @@ -3646,6 +3656,63 @@ class QwenAgent implements Agent { } } + /** + * Re-registers the `/goal` Stop hook when a resumed transcript ends on an + * unsatisfied goal — the daemon counterpart of the TUI's resume restore. + * Without this the goal loop silently dies whenever a session is reloaded or + * `qwen serve` restarts, even though the transcript still shows it as active. + * + * The `addItem` bridge that `restoreGoalFromHistory` takes in the TUI is not + * used here — the daemon's terminal card goes out over the wire, not into an + * Ink history. But restore reaches `unregisterGoalHook` on every path, + * including the one where there was nothing to restore, and that clears the + * observer the `Session` constructor installed. So it is put back afterwards, + * unconditionally: without it a restored goal that later achieves or fails + * emits no terminal update and persists no terminal card, and the next reload + * revives a goal that already finished. + * + * Best-effort: a failed restore must not block session load. + */ + #restoreGoalOnResume(config: Config, session: Session): void { + try { + const records = config.getResumedSessionData()?.conversation.messages; + if (!records?.length) return; + const restored = restoreGoalFromHistory( + collectGoalStatusItemsFromRecords(records), + config, + ); + if (restored.restored) { + debugLogger.info( + `ACP goal restored sessionId=${config.getSessionId()} condition=${restored.condition}`, + ); + } else if ( + restored.blockedBy && + restored.blockedBy !== 'condition-invalid' + ) { + // The transcript still holds an active goal card. `HistoryReplayer` + // supersedes it with a `cleared` card so the client does not show a + // goal that nothing is driving; say why on stderr. + // + // `condition-invalid` is excluded: `restoreGoalFromHistory` already + // wrote a line for it (it is the only caller that knows the condition + // is malformed). Logging here too would double-report the one case, + // while the env gates below report once. + writeStderrLineSafe( + `qwen: not restoring the active goal for session ${config.getSessionId()} (${restored.blockedBy}).`, + ); + } + } catch (error) { + // Not debugLogger: it no-ops unless a debug session is active, and a + // failed restore is invisible from the outside — the transcript still + // shows the goal as active while no hook drives it. + writeStderrLineSafe( + `qwen: goal restore failed for session ${config.getSessionId()}: ${error}`, + ); + } finally { + session.installGoalTerminalObserver(); + } + } + async unstable_listSessions( params: ListSessionsRequest, ): Promise { @@ -8034,6 +8101,34 @@ class QwenAgent implements Agent { condition: cleared?.condition, }; } + case SERVE_CONTROL_EXT_METHODS.sessionGoalGet: { + const sessionId = params['sessionId']; + if (typeof sessionId !== 'string' || sessionId.length === 0) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing sessionId', + ); + } + // Throws when the session is not live. That is the honest answer: the + // goal store is in-memory, so a goal only exists — and only advances — + // while its session is resident. + this.sessionOrThrow(sessionId); + const active = getActiveGoal(sessionId); + return { + // Projected field by field: `ActiveGoal` also carries `hookId` and + // `tokensAtStart`, which are this process's business. + active: active + ? { + condition: active.condition, + iterations: active.iterations, + setAt: active.setAt, + ...(active.lastReason !== undefined + ? { lastReason: active.lastReason } + : {}), + } + : null, + }; + } case SERVE_CONTROL_EXT_METHODS.sessionContinue: { const sessionId = params['sessionId']; if (typeof sessionId !== 'string' || sessionId.length === 0) { diff --git a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts index 5dffb72b483..b239d03c933 100644 --- a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts @@ -419,6 +419,7 @@ describe('QwenAgent loadSession — Phase C worktree context restore', () => { sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), + installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), dispose: vi.fn(), pendingWorktreeNotice: null as string | null, diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index cc4e7c449e5..1901ab3da32 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -8855,6 +8855,154 @@ describe('Session', () => { }); }); }); + + const recordedGoalCards = () => + mockChatRecordingService.recordSlashCommand.mock.calls + .map((call) => call[0] as { outputHistoryItems?: unknown[] }) + .flatMap((payload) => payload.outputHistoryItems ?? []) + .filter( + (item) => + (item as { type?: string }).type === MessageType.GOAL_STATUS, + ); + + it('persists a cleared card, so resume cannot revive a goal the user dropped', () => { + // The `sessionGoalClear` ext method reaches the transcript through this + // method. Without the record, the last persisted card stays `set` and + // the next resume re-registers a goal the user explicitly cleared. + session.emitGoalStatus({ + kind: 'cleared', + condition: 'check weather', + iterations: 2, + durationMs: 5000, + }); + + expect(recordedGoalCards()).toEqual([ + { + type: MessageType.GOAL_STATUS, + kind: 'cleared', + condition: 'check weather', + iterations: 2, + durationMs: 5000, + }, + ]); + }); + + it('persists the cleared card when /goal clear arrives as a prompt', async () => { + // The web shell clears via the `sessionGoalClear` ext method, but an ACP + // client (Zed) can send `/goal clear` as a prompt. That returns a + // `message` result, whose `outputHistoryItems` still carry the cleared + // card — `#emitGoalStatusItems` runs before the switch — so the card is + // persisted on this path too. + vi.mocked( + nonInteractiveCliCommands.handleSlashCommand, + ).mockResolvedValueOnce({ + type: 'message', + messageType: 'info', + content: 'Goal cleared: check weather', + outputHistoryItems: [ + { + type: MessageType.GOAL_STATUS, + kind: 'cleared', + condition: 'check weather', + iterations: 2, + durationMs: 5000, + }, + ], + }); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: '/goal clear' }], + }); + + expect(recordedGoalCards()).toEqual([ + { + type: MessageType.GOAL_STATUS, + kind: 'cleared', + condition: 'check weather', + iterations: 2, + durationMs: 5000, + }, + ]); + }); + + it('persists the goal card so a resumed session can restore the hook', async () => { + vi.mocked( + nonInteractiveCliCommands.handleSlashCommand, + ).mockResolvedValueOnce({ + type: 'submit_prompt', + content: [{ text: 'Continue until the goal is met.' }], + outputHistoryItems: [ + { + type: MessageType.GOAL_STATUS, + kind: 'set', + condition: 'check weather', + setAt: 1234, + }, + ], + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: '/goal check weather' }], + }); + + expect(recordedGoalCards()).toEqual([ + { + type: MessageType.GOAL_STATUS, + kind: 'set', + condition: 'check weather', + setAt: 1234, + }, + ]); + }); + + it('persists the terminal goal card so resume does not revive a finished goal', async () => { + vi.mocked( + nonInteractiveCliCommands.handleSlashCommand, + ).mockResolvedValueOnce({ + type: 'submit_prompt', + content: [{ text: 'Continue until the goal is met.' }], + outputHistoryItems: [ + { + type: MessageType.GOAL_STATUS, + kind: 'set', + condition: 'check weather', + setAt: 1234, + }, + ], + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: '/goal check weather' }], + }); + + core.notifyGoalTerminal('test-session-id', { + kind: 'achieved', + condition: 'check weather', + iterations: 1, + durationMs: 5000, + lastReason: 'Weather checked.', + }); + + await vi.waitFor(() => { + expect(recordedGoalCards()).toContainEqual({ + type: MessageType.GOAL_STATUS, + kind: 'achieved', + condition: 'check weather', + iterations: 1, + durationMs: 5000, + lastReason: 'Weather checked.', + }); + }); + }); }); describe('tool preparation stream lifecycle', () => { diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 23148b7caa8..b2568593888 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -193,6 +193,10 @@ import { MessageType, type HistoryItemGoalStatus, } from '../../ui/types.js'; +import { + goalTerminalEventToHistoryItem, + recordGoalStatusItem, +} from '../../ui/utils/restoreGoal.js'; import { ACP_ROUTE_ID_PREFIX, buildAcpModelOptions, @@ -1063,10 +1067,14 @@ export class Session implements SessionContext { // Initialize modular components with this session as context this.toolCallEmitter = new ToolCallEmitter(this); this.planEmitter = new PlanEmitter(this); - this.historyReplayer = new HistoryReplayer(this); + // This replayer only ever runs on resume, so it may correct an active goal + // card that `#restoreGoalOnResume` is about to refuse. + this.historyReplayer = new HistoryReplayer(this, { + supersedeUnrestorableGoal: true, + }); this.messageEmitter = new MessageEmitter(this); - this.#installGoalTerminalObserver(); + this.installGoalTerminalObserver(); this.#registerBackgroundNotificationCallbacks(); this.#registerSubSessionSpawner(); } @@ -1207,22 +1215,46 @@ export class Session implements SessionContext { } } - #installGoalTerminalObserver(): void { + /** + * Installs (or replaces) this session's goal-terminal observer. + * + * Public because it does not stay installed: `registerGoalHook` and + * `unregisterGoalHook` both clear the observer table for the session, so any + * caller that (re-)registers a goal outside `#processSlashCommandResult` — + * notably goal restore on resume — has to put it back. Idempotent. + */ + installGoalTerminalObserver(): void { setGoalTerminalObserver(this.sessionId, (event: GoalTerminalEvent) => { void this.messageEmitter.emitGoalTerminal(event).catch((error) => { debugLogger.warn( `Failed to emit goal terminal update: ${this.#formatError(error)}`, ); }); + // The wire update is live-only. Persist the terminal card too, so a + // resumed session sees the goal as finished instead of re-registering it + // from the still-present `set` card. + recordGoalStatusItem(this.config, goalTerminalEventToHistoryItem(event)); }); } + /** + * Emits a goal card and persists it to the transcript. Both `set` and + * `cleared` reach the client this way — from `#emitGoalStatusItems` for a + * `/goal` prompt, and from the `sessionGoalClear` ext method — so recording + * here (rather than at each call site) keeps the transcript in step with the + * hook. Replay goes through `messageEmitter.emitGoalStatus` directly and so + * does not re-record. + */ emitGoalStatus(status: Omit): void { void this.messageEmitter.emitGoalStatus(status).catch((error) => { debugLogger.warn( `Failed to emit goal status update: ${this.#formatError(error)}`, ); }); + recordGoalStatusItem(this.config, { + type: MessageType.GOAL_STATUS, + ...status, + }); } /** @@ -5873,7 +5905,7 @@ export class Session implements SessionContext { } } if (hasActiveGoalStatus) { - this.#installGoalTerminalObserver(); + this.installGoalTerminalObserver(); } } diff --git a/packages/cli/src/acp-integration/session/history-replay-page.ts b/packages/cli/src/acp-integration/session/history-replay-page.ts index c681b09058f..a07076b3fc9 100644 --- a/packages/cli/src/acp-integration/session/history-replay-page.ts +++ b/packages/cli/src/acp-integration/session/history-replay-page.ts @@ -139,6 +139,7 @@ export async function collectHistoryReplayUpdates({ gaps, cumulativeUsage, logger, + supersedeUnrestorableGoal, }: { sessionId: string; config?: Config; @@ -146,11 +147,18 @@ export async function collectHistoryReplayUpdates({ gaps?: HistoryGap[]; cumulativeUsage: CumulativeUsage; logger?: ReplayLogger; + /** + * Forwarded to `HistoryReplayer`. Only the resume path, where + * `#restoreGoalOnResume` follows, sets this. Reading another session's + * history must render it as it was, not editorialize a goal it won't restore. + */ + supersedeUnrestorableGoal?: boolean; }): Promise<{ updates: SessionUpdate[]; replayError?: string }> { const updates: SessionUpdate[] = []; try { await new HistoryReplayer( replayContext(sessionId, updates, cumulativeUsage, config), + { supersedeUnrestorableGoal }, ).replay(records, gaps); } catch (error) { const replayError = error instanceof Error ? error.message : String(error); diff --git a/packages/cli/src/acp-integration/session/history-replayer.test.ts b/packages/cli/src/acp-integration/session/history-replayer.test.ts index e3be5811742..108ac2ee821 100644 --- a/packages/cli/src/acp-integration/session/history-replayer.test.ts +++ b/packages/cli/src/acp-integration/session/history-replayer.test.ts @@ -5,6 +5,12 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Deliberately NOT mocked: `writeStderrLineSafe` is the thing under test in +// "survives a broken stderr" below, and a mocked stand-in would re-implement the +// very try/catch it is supposed to prove exists. Tests that trigger a stderr +// line spy on `process.stderr.write` instead. + import { HistoryReplayer, MISSING_TOOL_RESULT_MESSAGE, @@ -1011,6 +1017,353 @@ describe('HistoryReplayer', () => { }); }); + describe('goal card replay', () => { + const goalRecord = ( + ...outputHistoryItems: Array> + ): ChatRecord => + ({ + uuid: 'goal-uuid', + parentUuid: null, + sessionId: 'test-session', + timestamp: new Date().toISOString(), + type: 'system', + subtype: 'slash_command', + cwd: '/test', + version: '1.0.0', + systemPayload: { + phase: 'result', + rawCommand: '/goal', + outputHistoryItems, + }, + }) as unknown as ChatRecord; + + const goalStatuses = () => + sentUpdates() + .map((u) => u['_meta'] as Record | undefined) + .map((meta) => meta?.['goalStatus']) + .filter(Boolean); + + it('re-emits a persisted goal card as _meta.goalStatus, without the type field', async () => { + await replayer.replay([ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 1234, + }), + ]); + + expect(goalStatuses()).toEqual([ + { kind: 'set', condition: 'ship it', setAt: 1234 }, + ]); + }); + + it('re-emits terminal goal cards', async () => { + await replayer.replay([ + goalRecord({ + type: 'goal_status', + kind: 'achieved', + condition: 'ship it', + iterations: 3, + durationMs: 900, + lastReason: 'tests pass', + }), + ]); + + expect(goalStatuses()).toEqual([ + { + kind: 'achieved', + condition: 'ship it', + iterations: 3, + durationMs: 900, + lastReason: 'tests pass', + }, + ]); + }); + + it('skips per-iteration checking cards so a long TUI goal loop does not flood replay', async () => { + await replayer.replay([ + goalRecord({ type: 'goal_status', kind: 'set', condition: 'ship it' }), + goalRecord({ + type: 'goal_status', + kind: 'checking', + condition: 'ship it', + iterations: 1, + }), + goalRecord({ + type: 'goal_status', + kind: 'checking', + condition: 'ship it', + iterations: 2, + }), + ]); + + expect(goalStatuses()).toEqual([{ kind: 'set', condition: 'ship it' }]); + }); + + it('refuses to replay a goal card whose condition is empty', async () => { + // A transcript is a file: a corrupted or hand-edited condition would + // otherwise ride out to every client inside `_meta.goalStatus`. + // `restoreGoalFromHistory` refuses the same card, so neither the card nor + // the hook survives — they stay consistent. + await replayer.replay([ + goalRecord({ type: 'goal_status', kind: 'set', condition: '' }), + ]); + + expect(goalStatuses()).toEqual([]); + expect(sendUpdateSpy).not.toHaveBeenCalled(); + }); + + it('replays a goal card far longer than the old 4,000-char cap', async () => { + // `/goal` accepts any length (#6665); dropping the card here would hide a + // running goal from every client. + const condition = 'x'.repeat(10_000); + await replayer.replay([ + goalRecord({ type: 'goal_status', kind: 'set', condition }), + ]); + + expect(goalStatuses()).toEqual([{ kind: 'set', condition }]); + }); + + it('does not fall through to the plain-text path for goal cards', async () => { + await replayer.replay([ + goalRecord({ type: 'goal_status', kind: 'set', condition: 'ship it' }), + ]); + + expect(sentUpdates()).toHaveLength(1); + expect(sentUpdates()[0]['content']).toEqual({ type: 'text', text: '' }); + }); + + it('still replays non-goal output items as agent text', async () => { + await replayer.replay([ + goalRecord( + { type: 'goal_status', kind: 'set', condition: 'ship it' }, + { type: 'assistant', text: 'hello' }, + ), + ]); + + const texts = sentUpdates() + .map((u) => (u['content'] as Record)?.['text']) + .filter(Boolean); + expect(texts).toEqual(['hello']); + expect(goalStatuses()).toHaveLength(1); + }); + + it('survives a broken stderr instead of abandoning the transcript', async () => { + // The empty-condition card writes a diagnostic. `process.stderr.write` + // throws on EPIPE (`qwen … | head`, or a daemon whose stderr reader went + // away), and a raw `writeStderrLine` would take that throw out through the + // item loop and the record loop, aborting the whole replay: the user loses + // their transcript because we failed to *complain* about one bad card. + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => { + throw Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }); + }); + + const record = goalRecord(); + ( + record as unknown as { systemPayload: Record } + ).systemPayload['outputHistoryItems'] = [ + // Trips the diagnostic... + { type: 'goal_status', kind: 'set', condition: '' }, + // ...and this must still be replayed afterwards. + { type: 'goal_status', kind: 'set', condition: 'ship it' }, + { type: 'assistant', text: 'hello' }, + ]; + + await expect(replayer.replay([record])).resolves.toBeUndefined(); + + expect(stderr).toHaveBeenCalled(); + expect(goalStatuses()).toEqual([{ kind: 'set', condition: 'ship it' }]); + const texts = sentUpdates() + .map((u) => (u['content'] as Record)?.['text']) + .filter(Boolean); + expect(texts).toEqual(['hello']); + + stderr.mockRestore(); + }); + + it('survives a slash_command record whose outputHistoryItems is not an array', async () => { + const record = goalRecord(); + ( + record as unknown as { systemPayload: Record } + ).systemPayload['outputHistoryItems'] = { + type: 'goal_status', + kind: 'set', + condition: 'ship it', + }; + + await expect(replayer.replay([record])).resolves.toBeUndefined(); + expect(goalStatuses()).toEqual([]); + }); + + it('survives null entries and still replays the valid cards after them', async () => { + const record = goalRecord(); + ( + record as unknown as { systemPayload: Record } + ).systemPayload['outputHistoryItems'] = [ + null, + 'not an object', + { type: 'goal_status', kind: 'set', condition: 'ship it' }, + ]; + + await expect(replayer.replay([record])).resolves.toBeUndefined(); + expect(goalStatuses()).toEqual([{ kind: 'set', condition: 'ship it' }]); + }); + }); + + describe('an active goal that cannot be restored is superseded', () => { + // The client reads "a goal is running" off the newest goal card it saw. If + // restore is going to refuse the goal, replaying the `set` card alone + // leaves the UI claiming a live loop that nothing drives. + const goalRecord = ( + ...outputHistoryItems: Array> + ): ChatRecord => + ({ + uuid: 'goal-uuid', + parentUuid: null, + sessionId: 'test-session', + timestamp: new Date().toISOString(), + type: 'system', + subtype: 'slash_command', + cwd: '/test', + version: '1.0.0', + systemPayload: { + phase: 'result', + rawCommand: '/goal', + outputHistoryItems, + }, + }) as unknown as ChatRecord; + + const goalStatuses = () => + sentUpdates() + .map((u) => u['_meta'] as Record | undefined) + .map((meta) => meta?.['goalStatus'] as Record) + .filter(Boolean); + + const replayWithConfig = async ( + config: Partial>, + records: ChatRecord[], + ) => { + const ctx = { + ...mockContext, + config: { + getToolRegistry: () => ({ getTool: () => null }), + isTrustedFolder: () => true, + getDisableAllHooks: () => false, + getHookSystem: () => ({}), + ...config, + } as unknown as Config, + } as unknown as SessionContext; + await new HistoryReplayer(ctx, { + supersedeUnrestorableGoal: true, + }).replay(records); + }; + + it.each([ + [ + 'the folder is no longer trusted', + { isTrustedFolder: () => false }, + 'not trusted', + ], + [ + 'hooks are disabled by policy', + { getDisableAllHooks: () => true }, + 'hooks are disabled', + ], + [ + 'the hook system is unavailable', + { getHookSystem: () => undefined }, + 'hook system is unavailable', + ], + ])('emits a trailing cleared card when %s', async (_l, cfg, reason) => { + await replayWithConfig(cfg, [ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 1234, + }), + ]); + + const statuses = goalStatuses(); + expect(statuses).toHaveLength(2); + expect(statuses[0]).toMatchObject({ kind: 'set' }); + // Ordering is the whole point: `loadSession` batches replay updates into + // its response, so a card emitted after replay would reach the client + // first and lose to the `set` card. + expect(statuses[1]).toMatchObject({ + kind: 'cleared', + condition: 'ship it', + setAt: 1234, + }); + expect(statuses[1]['lastReason']).toContain(reason); + }); + + it('leaves a restorable goal alone', async () => { + await replayWithConfig({}, [ + goalRecord({ type: 'goal_status', kind: 'set', condition: 'ship it' }), + ]); + expect(goalStatuses()).toEqual([{ kind: 'set', condition: 'ship it' }]); + }); + + it('says nothing when the transcript has no active goal', async () => { + await replayWithConfig({ isTrustedFolder: () => false }, [ + goalRecord({ + type: 'goal_status', + kind: 'achieved', + condition: 'ship it', + iterations: 1, + durationMs: 5, + }), + ]); + expect(goalStatuses()).toHaveLength(1); + expect(goalStatuses()[0]).toMatchObject({ kind: 'achieved' }); + }); + + it('says nothing when the active card was already dropped as invalid', async () => { + // The empty-condition card never reached the client, so there is no + // phantom "running" state to correct — a `cleared` card would name a goal + // the user never saw. + await replayWithConfig({ isTrustedFolder: () => false }, [ + goalRecord({ type: 'goal_status', kind: 'set', condition: '' }), + ]); + expect(goalStatuses()).toEqual([]); + }); + + it('stays off by default, and never touches config when it is off', async () => { + // Export replays a transcript through this class with a config stub that + // throws on any method it does not implement. A replay that only renders + // history must not ask about trust or hook policy — or editorialize. + const ctx = { + ...mockContext, + config: new Proxy( + { getToolRegistry: () => ({ getTool: () => null }) }, + { + get(target: Record, prop: string | symbol) { + if (prop in target) return target[prop as string]; + if (typeof prop === 'symbol') return undefined; + throw new Error(`config does not implement ${String(prop)}`); + }, + }, + ) as unknown as Config, + } as unknown as SessionContext; + + await expect( + new HistoryReplayer(ctx).replay([ + goalRecord({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + }), + ]), + ).resolves.toBeUndefined(); + + expect(goalStatuses()).toEqual([{ kind: 'set', condition: 'ship it' }]); + }); + }); + describe('mixed record types', () => { it('should handle a complete conversation replay', async () => { const records: ChatRecord[] = [ diff --git a/packages/cli/src/acp-integration/session/history-replayer.ts b/packages/cli/src/acp-integration/session/history-replayer.ts index dd48950cc6b..93c393bb00f 100644 --- a/packages/cli/src/acp-integration/session/history-replayer.ts +++ b/packages/cli/src/acp-integration/session/history-replayer.ts @@ -16,6 +16,7 @@ import type { GenerateContentResponseUsageMetadata, } from '@google/genai'; import type { SessionEmitterContext } from './types.js'; +import { hasFullSessionContext } from './types.js'; import { MessageEmitter } from './emitters/MessageEmitter.js'; import { ToolCallEmitter } from './emitters/tool-call-emitter.js'; import { getToolResultCallId } from '../../utils/chat-record-tool-call-id.js'; @@ -23,6 +24,31 @@ import { formatHistoryGapNotice, indexGapsByChild, } from '../../ui/utils/history-gap-notice.js'; +import { + collectGoalStatusItemsFromRecords, + findGoalToRestore, + goalConditionBlockedBy, + goalRestoreBlockedBy, + isTranscriptItemRecord, + parseGoalStatusItem, + type GoalRestoreBlockedReason, +} from '../../ui/utils/restoreGoal.js'; +import { writeStderrLineSafe } from '../../utils/stdioHelpers.js'; + +/** + * Shown on the `cleared` card that supersedes an active goal the resumed + * session refuses to restore. `condition-invalid` never reaches here: such a + * card is dropped from the replay outright. + */ +const GOAL_NOT_RESTORED_REASON: Record< + Exclude, + string +> = { + 'untrusted-folder': + 'Goal not restored: this folder is not trusted, so its Stop hook cannot run.', + 'hooks-disabled': 'Goal not restored: hooks are disabled for this session.', + 'no-hook-system': 'Goal not restored: the hook system is unavailable.', +}; export const MISSING_TOOL_RESULT_MESSAGE = 'Tool result missing from saved history; the previous run likely ended ' + @@ -52,17 +78,37 @@ export interface HistoryReplayPageState { * This ensures that replayed history looks identical to how it would * have appeared during the original session. */ +export interface HistoryReplayerOptions { + /** + * Emit a trailing `cleared` card when the transcript ends on an active goal + * this session will refuse to restore. Only meaningful where goal restore + * actually follows the replay — i.e. resuming a session into a live agent. + * + * Off by default. A replay that merely renders a transcript (export, or + * reading another session's history) must reproduce what happened, not + * editorialize about a Stop hook it was never going to register. It also has + * no business asking `config` for trust and hook policy: the export path + * supplies a config stub that throws on any method it does not implement. + */ + supersedeUnrestorableGoal?: boolean; +} + export class HistoryReplayer { private readonly ctx: SessionEmitterContext; private readonly messageEmitter: MessageEmitter; private readonly toolCallEmitter: ToolCallEmitter; + private readonly options: HistoryReplayerOptions; private readonly pendingReplayToolCalls = new Map< string, PendingReplayToolCall >(); - constructor(ctx: SessionEmitterContext) { + constructor( + ctx: SessionEmitterContext, + options: HistoryReplayerOptions = {}, + ) { this.ctx = ctx; + this.options = options; this.messageEmitter = new MessageEmitter(ctx); this.toolCallEmitter = new ToolCallEmitter(ctx); } @@ -78,6 +124,7 @@ export class HistoryReplayer { async replay(records: ChatRecord[], gaps?: HistoryGap[]): Promise { try { await this.replayPage(records, { finalizeDangling: true, gaps }); + await this.supersedeUnrestorableGoal(records); } finally { this.pendingReplayToolCalls.clear(); this.setActiveRecordId(null); @@ -418,16 +465,49 @@ export class HistoryReplayer { * Replays a slash_command system record by re-emitting its output as an * agent message chunk. This allows Zed to reconstruct the correct turn * structure (user → agent) on session resume without polluting model context. + * + * Goal cards are re-emitted as `_meta.goalStatus` rather than text: they carry + * no `text` field, so the plain-text path below would silently drop them and + * the client would lose the goal card (and its status pill) on every reload. + * Per-iteration `checking` cards are skipped — a TUI transcript persists one + * per stop-hook turn, and clients suppress them as noise. Skipping costs no + * fidelity: goal restore reads the records directly, not this replay. */ private async replaySlashCommandResult(record: ChatRecord): Promise { const payload = record.systemPayload as | SlashCommandRecordPayload | undefined; - if (payload?.phase !== 'result' || !payload.outputHistoryItems?.length) { - return; - } - for (const item of payload.outputHistoryItems) { - const text = typeof item['text'] === 'string' ? item['text'] : ''; + if (payload?.phase !== 'result') return; + // Typed as an array, but it came off disk: a hand-edited record could make + // it any JSON value, and iterating a plain object throws. + const items: unknown = payload.outputHistoryItems; + if (!Array.isArray(items) || items.length === 0) return; + for (const item of items) { + const goalStatus = parseGoalStatusItem(item); + if (goalStatus) { + if (goalConditionBlockedBy(goalStatus.condition)) { + // A transcript is a file: a corrupted or hand-edited condition would + // otherwise ride out to every client inside `_meta.goalStatus`. + // `restoreGoalFromHistory` refuses the same card, so skipping it here + // keeps the card and the hook consistent — neither survives. + // + // Safe variant: a throwing stderr would abandon this record's + // remaining cards and then abort the whole replay, losing the + // transcript over a failed diagnostic about one bad card. + writeStderrLineSafe( + 'qwen: skipping replay of a goal card whose condition is empty.', + ); + } else if (goalStatus.kind !== 'checking') { + const { type: _type, ...status } = goalStatus; + await this.messageEmitter.emitGoalStatus(status); + } + continue; + } + // Not a goal card, and not necessarily an object either. + const text = + isTranscriptItemRecord(item) && typeof item['text'] === 'string' + ? item['text'] + : ''; if (text) { await this.messageEmitter.emitSlashCommandOutput( text.replace(/\n/g, ' \n'), @@ -437,6 +517,47 @@ export class HistoryReplayer { } } + /** + * Emits a trailing `cleared` card when the transcript ends on an active goal + * that `restoreGoalFromHistory` is about to refuse. + * + * A client reads "there is an active goal" off the newest goal card it has + * seen, so replaying a `set` card that no Stop hook will drive leaves the UI + * claiming a goal is running when the loop is dead. The gates are pure + * functions of `config`, so the answer is known here, before restore runs. + * + * This card is emitted, not recorded: the transcript keeps its `set` card, so + * a later resume in a trusted folder (or with hooks re-enabled) restores the + * goal instead of finding it destroyed. Emitting from inside replay is also + * what puts the card *after* the `set` card — `loadSession` batches replay + * updates into its response, and a notification sent afterwards would reach + * the client first. + * + * Gated on `supersedeUnrestorableGoal`: only a resume registers a hook, and + * only a resume has a `config` that answers trust and hook-policy questions. + */ + private async supersedeUnrestorableGoal( + records: ChatRecord[], + ): Promise { + if (!this.options.supersedeUnrestorableGoal) return; + const active = findGoalToRestore( + collectGoalStatusItemsFromRecords(records), + ); + // An invalid condition was never replayed, so no active card is on screen. + if (!active || goalConditionBlockedBy(active.condition)) return; + // Goal restore only follows a resume, where the context carries a config. + if (!hasFullSessionContext(this.ctx)) return; + const blockedBy = goalRestoreBlockedBy(this.ctx.config); + if (!blockedBy) return; + await this.messageEmitter.emitGoalStatus({ + kind: 'cleared', + condition: active.condition, + iterations: active.iterations, + ...(active.setAt !== undefined ? { setAt: active.setAt } : {}), + lastReason: GOAL_NOT_RESTORED_REASON[blockedBy], + }); + } + /** * Extracts tool name from a chat record's function response. */ diff --git a/packages/cli/src/serve/routes/goals.test.ts b/packages/cli/src/serve/routes/goals.test.ts new file mode 100644 index 00000000000..7cbb04f6cfe --- /dev/null +++ b/packages/cli/src/serve/routes/goals.test.ts @@ -0,0 +1,262 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import express from 'express'; +import { describe, expect, it, vi } from 'vitest'; +import request from 'supertest'; + +vi.mock('../../utils/stdioHelpers.js', () => ({ + writeStderrLine: vi.fn(), +})); + +import type { + BridgeSessionGoal, + BridgeSessionSummary, +} from '@qwen-code/acp-bridge'; +import { writeStderrLine } from '../../utils/stdioHelpers.js'; +import { registerGoalsRoutes, type GoalsSessionBridge } from './goals.js'; + +const WORKSPACE = '/w'; + +const summary = ( + sessionId: string, + overrides: Partial = {}, +): BridgeSessionSummary => ({ + sessionId, + workspaceCwd: WORKSPACE, + createdAt: new Date(0).toISOString(), + clientCount: 1, + hasActivePrompt: false, + ...overrides, +}); + +const activeGoal = ( + condition: string, + overrides: Partial> = {}, +): BridgeSessionGoal => ({ + active: { condition, iterations: 0, setAt: 1000, ...overrides }, +}); + +const noGoal: BridgeSessionGoal = { active: null }; + +function makeApp(bridge: GoalsSessionBridge) { + const app = express(); + registerGoalsRoutes(app, { boundWorkspace: WORKSPACE, bridge }); + return app; +} + +describe('GET /goals', () => { + it('returns an empty list when no session has a goal', async () => { + const app = makeApp({ + listWorkspaceSessions: () => [summary('s1'), summary('s2')], + getSessionGoal: async () => noGoal, + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ v: 1, goals: [], droppedCount: 0 }); + }); + + it('projects each active goal onto its session, newest first', async () => { + const goals: Record = { + s1: activeGoal('fix flaky tests', { + iterations: 3, + setAt: 1000, + lastReason: 'two tests still fail', + }), + s2: activeGoal('raise coverage', { setAt: 2000 }), + }; + const app = makeApp({ + listWorkspaceSessions: () => [ + summary('s1', { displayName: 'fix-ci' }), + summary('s2', { hasActivePrompt: true }), + ], + getSessionGoal: async (id) => goals[id], + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + expect(res.body.goals).toEqual([ + { + sessionId: 's2', + displayName: null, + condition: 'raise coverage', + iterations: 0, + setAt: 2000, + hasActivePrompt: true, + }, + { + sessionId: 's1', + displayName: 'fix-ci', + condition: 'fix flaky tests', + iterations: 3, + setAt: 1000, + lastReason: 'two tests still fail', + hasActivePrompt: false, + }, + ]); + }); + + it('drops a session whose probe rejects rather than failing the whole list', async () => { + vi.mocked(writeStderrLine).mockClear(); + const app = makeApp({ + listWorkspaceSessions: () => [summary('dead'), summary('alive')], + getSessionGoal: async (id) => { + if (id === 'dead') throw new Error('Session not found: dead'); + return activeGoal('keep going'); + }, + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + expect(res.body.goals).toEqual([ + { + sessionId: 'alive', + displayName: null, + condition: 'keep going', + iterations: 0, + setAt: 1000, + hasActivePrompt: false, + }, + ]); + + // An empty page and a page whose probes all failed look identical to the + // client, so the drop must not be silent. + expect(res.body.droppedCount).toBe(1); + const logged = vi.mocked(writeStderrLine).mock.calls.map((c) => c[0]); + expect(logged.join('\n')).toContain('could not probe 1 of 2 session(s)'); + expect(logged.join('\n')).toContain('dead: Session not found: dead'); + }); + + it('reports a total brownout as dropped rather than as an empty workspace', async () => { + // Without droppedCount the client cannot tell "no goals" from "we could not + // ask", and users re-create goals that are already running. + const app = makeApp({ + listWorkspaceSessions: () => [summary('a'), summary('b')], + getSessionGoal: async () => { + throw new Error('agent channel closed'); + }, + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ v: 1, goals: [], droppedCount: 2 }); + }); + + it('does not log when every probe succeeds', async () => { + vi.mocked(writeStderrLine).mockClear(); + const app = makeApp({ + listWorkspaceSessions: () => [summary('s1')], + getSessionGoal: async () => noGoal, + }); + + await request(app).get('/goals'); + + expect(writeStderrLine).not.toHaveBeenCalled(); + }); + + it('probes the live sessions concurrently, one call each', async () => { + const getSessionGoal = vi.fn(async (_sessionId: string) => noGoal); + const app = makeApp({ + listWorkspaceSessions: () => [ + summary('s1'), + summary('s2'), + summary('s3'), + ], + getSessionGoal, + }); + + await request(app).get('/goals'); + + expect(getSessionGoal).toHaveBeenCalledTimes(3); + expect(getSessionGoal.mock.calls.map((c) => c[0])).toEqual([ + 's1', + 's2', + 's3', + ]); + }); + + it('scopes the listing to the bound workspace', async () => { + const listWorkspaceSessions = vi.fn(() => []); + const app = makeApp({ + listWorkspaceSessions, + getSessionGoal: async () => noGoal, + }); + + await request(app).get('/goals'); + + expect(listWorkspaceSessions).toHaveBeenCalledWith(WORKSPACE); + }); + + it('returns 500 when enumerating sessions throws', async () => { + const app = makeApp({ + listWorkspaceSessions: () => { + throw new Error('bridge is shutting down'); + }, + getSessionGoal: async () => noGoal, + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(500); + expect(res.body.code).toBe('goals_read_failed'); + }); + + it('caps how many sessions it probes at once', async () => { + // Every probe is an IPC round-trip to a separate child process. A + // workspace with many live sessions would otherwise open one per session + // on every 10s poll from the Goals page. + const sessions = Array.from({ length: 25 }, (_, i) => summary(`s${i}`)); + let inFlight = 0; + let peak = 0; + + const app = makeApp({ + listWorkspaceSessions: () => sessions, + getSessionGoal: async (id) => { + inFlight++; + peak = Math.max(peak, inFlight); + // Yield so every probe the pool started is counted as concurrent. + await new Promise((resolve) => setTimeout(resolve, 1)); + inFlight--; + return activeGoal(`goal ${id}`); + }, + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + // All 25 are still probed — the cap bounds the burst, not the coverage. + expect(res.body.goals).toHaveLength(25); + expect(peak).toBe(10); + }); + + it('keeps a rejection attributed to the session that caused it', async () => { + // Index alignment is what lets the drop log name the bad session. A + // concurrency-limited fan-out that collects results out of order would + // silently misattribute them. + const sessions = [summary('good-1'), summary('bad'), summary('good-2')]; + const app = makeApp({ + listWorkspaceSessions: () => sessions, + getSessionGoal: async (id) => { + if (id === 'bad') throw new Error('child is wedged'); + return activeGoal(`goal ${id}`); + }, + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + expect(res.body.droppedCount).toBe(1); + expect( + res.body.goals.map((g: { sessionId: string }) => g.sessionId), + ).toEqual(expect.arrayContaining(['good-1', 'good-2'])); + expect(res.body.goals).toHaveLength(2); + }); +}); diff --git a/packages/cli/src/serve/routes/goals.ts b/packages/cli/src/serve/routes/goals.ts new file mode 100644 index 00000000000..c1b9b74df33 --- /dev/null +++ b/packages/cli/src/serve/routes/goals.ts @@ -0,0 +1,175 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Workspace-wide `/goal` listing — the daemon-side surface behind the Web Shell + * "Goals" page. + * + * A goal is a session-scoped Stop hook whose state (condition, judge turn count, + * last verdict) lives only in the `qwen --acp` child's in-memory store. The serve + * process holds no copy, so this route fans out one `sessionGoalGet` ext-method + * call per live session and collects the answers. There is no durable goal store + * to read instead: a goal only advances while its session is resident, so "the + * live sessions" IS the complete set of goals that are actually running. + * + * A session whose child is wedged or dying rejects; those are dropped (and + * logged) rather than failing the whole list, so one bad session can't hide the + * others. The per-call timeout is the bridge's, and the calls run concurrently + * (up to `PROBE_CONCURRENCY`), so a wedged child costs one timeout rather than + * one per session. + * + * Read-only: clearing a goal stays on `POST /session/:id/goal/clear`, and + * setting one stays a prompt (`/goal ` registers the hook and kicks + * off the first turn — it is not a pure write). + */ + +import type { Application } from 'express'; +import type { + BridgeSessionGoal, + BridgeSessionSummary, +} from '@qwen-code/acp-bridge'; +import { writeStderrLine } from '../../utils/stdioHelpers.js'; + +/** + * The slice of the session bridge this route needs. Narrowed to a structural + * type so tests can stub it without the full bridge. + */ +export interface GoalsSessionBridge { + listWorkspaceSessions(workspaceCwd: string): BridgeSessionSummary[]; + getSessionGoal(sessionId: string): Promise; +} + +export interface RegisterGoalsRoutesDeps { + boundWorkspace: string; + bridge: GoalsSessionBridge; +} + +/** + * Ceiling on in-flight `sessionGoalGet` probes. Each is an IPC round-trip to a + * separate child process, so a workspace with dozens of live sessions would + * otherwise open dozens at once every poll. + */ +const PROBE_CONCURRENCY = 10; + +/** + * `Promise.allSettled` over `items`, but with at most `limit` calls in flight. + * Results stay index-aligned with `items` so the caller can still name the + * session behind a rejection. + */ +async function allSettledWithLimit( + items: readonly T[], + limit: number, + fn: (item: T) => Promise, +): Promise>> { + const results = new Array>(items.length); + let next = 0; + const worker = async (): Promise => { + while (next < items.length) { + const index = next++; + try { + results[index] = { status: 'fulfilled', value: await fn(items[index]) }; + } catch (reason) { + results[index] = { status: 'rejected', reason }; + } + } + }; + // `Math.max(1, …)` so a zero limit can never leave the array sparse: the + // caller reads `outcome.status` off every index. + const workers = Math.min(Math.max(1, limit), items.length); + await Promise.all(Array.from({ length: workers }, worker)); + return results; +} + +/** One row of the Goals page. */ +interface GoalView { + sessionId: string; + /** The session's label, when it has one — otherwise the client shows the id. */ + displayName: string | null; + condition: string; + iterations: number; + setAt: number; + lastReason?: string; + /** + * The owning session is mid-turn. For a goal session that is almost always + * the loop working, but a manual prompt in the same session sets it too — so + * this reports what the daemon actually knows rather than claiming to know + * that the goal specifically is running. + */ + hasActivePrompt: boolean; +} + +export function registerGoalsRoutes( + app: Application, + deps: RegisterGoalsRoutesDeps, +): void { + const { boundWorkspace, bridge } = deps; + + app.get('/goals', async (_req, res) => { + try { + const sessions = bridge.listWorkspaceSessions(boundWorkspace); + const settled = await allSettledWithLimit( + sessions, + PROBE_CONCURRENCY, + async (session) => ({ + session, + goal: await bridge.getSessionGoal(session.sessionId), + }), + ); + + const goals: GoalView[] = []; + const dropped: string[] = []; + for (const [index, outcome] of settled.entries()) { + // A session that died between the list and the probe simply has no + // goal to report. Dropping it keeps one bad session from hiding the + // others, but do not drop it silently: an empty page and a page whose + // probes all failed look identical from the client. + if (outcome.status !== 'fulfilled') { + const sessionId = sessions[index]?.sessionId ?? '(unknown)'; + const reason = + outcome.reason instanceof Error + ? outcome.reason.message + : String(outcome.reason); + dropped.push(`${sessionId}: ${reason}`); + continue; + } + const { session, goal } = outcome.value; + if (!goal.active) continue; + goals.push({ + sessionId: session.sessionId, + displayName: session.displayName ?? null, + condition: goal.active.condition, + iterations: goal.active.iterations, + setAt: goal.active.setAt, + ...(goal.active.lastReason !== undefined + ? { lastReason: goal.active.lastReason } + : {}), + hasActivePrompt: session.hasActivePrompt, + }); + } + if (dropped.length > 0) { + writeStderrLine( + `qwen serve: GET /goals could not probe ${dropped.length} of ${sessions.length} session(s): ${dropped.join('; ')}`, + ); + } + + // Newest first, matching the scheduled-tasks page. + goals.sort((a, b) => b.setAt - a.setAt); + + // `droppedCount` lets the client tell "no goals" apart from "we could not + // ask". Without it a brownout looks like an empty workspace, and the user + // re-creates goals that are already running. + res.status(200).json({ v: 1, goals, droppedCount: dropped.length }); + } catch (err) { + writeStderrLine( + `qwen serve: GET /goals failed: ${err instanceof Error ? err.message : String(err)}`, + ); + res.status(500).json({ + error: 'Failed to list active goals', + code: 'goals_read_failed', + }); + } + }); +} diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index dc122a902c2..8574dce2b4d 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -99,6 +99,7 @@ import { registerScheduledTasksRoutes, registerWorkspaceQualifiedScheduledTasksRoutes, } from './routes/scheduled-tasks.js'; +import { registerGoalsRoutes } from './routes/goals.js'; import { registerUsageStatsRoutes } from './routes/usage-stats.js'; import { startScheduledTaskKeepalive, @@ -1538,6 +1539,14 @@ export function createServeApp( bridge: deps.manageScheduledTaskSessions ? bridge : undefined, }); + // Workspace-wide active-goal listing (the Web Shell "Goals" page). Read-only + // GET like /daemon/status: it fans out to the live sessions and reports what + // their in-memory goal stores hold. + registerGoalsRoutes(app, { + boundWorkspace: primaryBoundWorkspace, + bridge: primaryBridge, + }); + // The same CRUD surface, workspace-qualified, so a multi-workspace Web Shell // manages every registered project's schedule against that project's own cron // file (and its own session bridge) rather than always the primary's. Each diff --git a/packages/cli/src/ui/commands/goalCommand.ts b/packages/cli/src/ui/commands/goalCommand.ts index e23f2018eba..c3d157ecf7b 100644 --- a/packages/cli/src/ui/commands/goalCommand.ts +++ b/packages/cli/src/ui/commands/goalCommand.ts @@ -24,7 +24,10 @@ import { installGoalTerminalObserver } from '../utils/restoreGoal.js'; import { formatDuration } from '../utils/formatters.js'; import { t } from '../../i18n/index.js'; -// Keep in sync with GOAL_CLEAR_KEYWORDS in packages/web-shell/client/App.tsx +// Mirrored by GOAL_CLEAR_KEYWORDS in +// packages/web-shell/client/utils/goalCondition.ts, whose test reads this +// literal and fails on drift. The Web Shell client bundles for the browser and +// cannot import from core, so this is duplicated rather than shared. const CLEAR_KEYWORDS = new Set([ 'clear', 'stop', diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 846af9ab8be..e19fc9e7387 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -1917,6 +1917,9 @@ export const useGeminiStream = ( kind: 'checking', condition: activeGoal.condition, iterations: activeGoal.iterations, + // Carried so a transcript truncated past its `set` card can still + // restore the goal's original start time. + setAt: activeGoal.setAt, lastReason: activeGoal.lastReason ?? value.reasons[value.reasons.length - 1], }; diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index c4793ba809a..9a3f7f66ea1 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -582,6 +582,23 @@ export type GoalStatusKind = | 'aborted' | 'checking'; +export const GOAL_STATUS_KINDS = [ + 'set', + 'achieved', + 'cleared', + 'failed', + 'aborted', + 'checking', +] as const satisfies readonly GoalStatusKind[]; + +/** Narrows an untrusted value (e.g. a persisted transcript field). */ +export function isGoalStatusKind(value: unknown): value is GoalStatusKind { + return ( + typeof value === 'string' && + (GOAL_STATUS_KINDS as readonly string[]).includes(value) + ); +} + export const TERMINAL_GOAL_STATUS_KINDS = [ 'achieved', 'aborted', diff --git a/packages/cli/src/ui/utils/export/collect.test.ts b/packages/cli/src/ui/utils/export/collect.test.ts index 7cc194128dd..13a5a9cdd09 100644 --- a/packages/cli/src/ui/utils/export/collect.test.ts +++ b/packages/cli/src/ui/utils/export/collect.test.ts @@ -120,6 +120,42 @@ describe('collectSessionData', () => { expect(data.messages[0]?.message?.parts?.[0]?.text).toBe('hello'); }); + it('exports a session whose transcript ends on an active goal', async () => { + // The daemon export config is a Proxy that throws on any method it does not + // implement, and it implements none of the /goal trust gates. Anything the + // replayer asks of `config` beyond that shape takes the whole export down. + const minimalConfig: ExportConfig = { getChannel: () => 'daemon' }; + + const data = await collectSessionData( + { + sessionId: 'session-goal', + startTime: '2025-01-01T00:00:00.000Z', + messages: [ + { + uuid: 'goal-1', + parentUuid: null, + sessionId: 'session-goal', + timestamp: '2025-01-01T00:00:00.000Z', + type: 'system', + subtype: 'slash_command', + cwd: '', + version: '1.0.0', + systemPayload: { + phase: 'result', + rawCommand: '/goal', + outputHistoryItems: [ + { type: 'goal_status', kind: 'set', condition: 'ship it' }, + ], + }, + } as unknown as ChatRecord, + ], + }, + minimalConfig, + ); + + expect(data.metadata?.channel).toBe('daemon'); + }); + it('replays tool calls when daemon export config has no tool registry', async () => { const minimalConfig: ExportConfig = {}; diff --git a/packages/cli/src/ui/utils/restoreGoal.test.ts b/packages/cli/src/ui/utils/restoreGoal.test.ts index 2bea5495084..ff8dcd6d7ba 100644 --- a/packages/cli/src/ui/utils/restoreGoal.test.ts +++ b/packages/cli/src/ui/utils/restoreGoal.test.ts @@ -5,19 +5,27 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { MockInstance } from 'vitest'; import { __resetActiveGoalStoreForTests, getActiveGoal, getLastGoalTerminal, notifyGoalTerminal, setActiveGoal, + setGoalTerminalObserver, + type ChatRecord, type Config, } from '@qwen-code/qwen-code-core'; import type { HistoryItem } from '../types.js'; import { + collectGoalStatusItemsFromRecords, findGoalToRestore, findLastTerminalGoal, + goalTerminalEventToHistoryItem, + parseGoalStatusItem, + recordGoalStatusItem, restoreGoalFromHistory, + type GoalStatusItem, } from './restoreGoal.js'; const goalItem = ( @@ -172,7 +180,10 @@ describe('restoreGoalFromHistory', () => { [goalItem({ kind: 'set', condition: 'do x' })], cfg, ); - expect(result).toEqual({ restored: false }); + expect(result).toEqual({ + restored: false, + blockedBy: 'untrusted-folder', + }); expect(getActiveGoal('sess-1')).toBeUndefined(); }); @@ -184,7 +195,7 @@ describe('restoreGoalFromHistory', () => { [goalItem({ kind: 'set', condition: 'do x' })], cfg, ); - expect(result).toEqual({ restored: false }); + expect(result).toEqual({ restored: false, blockedBy: 'hooks-disabled' }); }); it('skips restore when hook system is unavailable', () => { @@ -195,7 +206,7 @@ describe('restoreGoalFromHistory', () => { [goalItem({ kind: 'set', condition: 'do x' })], cfg, ); - expect(result).toEqual({ restored: false }); + expect(result).toEqual({ restored: false, blockedBy: 'no-hook-system' }); }); it('rehydrates the last completed goal cache from history on resume', () => { @@ -271,6 +282,35 @@ describe('restoreGoalFromHistory', () => { ], }); }); + + it.each([ + ['an active goal is restored', 'checking' as const], + ['there is no goal to restore', 'achieved' as const], + ])( + 'tears down an existing terminal observer when %s and no addItem is given', + (_label, kind) => { + // The ACP path calls restore without `addItem` and relies on this: every + // exit re-enters `unregisterGoalHook`, which clears the observer table. + // `acpAgent.#restoreGoalOnResume` reinstalls the Session's observer + // afterwards. If that ever stops being true, a restored goal reaches its + // terminal state with nobody listening — this pins the reason why. + const observer = vi.fn(); + setGoalTerminalObserver('sess-1', observer); + + restoreGoalFromHistory( + [goalItem({ kind, condition: 'do x' })], + makeConfig(), + ); + + notifyGoalTerminal('sess-1', { + kind: 'achieved', + condition: 'do x', + iterations: 1, + durationMs: 10, + }); + expect(observer).not.toHaveBeenCalled(); + }, + ); }); describe('findLastTerminalGoal', () => { @@ -324,3 +364,498 @@ describe('findLastTerminalGoal', () => { }); }); }); + +const slashCommandRecord = ( + outputHistoryItems: Array>, + phase: 'invocation' | 'result' = 'result', +): ChatRecord => + ({ + uuid: 'rec-1', + parentUuid: null, + sessionId: 'sess-1', + timestamp: new Date(0).toISOString(), + type: 'system', + subtype: 'slash_command', + cwd: '/w', + version: '1.0.0', + systemPayload: { phase, rawCommand: '/goal', outputHistoryItems }, + }) as unknown as ChatRecord; + +describe('parseGoalStatusItem', () => { + it('rebuilds a goal card, dropping absent optional fields', () => { + expect( + parseGoalStatusItem({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 42, + }), + ).toEqual({ + type: 'goal_status', + kind: 'set', + condition: 'ship it', + setAt: 42, + }); + }); + + it('keeps iterations, durationMs and lastReason when present', () => { + expect( + parseGoalStatusItem({ + type: 'goal_status', + kind: 'achieved', + condition: 'ship it', + iterations: 3, + durationMs: 1000, + lastReason: 'tests pass', + }), + ).toEqual({ + type: 'goal_status', + kind: 'achieved', + condition: 'ship it', + iterations: 3, + durationMs: 1000, + lastReason: 'tests pass', + }); + }); + + it('returns null for non-goal items', () => { + expect(parseGoalStatusItem({ type: 'assistant', text: 'hi' })).toBeNull(); + }); + + it('returns null for an unknown kind', () => { + expect( + parseGoalStatusItem({ + type: 'goal_status', + kind: 'bogus', + condition: 'x', + }), + ).toBeNull(); + }); + + it('returns null when condition is missing or not a string', () => { + expect( + parseGoalStatusItem({ type: 'goal_status', kind: 'set' }), + ).toBeNull(); + expect( + parseGoalStatusItem({ type: 'goal_status', kind: 'set', condition: 7 }), + ).toBeNull(); + }); + + it('drops non-finite numeric fields rather than propagating NaN', () => { + expect( + parseGoalStatusItem({ + type: 'goal_status', + kind: 'set', + condition: 'x', + setAt: Number.NaN, + iterations: '3', + }), + ).toEqual({ type: 'goal_status', kind: 'set', condition: 'x' }); + }); +}); + +describe('collectGoalStatusItemsFromRecords', () => { + it('collects goal cards from slash_command result records, oldest first', () => { + const items = collectGoalStatusItemsFromRecords([ + slashCommandRecord([ + { type: 'goal_status', kind: 'set', condition: 'goal A' }, + ]), + slashCommandRecord([ + { type: 'assistant', text: 'chatter' }, + { + type: 'goal_status', + kind: 'checking', + condition: 'goal A', + iterations: 2, + }, + ]), + ]); + expect(items.map((i) => i.kind)).toEqual(['set', 'checking']); + expect(items[1]).toMatchObject({ condition: 'goal A', iterations: 2 }); + }); + + it('ignores invocation-phase records', () => { + expect( + collectGoalStatusItemsFromRecords([ + slashCommandRecord( + [{ type: 'goal_status', kind: 'set', condition: 'goal A' }], + 'invocation', + ), + ]), + ).toEqual([]); + }); + + it('ignores non-slash_command system records and other record types', () => { + const compression = { + ...slashCommandRecord([]), + subtype: 'chat_compression', + } as ChatRecord; + const user = { ...slashCommandRecord([]), type: 'user' } as ChatRecord; + expect(collectGoalStatusItemsFromRecords([compression, user])).toEqual([]); + }); + + it('feeds findGoalToRestore so a daemon transcript restores its iteration count', () => { + const items = collectGoalStatusItemsFromRecords([ + slashCommandRecord([ + { type: 'goal_status', kind: 'set', condition: 'goal A' }, + ]), + slashCommandRecord([ + { + type: 'goal_status', + kind: 'checking', + condition: 'goal A', + iterations: 4, + }, + ]), + ]); + expect(findGoalToRestore(items)).toEqual({ + condition: 'goal A', + iterations: 4, + }); + }); + + it('yields no restorable goal once the transcript records a terminal card', () => { + const items = collectGoalStatusItemsFromRecords([ + slashCommandRecord([ + { type: 'goal_status', kind: 'set', condition: 'goal A' }, + ]), + slashCommandRecord([ + { + type: 'goal_status', + kind: 'achieved', + condition: 'goal A', + iterations: 2, + durationMs: 500, + }, + ]), + ]); + expect(findGoalToRestore(items)).toBeNull(); + expect(findLastTerminalGoal(items)).toMatchObject({ + kind: 'achieved', + condition: 'goal A', + }); + }); +}); + +describe('restoreGoalFromHistory has no condition cap', () => { + beforeEach(() => __resetActiveGoalStoreForTests()); + afterEach(() => __resetActiveGoalStoreForTests()); + + it('restores a condition far longer than the old 4,000-char cap', () => { + // `/goal` accepts a condition of any length (#6665). A cap here would + // refuse, on reload, a goal the user legitimately set — and the replay + // drops the card too, so they would never see why it vanished. + const cfg = makeConfig(); + const condition = 'x'.repeat(10_000); + expect(restoreGoalFromHistory([goalItem({ condition })], cfg)).toEqual({ + restored: true, + condition, + }); + expect(getActiveGoal('sess-1')).toMatchObject({ condition }); + }); + + it('drops a stale in-memory goal when the transcript condition is empty', () => { + setActiveGoal('sess-1', { + condition: 'stale goal', + iterations: 0, + setAt: 100, + tokensAtStart: 0, + hookId: 'stale-hook', + }); + const cfg = makeConfig(); + restoreGoalFromHistory([goalItem({ condition: '' })], cfg); + expect(getActiveGoal('sess-1')).toBeUndefined(); + }); +}); + +describe('goalTerminalEventToHistoryItem', () => { + it('keeps lastReason when the judge produced one', () => { + expect( + goalTerminalEventToHistoryItem({ + kind: 'achieved', + condition: 'ship it', + iterations: 2, + durationMs: 900, + lastReason: 'tests pass', + }), + ).toMatchObject({ kind: 'achieved', lastReason: 'tests pass' }); + }); + + it('falls back to systemMessage when the judge never ran', () => { + // `aborted` events carry the cap message in systemMessage, not lastReason. + expect( + goalTerminalEventToHistoryItem({ + kind: 'aborted', + condition: 'ship it', + iterations: 50, + durationMs: 900, + systemMessage: 'Goal max iterations reached; cleared.', + }), + ).toMatchObject({ + kind: 'aborted', + lastReason: 'Goal max iterations reached; cleared.', + }); + }); + + it('prefers lastReason over systemMessage when both are present', () => { + // Known lossy collapse: HistoryItemGoalStatus has no systemMessage field. + expect( + goalTerminalEventToHistoryItem({ + kind: 'aborted', + condition: 'ship it', + iterations: 50, + durationMs: 900, + lastReason: 'two tests still fail', + systemMessage: 'Goal max iterations reached; cleared.', + }).lastReason, + ).toBe('two tests still fail'); + }); +}); + +describe('parseGoalStatusItem keeps refusable cards so ordering survives', () => { + beforeEach(() => __resetActiveGoalStoreForTests()); + afterEach(() => __resetActiveGoalStoreForTests()); + + it('parses a card whose condition is empty rather than dropping it', () => { + // Rejecting at parse time looks like a tidy shared gate, but the scanners + // below decide on the LAST goal card. Dropping one silently promotes the + // card before it. + expect( + parseGoalStatusItem({ + type: 'goal_status', + kind: 'cleared', + condition: '', + }), + ).toMatchObject({ kind: 'cleared' }); + }); + + it('lets a card with an empty condition still cancel an earlier goal', () => { + // If parse dropped the `cleared` card, findGoalToRestore would walk past it + // to `set` and resurrect a goal the user explicitly cleared — the exact bug + // persisting `cleared` exists to prevent. + const items = collectGoalStatusItemsFromRecords([ + slashCommandRecord([ + { type: 'goal_status', kind: 'set', condition: 'goal A' }, + ]), + slashCommandRecord([ + { type: 'goal_status', kind: 'cleared', condition: '' }, + ]), + ]); + + expect(findGoalToRestore(items)).toBeNull(); + expect(restoreGoalFromHistory(items, makeConfig())).toEqual({ + restored: false, + }); + expect(getActiveGoal('sess-1')).toBeUndefined(); + }); + + it('fails closed on an empty set card instead of restoring an older goal', () => { + const items = collectGoalStatusItemsFromRecords([ + slashCommandRecord([ + { type: 'goal_status', kind: 'set', condition: 'goal A' }, + ]), + slashCommandRecord([{ type: 'goal_status', kind: 'set', condition: '' }]), + ]); + + // The newest card wins the scan, and the empty gate then refuses it. Goal + // A must NOT come back to life. + expect(findGoalToRestore(items)?.condition).toBe(''); + expect(restoreGoalFromHistory(items, makeConfig())).toEqual({ + restored: false, + blockedBy: 'condition-invalid', + }); + expect(getActiveGoal('sess-1')).toBeUndefined(); + }); +}); + +describe('transcript payloads are untrusted', () => { + // A transcript is a file on disk. Anything in it may have been hand-edited, + // truncated, or written by an older version. A throw here is not contained: + // `#restoreGoalOnResume` catches it and skips the hook, leaving a replayed + // `set` card on screen with nothing driving it. + + it.each([ + ['null', null], + ['undefined', undefined], + ['an array', []], + ['a string', 'goal_status'], + ['a number', 7], + ])('parseGoalStatusItem returns null for %s', (_label, value) => { + expect(parseGoalStatusItem(value)).toBeNull(); + }); + + it('collectGoalStatusItemsFromRecords skips a non-array outputHistoryItems', () => { + const record = { + type: 'system', + subtype: 'slash_command', + systemPayload: { + phase: 'result', + // A plain object, not an array: `for..of` would throw. + outputHistoryItems: { type: 'goal_status', kind: 'set' }, + }, + } as unknown as ChatRecord; + expect(collectGoalStatusItemsFromRecords([record])).toEqual([]); + }); + + it('collectGoalStatusItemsFromRecords skips null entries and keeps later valid cards', () => { + const record = { + type: 'system', + subtype: 'slash_command', + systemPayload: { + phase: 'result', + outputHistoryItems: [ + null, + 'not an object', + { type: 'goal_status', kind: 'set', condition: 'survives' }, + ], + }, + } as unknown as ChatRecord; + expect(collectGoalStatusItemsFromRecords([record])).toEqual([ + { type: 'goal_status', kind: 'set', condition: 'survives' }, + ]); + }); +}); + +describe('restoreGoalFromHistory carries the original start time', () => { + beforeEach(() => __resetActiveGoalStoreForTests()); + afterEach(() => __resetActiveGoalStoreForTests()); + + it('restores setAt from the set card rather than restarting the clock', () => { + const cfg = makeConfig(); + restoreGoalFromHistory( + [goalItem({ kind: 'set', condition: 'do x', setAt: 1000 })], + cfg, + ); + expect(getActiveGoal('sess-1')).toMatchObject({ setAt: 1000 }); + }); + + it('finds setAt on the set card when the newest card is a checking card', () => { + // `checking` cards written before this change carry no setAt at all, so the + // scan has to walk back to the `set` card that opened the run. + const cfg = makeConfig(); + restoreGoalFromHistory( + [ + goalItem({ kind: 'set', condition: 'do x', setAt: 1000 }), + userItem(), + goalItem({ kind: 'checking', condition: 'do x', iterations: 3 }), + ], + cfg, + ); + expect(getActiveGoal('sess-1')).toMatchObject({ + setAt: 1000, + iterations: 3, + }); + }); + + it('does not borrow setAt from a previous, already-finished goal', () => { + const cfg = makeConfig(); + const now = Date.now(); + restoreGoalFromHistory( + [ + goalItem({ kind: 'set', condition: 'goal A', setAt: 1000 }), + goalItem({ kind: 'achieved', condition: 'goal A', durationMs: 5 }), + // Goal B's own `set` card is gone (truncated transcript). + goalItem({ kind: 'checking', condition: 'goal B', iterations: 1 }), + ], + cfg, + ); + const goal = getActiveGoal('sess-1'); + expect(goal).toMatchObject({ condition: 'goal B' }); + expect(goal!.setAt).not.toBe(1000); + expect(goal!.setAt).toBeGreaterThanOrEqual(now); + }); + + it('does not borrow setAt from a previous goal that has no terminal card', () => { + // The run-boundary scan cannot rely on a terminal card being there: a + // truncated or hand-edited transcript can put two goals back to back. The + // condition is what identifies the run, so goal B must not inherit goal A's + // clock just because nothing separates them. + const cfg = makeConfig(); + const now = Date.now(); + restoreGoalFromHistory( + [ + goalItem({ kind: 'set', condition: 'goal A', setAt: 1000 }), + goalItem({ kind: 'checking', condition: 'goal A', iterations: 2 }), + // Goal B's own `set` card survived but lost its setAt, and no terminal + // card was ever written for goal A. + goalItem({ kind: 'set', condition: 'goal B' }), + goalItem({ kind: 'checking', condition: 'goal B', iterations: 1 }), + ], + cfg, + ); + const goal = getActiveGoal('sess-1'); + expect(goal).toMatchObject({ condition: 'goal B' }); + expect(goal!.setAt).not.toBe(1000); + expect(goal!.setAt).toBeGreaterThanOrEqual(now); + }); + + it('ignores a non-positive setAt from a corrupted transcript', () => { + const cfg = makeConfig(); + const now = Date.now(); + restoreGoalFromHistory( + [goalItem({ kind: 'set', condition: 'do x', setAt: 0 })], + cfg, + ); + expect(getActiveGoal('sess-1')!.setAt).toBeGreaterThanOrEqual(now); + }); +}); + +describe('restoreGoalFromHistory refuses an empty condition', () => { + beforeEach(() => __resetActiveGoalStoreForTests()); + afterEach(() => __resetActiveGoalStoreForTests()); + + it('does not register a hook for a blank condition', () => { + // `/goal` never sets one — a bare `/goal` reports status. Only a corrupted + // transcript gets here, and a blank condition makes every judge call ask + // the model to check nothing. + const cfg = makeConfig(); + expect( + restoreGoalFromHistory([goalItem({ kind: 'set', condition: '' })], cfg), + ).toEqual({ restored: false, blockedBy: 'condition-invalid' }); + expect(getActiveGoal('sess-1')).toBeUndefined(); + }); +}); + +describe('recordGoalStatusItem', () => { + let stderr: MockInstance; + beforeEach(() => { + stderr = vi + .spyOn(process.stderr, 'write') + .mockReturnValue(true) as MockInstance; + }); + afterEach(() => stderr.mockRestore()); + + const item = { + type: 'goal_status', + kind: 'set', + condition: 'do x', + } as GoalStatusItem; + + it('warns when there is no chat recording service to persist the card', () => { + // Optional chaining used to swallow this: the goal then works for the rest + // of the session and silently fails to come back on resume. + recordGoalStatusItem( + makeConfig({ + getChatRecordingService: vi.fn().mockReturnValue(undefined), + } as unknown as Partial), + item, + ); + expect(stderr).toHaveBeenCalledWith( + expect.stringContaining('no chat recording service'), + ); + }); + + it('warns but does not throw when the recording write fails', () => { + const cfg = makeConfig({ + getChatRecordingService: vi.fn().mockReturnValue({ + recordSlashCommand: vi.fn().mockImplementation(() => { + throw new Error('disk full'); + }), + }), + } as unknown as Partial); + expect(() => recordGoalStatusItem(cfg, item)).not.toThrow(); + expect(stderr).toHaveBeenCalledWith( + expect.stringContaining('failed to record goal_status'), + ); + }); +}); diff --git a/packages/cli/src/ui/utils/restoreGoal.ts b/packages/cli/src/ui/utils/restoreGoal.ts index ab4afe3b861..cbe7f662856 100644 --- a/packages/cli/src/ui/utils/restoreGoal.ts +++ b/packages/cli/src/ui/utils/restoreGoal.ts @@ -9,16 +9,27 @@ import { setGoalTerminalObserver, setLastGoalTerminal, unregisterGoalHook, + type ChatRecord, type Config, type GoalTerminalEvent, type GoalTerminalKind, + type SlashCommandRecordPayload, } from '@qwen-code/qwen-code-core'; import { + isGoalStatusKind, isTerminalGoalStatusKind, MessageType, - type HistoryItem, type HistoryItemGoalStatus, + type HistoryItemWithoutId, } from '../types.js'; +import { writeStderrLineSafe } from '../../utils/stdioHelpers.js'; + +export interface RestorableGoal { + condition: string; + iterations: number; + /** Absent when no card of this goal's run carried one. */ + setAt?: number; +} /** * Finds the most recent `goal_status` history item. Returns the active @@ -30,22 +41,58 @@ import { * resume instead of resetting to zero. `checking` items persist the running * count (see useGeminiStream's continuation handler); `set` items predate any * iteration, so they restore at 0. + * + * `setAt` is carried so elapsed time keeps measuring from the original `/goal`. + * The newest card is not necessarily the one that has it — only `set` cards are + * written with a `setAt` — so we keep scanning back through this same run's + * cards for it, stopping at the terminal card that ends the previous run. */ export function findGoalToRestore( - history: HistoryItem[], -): { condition: string; iterations: number } | null { + history: readonly HistoryItemWithoutId[], +): RestorableGoal | null { for (let i = history.length - 1; i >= 0; i--) { const item = history[i]; if (item?.type !== MessageType.GOAL_STATUS) continue; const goal = item as HistoryItemGoalStatus; - if (goal.kind === 'set' || goal.kind === 'checking') { - return { condition: goal.condition, iterations: goal.iterations ?? 0 }; - } - return null; + if (goal.kind !== 'set' && goal.kind !== 'checking') return null; + const setAt = goal.setAt ?? findSetAtOfRun(history, i); + return { + condition: goal.condition, + iterations: goal.iterations ?? 0, + ...(setAt !== undefined ? { setAt } : {}), + }; } return null; } +/** + * Walks back from the active goal card at `startIndex` for the `setAt` stamped + * on the `set` card that opened this run. + * + * A run ends at any card that is not `set`/`checking` — but that alone is not + * enough to stay inside it. A transcript is a file: two goals can sit back to + * back with no terminal card between them (hand-edited, truncated, or written + * by a version that did not persist terminal cards). The condition is what + * actually identifies the run, so the scan stops as soon as it changes rather + * than walking into the previous goal and returning *its* start time. + */ +function findSetAtOfRun( + history: readonly HistoryItemWithoutId[], + startIndex: number, +): number | undefined { + const start = history[startIndex] as HistoryItemGoalStatus | undefined; + const condition = start?.condition; + for (let i = startIndex - 1; i >= 0; i--) { + const item = history[i]; + if (item?.type !== MessageType.GOAL_STATUS) continue; + const goal = item as HistoryItemGoalStatus; + if (goal.kind !== 'set' && goal.kind !== 'checking') return undefined; + if (goal.condition !== condition) return undefined; + if (goal.setAt !== undefined) return goal.setAt; + } + return undefined; +} + /** * Finds the most recent terminal (achieved / failed / aborted) goal_status item in * the transcript. Sentinel-style entries (`set`, `cleared`, `checking`) are @@ -55,7 +102,7 @@ export function findGoalToRestore( * goal" cache so empty `/goal` after a reload still shows the summary card. */ export function findLastTerminalGoal( - history: HistoryItem[], + history: readonly HistoryItemWithoutId[], ): GoalTerminalEvent | null { for (let i = history.length - 1; i >= 0; i--) { const item = history[i]; @@ -73,9 +120,89 @@ export function findLastTerminalGoal( return null; } -type GoalStatusItem = Omit; +export type GoalStatusItem = Omit; type AddGoalStatusItem = (item: GoalStatusItem, timestamp: number) => void; +function finiteNumber(value: unknown): number | undefined { + return typeof value === 'number' && Number.isFinite(value) + ? value + : undefined; +} + +/** + * Narrows one untrusted `outputHistoryItems` entry before any field is read. + * A transcript is a file: an entry may be any JSON value, and only a plain + * object is safely indexable. + */ +export function isTranscriptItemRecord( + item: unknown, +): item is Record { + return typeof item === 'object' && item !== null && !Array.isArray(item); +} + +/** + * Rebuilds a goal card from one persisted `outputHistoryItems` entry, or + * returns null when the entry is not a well-formed goal card. Transcripts are + * files on disk: an entry may be any JSON value at all — including `null` or an + * array — so the shape is checked before any field is read, and then every + * field is re-validated rather than cast. + */ +export function parseGoalStatusItem(item: unknown): GoalStatusItem | null { + if (!isTranscriptItemRecord(item)) return null; + if (item['type'] !== MessageType.GOAL_STATUS) return null; + const kind = item['kind']; + const condition = item['condition']; + if (!isGoalStatusKind(kind) || typeof condition !== 'string') return null; + + const iterations = finiteNumber(item['iterations']); + const setAt = finiteNumber(item['setAt']); + const durationMs = finiteNumber(item['durationMs']); + const lastReason = + typeof item['lastReason'] === 'string' ? item['lastReason'] : undefined; + + return { + type: MessageType.GOAL_STATUS, + kind, + condition, + ...(iterations !== undefined ? { iterations } : {}), + ...(setAt !== undefined ? { setAt } : {}), + ...(durationMs !== undefined ? { durationMs } : {}), + ...(lastReason !== undefined ? { lastReason } : {}), + }; +} + +/** + * Extracts the goal cards a transcript persisted inside its `system` / + * `slash_command` records, oldest first. This is the daemon-side counterpart to + * the TUI's in-memory `HistoryItem[]`: on the ACP path no `HistoryItem[]` ever + * exists, so `findGoalToRestore` / `findLastTerminalGoal` are fed from here. + */ +export function collectGoalStatusItemsFromRecords( + records: readonly ChatRecord[], +): GoalStatusItem[] { + const items: GoalStatusItem[] = []; + for (const record of records) { + if (record.type !== 'system' || record.subtype !== 'slash_command') { + continue; + } + const payload = record.systemPayload as + | SlashCommandRecordPayload + | undefined; + if (payload?.phase !== 'result') continue; + // The type says `outputHistoryItems?: Record[]`, but the + // value came off disk. A hand-edited record that made it a plain object + // would throw here and take the whole restore down with it — including the + // valid goal cards further along. + const raws: unknown = payload.outputHistoryItems; + if (!Array.isArray(raws)) continue; + for (const raw of raws) { + const item = parseGoalStatusItem(raw); + if (item) items.push(item); + } + } + return items; +} + export function goalTerminalEventToHistoryItem( event: GoalTerminalEvent, ): GoalStatusItem { @@ -95,14 +222,31 @@ export function recordGoalStatusItem( rawCommand = '/goal', ): void { try { - config.getChatRecordingService?.()?.recordSlashCommand({ + const recording = config.getChatRecordingService?.(); + if (!recording) { + // Optional chaining used to swallow this. A goal set without a recording + // service works for the rest of the session and then vanishes on resume, + // which is indistinguishable from the restore bug this module fixes. + writeStderrLineSafe( + `qwen: no chat recording service; goal_status (kind=${item.kind}) will not survive a resume.`, + ); + return; + } + recording.recordSlashCommand({ phase: 'result', rawCommand, outputHistoryItems: [{ ...item } as Record], }); - } catch { + } catch (error) { // Recording is best-effort; the live goal loop must not fail because the - // session transcript could not be appended. + // session transcript could not be appended. But swallowing it silently is + // how a goal ends up unrecoverable on resume — the failure mode this + // recording exists to prevent — so leave a trace. + // Not debugLogger: that no-ops unless a debug session is active, and a + // lost write here is invisible until the goal fails to survive a resume. + writeStderrLineSafe( + `qwen: failed to record goal_status (kind=${item.kind}): ${error}`, + ); } } @@ -119,19 +263,69 @@ export function installGoalTerminalObserver(args: { }); } +/** + * Why a transcript's active goal could not be put back under a live Stop hook. + * `condition-invalid` covers a transcript that no longer describes a goal + * `/goal` itself would accept. + */ +export type GoalRestoreBlockedReason = + | 'untrusted-folder' + | 'hooks-disabled' + | 'no-hook-system' + | 'condition-invalid'; + +/** + * The environment half of `/goal`'s gates, as a pure function of `config`. + * + * Split out so the history replay can ask the question *before* restore runs: + * a client derives "there is an active goal" from the newest replayed goal + * card, so a card that is about to be refused must not be replayed as active. + */ +export function goalRestoreBlockedBy( + config: Config, +): Exclude | null { + if (!config.isTrustedFolder()) return 'untrusted-folder'; + if (config.getDisableAllHooks()) return 'hooks-disabled'; + if (!config.getHookSystem()) return 'no-hook-system'; + return null; +} + +/** + * Mirrors the gates `/goal` applies to a condition at set time. + * + * There is deliberately no length cap: #6665 removed the one `/goal` had, so + * capping here would silently destroy a long goal the user legitimately set — + * refused on restore, and dropped from the replay so they never see why. + */ +export function goalConditionBlockedBy( + condition: string, +): 'condition-invalid' | null { + if (condition.length === 0) return 'condition-invalid'; + return null; +} + +export type RestoreGoalResult = + | { restored: true; condition: string } + | { restored: false; blockedBy?: GoalRestoreBlockedReason }; + /** * On session resume, restores the active /goal hook if the transcript ended * with an unsatisfied goal. Idempotent — safe to call on a fresh session. * - * Re-runs the same trust/policy gates as `/goal`; if a gate now fails, we - * silently skip restoration rather than re-register a goal the user can no - * longer cancel. + * Re-runs the same trust/policy/length gates as `/goal`; if a gate now fails, + * we skip restoration rather than re-register a goal the user can no longer + * cancel. That case reports `blockedBy`, which callers must not confuse with + * "the transcript had no goal": the transcript still shows one as active, so + * something has to say otherwise. + * + * Note that every `{ restored: false }` path unregisters, which clears the + * session's goal-terminal observer as a side effect. ACP callers reinstall it. */ export function restoreGoalFromHistory( - history: HistoryItem[], + history: readonly HistoryItemWithoutId[], config: Config, addItem?: AddGoalStatusItem, -): { restored: true; condition: string } | { restored: false } { +): RestoreGoalResult { const sessionId = config.getSessionId(); // Always rehydrate the "last completed goal" cache from transcript so empty // `/goal` after resume can render the most recent achievement summary. @@ -148,13 +342,28 @@ export function restoreGoalFromHistory( return { restored: false }; } - if (!config.isTrustedFolder() || config.getDisableAllHooks()) { + const blockedBy = goalRestoreBlockedBy(config); + if (blockedBy) { unregisterGoalHook(config, sessionId); - return { restored: false }; + return { restored: false, blockedBy }; } - if (!config.getHookSystem()) { + // `/goal` gates the condition at set time, but a transcript is a file: a + // corrupted or hand-edited `condition` would otherwise be re-registered — + // empty and meaningless — and then embedded verbatim in every judge call and + // continuation prompt for the rest of the session. + // + // This is the only blocked path that reports itself. The env gates above stay + // silent because their caller knows the policy and says so; a malformed + // condition is known only here, and three of the four callers (the TUI ones) + // discard the result entirely, so saying nothing would lose it completely. + // ACP's `#restoreGoalOnResume` skips its own line for this reason, so exactly + // one line is written either way. + if (goalConditionBlockedBy(restorable.condition)) { + writeStderrLineSafe( + `qwen: refusing to restore a goal for session ${sessionId}: the condition is empty.`, + ); unregisterGoalHook(config, sessionId); - return { restored: false }; + return { restored: false, blockedBy: 'condition-invalid' }; } registerGoalHook({ @@ -165,6 +374,11 @@ export function restoreGoalFromHistory( // Resume the iteration count so MAX_GOAL_ITERATIONS is a cross-resume cap, // not a per-resume one. initialIterations: restorable.iterations, + // Likewise the start time: without it every reload restarts the clock, and + // `GET /goals` reports a long-running goal as freshly started. + ...(restorable.setAt !== undefined + ? { initialSetAt: restorable.setAt } + : {}), }); if (addItem) { installGoalTerminalObserver({ sessionId, config, addItem }); diff --git a/packages/cli/src/utils/stdioHelpers.test.ts b/packages/cli/src/utils/stdioHelpers.test.ts new file mode 100644 index 00000000000..db30c6c5831 --- /dev/null +++ b/packages/cli/src/utils/stdioHelpers.test.ts @@ -0,0 +1,52 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { writeStderrLine, writeStderrLineSafe } from './stdioHelpers.js'; + +afterEach(() => vi.restoreAllMocks()); + +describe('writeStderrLine', () => { + it('appends a newline, but not a second one', () => { + const write = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + + writeStderrLine('plain'); + writeStderrLine('already\n'); + + expect(write).toHaveBeenNthCalledWith(1, 'plain\n'); + expect(write).toHaveBeenNthCalledWith(2, 'already\n'); + }); + + it('propagates a write failure', () => { + // The default on purpose: most of the CLI wants a broken stderr to be loud. + vi.spyOn(process.stderr, 'write').mockImplementation(() => { + throw Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }); + }); + + expect(() => writeStderrLine('boom')).toThrow('write EPIPE'); + }); +}); + +describe('writeStderrLineSafe', () => { + it('writes exactly like writeStderrLine when stderr is healthy', () => { + const write = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + + writeStderrLineSafe('hello'); + + expect(write).toHaveBeenCalledWith('hello\n'); + }); + + it('swallows EPIPE instead of taking the caller down with it', () => { + // `qwen … | head`, or a daemon whose stderr reader went away. Callers use + // this where the write is incidental and a throw would destroy real work — + // abandoning a transcript replay over a failed diagnostic, say. + vi.spyOn(process.stderr, 'write').mockImplementation(() => { + throw Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }); + }); + + expect(() => writeStderrLineSafe('boom')).not.toThrow(); + }); +}); diff --git a/packages/cli/src/utils/stdioHelpers.ts b/packages/cli/src/utils/stdioHelpers.ts index ca5e30f9ebd..d0c88ca8a4c 100644 --- a/packages/cli/src/utils/stdioHelpers.ts +++ b/packages/cli/src/utils/stdioHelpers.ts @@ -32,6 +32,25 @@ export const writeStderrLine = (message: string): void => { process.stderr.write(message.endsWith('\n') ? message : `${message}\n`); }; +/** + * `writeStderrLine` that cannot throw. + * + * `process.stderr.write` throws on EPIPE or a closed fd — reachable whenever + * the reader goes away (`qwen … | head`) or a daemon redirects its stderr. Most + * of the CLI *wants* that to be loud, so this is not the default. + * + * Use it only where the write is incidental to the work in hand and failing it + * would destroy something real: a diagnostic emitted mid-way through replaying + * a transcript, say, where a throw would abandon the remaining records. + */ +export const writeStderrLineSafe = (message: string): void => { + try { + writeStderrLine(message); + } catch { + // stderr is gone. There is, definitionally, nowhere to report that. + } +}; + /** * Clears the terminal screen. * Use instead of console.clear() to satisfy no-console lint rules. diff --git a/packages/core/src/goals/goalHook.test.ts b/packages/core/src/goals/goalHook.test.ts index db9070c1eea..57347aa80c0 100644 --- a/packages/core/src/goals/goalHook.test.ts +++ b/packages/core/src/goals/goalHook.test.ts @@ -707,6 +707,44 @@ describe('registerGoalHook / unregisterGoalHook', () => { expect(goal.iterations).toBe(0); }); + it.each([ + ['a future timestamp', () => Date.now() + 86_400_000], + ['NaN', () => Number.NaN], + ['Infinity', () => Number.POSITIVE_INFINITY], + ['zero', () => 0], + ['a negative timestamp', () => -1], + ])( + 'ignores an unusable initialSetAt (%s) and starts the clock now', + (_label, make) => { + // `setAt` arrives from a transcript, and every duration downstream is + // `Date.now() - setAt`. A future value would render negative elapsed times on + // the Goals page and in `GET /goals` rather than fail loudly, so it falls back + // to now along with the non-finite and non-positive cases. + const before = Date.now(); + const goal = registerGoalHook({ + config, + sessionId: 'sess-1', + condition: 'tests pass', + tokensAtStart: 0, + initialSetAt: make(), + }); + expect(goal.setAt).toBeGreaterThanOrEqual(before); + expect(goal.setAt).toBeLessThanOrEqual(Date.now()); + }, + ); + + it('carries a usable initialSetAt so elapsed time survives resume', () => { + const setAt = Date.now() - 60_000; + const goal = registerGoalHook({ + config, + sessionId: 'sess-1', + condition: 'tests pass', + tokensAtStart: 0, + initialSetAt: setAt, + }); + expect(goal.setAt).toBe(setAt); + }); + it('honors a resumed near-cap count so MAX survives resume (no fresh budget)', async () => { // Simulate resume re-arming a goal that was already at the cap last session. registerGoalHook({ diff --git a/packages/core/src/goals/goalHook.ts b/packages/core/src/goals/goalHook.ts index 1905c5beda6..43ccbf8d76e 100644 --- a/packages/core/src/goals/goalHook.ts +++ b/packages/core/src/goals/goalHook.ts @@ -299,6 +299,13 @@ export function registerGoalHook(args: { * every resume). Defaults to 0 for a freshly set goal. */ initialIterations?: number; + /** + * Wall-clock start of the goal, carried across resume so elapsed time keeps + * measuring from the original `/goal` rather than from the reload. A + * transcript is a file, so a non-finite or non-positive value is ignored + * rather than trusted. Defaults to now for a freshly set goal. + */ + initialSetAt?: number; }): ActiveGoal { const { config, sessionId, condition, tokensAtStart } = args; const system = config.getHookSystem(); @@ -331,10 +338,22 @@ export function registerGoalHook(args: { ); hookRef.hookId = hookId; + const now = Date.now(); + const restoredSetAt = args.initialSetAt; const goal: ActiveGoal = { condition, iterations: Math.max(0, args.initialIterations ?? 0), - setAt: Date.now(), + // A future `setAt` is rejected along with a non-finite or non-positive one. + // Every duration downstream is `Date.now() - setAt`, so a transcript + // claiming the goal starts tomorrow would render negative elapsed times + // rather than fail loudly. + setAt: + typeof restoredSetAt === 'number' && + Number.isFinite(restoredSetAt) && + restoredSetAt > 0 && + restoredSetAt <= now + ? restoredSetAt + : now, tokensAtStart, hookId, }; diff --git a/packages/core/src/services/cronScheduler.ts b/packages/core/src/services/cronScheduler.ts index 10ebfe3e656..02ece6495a7 100644 --- a/packages/core/src/services/cronScheduler.ts +++ b/packages/core/src/services/cronScheduler.ts @@ -378,7 +378,8 @@ export class CronScheduler { // tick takes over for subsequent fires of recurring jobs. Timers are // tracked in testFireTimers and cleared on stop()/destroy(). if (process.env['QWEN_CODE_TEST_CRON_FAST'] === '1' && !job.durable) { - const delayMs = Number(process.env['QWEN_CODE_TEST_CRON_DELAY_MS']) || 5000; + const delayMs = + Number(process.env['QWEN_CODE_TEST_CRON_DELAY_MS']) || 5000; const timer = setTimeout(() => { this.testFireTimers.delete(id); this.forceFireJob(id); diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index b3bd8c1e16f..89c0788f839 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -164,6 +164,10 @@ const { workspaces?: Array<{ id: string; cwd: string }>; lockedWorkspace?: { id: string; cwd: string; primary: boolean }; } | null, + latestGoalsProps: null as { + onCreateGoal?: (condition: string) => Promise; + onOpenSession?: (sessionId: string) => void; + } | null, }, sidebarTokens: [] as Array, rawEnqueuePrompt: vi.fn(() => true), @@ -695,6 +699,20 @@ vi.doMock('./components/dialogs/ScheduledTasksDialog', async () => { }, }; }); +// Capturing mock: stores App's real onCreateGoal / onOpenSession handlers so +// tests can drive the goal-creation orchestration without a daemon. +vi.doMock('./components/dialogs/GoalsDialog', async () => { + const React = await import('react'); + return { + GoalsDialog: (props: { + onCreateGoal?: (condition: string) => Promise; + onOpenSession?: (sessionId: string) => void; + }) => { + testState.latestGoalsProps = props; + return React.createElement('div'); + }, + }; +}); vi.doMock('./components/extensions/ExtensionsManagerPage', async () => { const React = await import('react'); return { @@ -871,6 +889,7 @@ beforeEach(() => { testState.messages = []; testState.latestChatEditorProps = null; testState.latestScheduledTasksProps = null; + testState.latestGoalsProps = null; sidebarTokens.length = 0; rawEnqueuePrompt.mockClear(); editorClear.mockClear(); @@ -4397,6 +4416,391 @@ describe('App session callbacks', () => { }); }); +describe('App /goal command', () => { + it('opens the Goals page for a bare /goal instead of sending a prompt', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + expect( + container.querySelector('[data-testid="goals-page"]'), + ).not.toBeNull(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + + it('opens the Goals page for a bare /goal even while a turn is running', async () => { + const { container, rerender } = renderApp(); + await flush(); + act(() => { + testState.streamingState = 'responding'; + rerender({}); + }); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + expect( + container.querySelector('[data-testid="goals-page"]'), + ).not.toBeNull(); + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + }); + + it('still sends /goal as a prompt rather than opening the page', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal ship it'; + await clickSubmit(container); + await flush(); + + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + expect(mockSessionActions.sendPrompt).toHaveBeenCalled(); + }); + + it('still routes /goal clear through the daemon clear path', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal clear'; + await clickSubmit(container); + await flush(); + + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + expect(mockSessionActions.clearGoal).toHaveBeenCalled(); + }); + + it('starts a goal in a fresh session from the Goals page', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.clearSession.mockClear(); + mockSessionActions.sendPrompt.mockClear(); + + await act(async () => { + await onCreateGoal('all tests pass'); + }); + + // A goal takes over its session's turns, so it starts in a NEW one + // (clearSession is how createNewSession starts one) rather than hijacking + // the conversation the user was already having. + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + '/goal all tests pass', + expect.anything(), + ); + }); + + it('keeps the Goals page mounted across createNewSession, not just after it', async () => { + // `createNewSession` switches to the chat view itself, before any await. That + // silently defeated the deferred switch below: by the time `sendPrompt` + // rejected, the Goals page — and the form that renders the error — was already + // gone, dumping the user in an empty chat with no explanation. The handler + // passes `keepView` so the page survives until the prompt is admitted. + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.sendPrompt.mockRejectedValueOnce( + new Error('daemon says no'), + ); + + await act(async () => { + await expect(onCreateGoal('all tests pass')).rejects.toThrow( + 'daemon says no', + ); + }); + + // createNewSession ran (a fresh session was started) … + expect(mockSessionActions.clearSession).toHaveBeenCalled(); + // … and the Goals page is STILL up, so the rejection has somewhere to land. + expect( + container.querySelector('[data-testid="goals-page"]'), + ).not.toBeNull(); + }); + + it('keeps the Goals page open when the goal prompt is rejected', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.sendPrompt.mockRejectedValueOnce( + new Error('daemon says no'), + ); + + await act(async () => { + await expect(onCreateGoal('all tests pass')).rejects.toThrow( + 'daemon says no', + ); + }); + + // Switching to the chat first would unmount the page, leaving the rejection + // with nowhere to render: the user would land in an empty session with no + // explanation. + expect( + container.querySelector('[data-testid="goals-page"]'), + ).not.toBeNull(); + }); + + it('switches to the chat view only after the goal prompt is admitted', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + + await act(async () => { + await onCreateGoal('all tests pass'); + }); + + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + }); + + it("opens a goal's session in the chat view", async () => { + // The goal's session transcript IS its history, so the Goals page has to be + // able to hand off to it. Nothing exercised this wiring before. + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + expect( + container.querySelector('[data-testid="goals-page"]'), + ).not.toBeNull(); + + const onOpenSession = testState.latestGoalsProps?.onOpenSession; + if (!onOpenSession) throw new Error('onOpenSession was not captured'); + mockSessionActions.loadSession.mockClear(); + + await act(async () => { + onOpenSession('goal-session-9'); + }); + await flush(); + + // Pin the session id, not the options bag — main added a `{ workspaceCwd }` + // second argument and will likely keep evolving it; the id is what this test + // is about. + expect(mockSessionActions.loadSession.mock.calls[0][0]).toBe( + 'goal-session-9', + ); + // It must leave the Goals page, or the user loads a transcript they cannot see. + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + }); + + it("reports a failure to open a goal's session instead of swallowing it", async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onOpenSession = testState.latestGoalsProps?.onOpenSession; + if (!onOpenSession) throw new Error('onOpenSession was not captured'); + mockSessionActions.loadSession.mockRejectedValueOnce( + new Error('session is gone'), + ); + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + await act(async () => { + onOpenSession('goal-session-9'); + }); + await flush(); + + // `loadSidebarSession` rethrows, so the handler's own `.catch` is the only + // thing standing between a dead session and an unhandled rejection. It has + // to route the failure to `reportError` (console + toast), not swallow it. + expect(consoleError).toHaveBeenCalledWith( + '[web-shell]', + expect.stringContaining('session is gone'), + expect.anything(), + ); + consoleError.mockRestore(); + }); + + it('reuses the empty session a failed goal attempt left behind', async () => { + // `sendPrompt` creates the daemon session lazily, so a prompt that fails + // after admission leaves a created-but-empty session. The form keeps the + // condition and invites a retry; if that retry started ANOTHER new session, + // every failed attempt would strand a blank chat in the sidebar. + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + + mockSessionActions.clearSession.mockClear(); + mockSessionActions.sendPrompt.mockRejectedValueOnce( + new Error('daemon says no'), + ); + + await act(async () => { + await expect(onCreateGoal('all tests pass')).rejects.toThrow( + 'daemon says no', + ); + }); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + + // Retry: the session from the failed attempt is still current and empty, so + // it is reused rather than abandoned. No second clearSession. + await act(async () => { + await onCreateGoal('all tests pass'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + expect(mockSessionActions.sendPrompt).toHaveBeenLastCalledWith( + '/goal all tests pass', + expect.anything(), + ); + }); + + it('forgets the stranded session once the user leaves the Goals page', async () => { + // The stranded session is only a scratch session while the Goals page is + // up. Leave, and the composer can talk to it — reusing it for a later goal + // would drop the goal loop on top of a real conversation, which is the very + // thing starting a fresh session exists to prevent. + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + + mockSessionActions.clearSession.mockClear(); + mockSessionActions.sendPrompt.mockRejectedValueOnce( + new Error('daemon says no'), + ); + await act(async () => { + await expect(onCreateGoal('all tests pass')).rejects.toThrow( + 'daemon says no', + ); + }); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + + // Leave the Goals page via its Back button, then use the session from the + // composer — it is now a real conversation, not a scratch session. + const back = container.querySelector( + '[data-testid="goals-page"] button[aria-label="back"]', + ); + if (!back) throw new Error('Back button not found'); + await act(async () => { + back.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + await flush(); + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + + testState.prompt = 'hello from the composer'; + await clickSubmit(container); + await flush(); + + // Re-open Goals and set a goal: it must NOT reuse the session the user has + // since been talking to. + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoalAgain = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoalAgain) throw new Error('onCreateGoal was not captured'); + mockSessionActions.clearSession.mockClear(); + + await act(async () => { + await onCreateGoalAgain('all tests pass'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + }); + + it('starts a fresh session again once a goal has actually been sent', async () => { + // The reuse above is only for a session stranded by a failure. Once a goal + // lands, that session belongs to it, and the next goal must not be dropped + // on top of the running one. + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + + mockSessionActions.clearSession.mockClear(); + mockSessionActions.sendPrompt.mockRejectedValueOnce( + new Error('daemon says no'), + ); + await act(async () => { + await expect(onCreateGoal('first goal')).rejects.toThrow( + 'daemon says no', + ); + }); + await act(async () => { + await onCreateGoal('first goal'); + }); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + + // A brand-new goal after a successful send: fresh session again. + await act(async () => { + await onCreateGoal('second goal'); + }); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); + }); + + it('does not drop the goal into the current session when the new session fails', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.clearSession.mockRejectedValueOnce( + new Error('daemon unreachable'), + ); + mockSessionActions.sendPrompt.mockClear(); + + await act(async () => { + await onCreateGoal('all tests pass'); + }); + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); +}); + describe('App manual-run orchestration (scheduled tasks)', () => { // Drives App's real runTaskManually / enqueueManualRun / tryFireBoundRun via // the onRunPrompt prop the (captured) ScheduledTasksDialog mock receives. diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 7fcacc18d9a..9f971b2b780 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -96,6 +96,12 @@ import { import { useIsLargeScreen } from './hooks/useIsLargeScreen'; import { MAX_SPLIT_PANES, parseSplitSessionIds } from './utils/splitUrl'; import { ScheduledTasksDialog } from './components/dialogs/ScheduledTasksDialog'; +import { GoalsDialog } from './components/dialogs/GoalsDialog'; +import { + goalArgOf, + isGoalClearCommand, + isGoalClearKeyword, +} from './utils/goalCondition'; import { ExtensionsManagerPage } from './components/extensions/ExtensionsManagerPage'; import { PluginManagerPage } from './components/plugins/PluginManagerPage'; import { SettingsMessage } from './components/messages/SettingsMessage'; @@ -337,24 +343,6 @@ function normalizeHiddenCommand(command: string): string { return command.trim().replace(/^\/+/, '').toLowerCase(); } -// Keep in sync with CLEAR_KEYWORDS in packages/cli/src/ui/commands/goalCommand.ts -const GOAL_CLEAR_KEYWORDS = new Set([ - 'clear', - 'stop', - 'off', - 'reset', - 'none', - 'cancel', -]); - -function isGoalClearCommand(text: string): boolean { - const goalArg = text - .replace(/^\/goal\b/i, '') - .trim() - .toLowerCase(); - return GOAL_CLEAR_KEYWORDS.has(goalArg); -} - interface ActiveGoalStatus { condition: string; setAt: number; @@ -2157,9 +2145,9 @@ export function App({ // (not a modal overlay), mirroring the reference design; creating or opening // a chat returns to 'chat'. (Daemon Status is no longer a boolean dialog — it // is one of the activePanel values below.) - const [mainView, setMainView] = useState<'chat' | 'scheduledTasks' | 'split'>( - 'chat', - ); + const [mainView, setMainView] = useState< + 'chat' | 'scheduledTasks' | 'goals' | 'split' + >('chat'); // Sessions to seed the split view with (e.g. the selection from the overview). const [splitSessionIds, setSplitSessionIds] = useState([]); // Latest pane list, readable from the shrink-close effect without making it a @@ -2229,6 +2217,10 @@ export function App({ setActivePanel(null); setMainView('scheduledTasks'); }, []); + const openGoals = useCallback(() => { + setActivePanel(null); + setMainView('goals'); + }, []); const openSessionDrawer = useCallback(() => { if (!sidebarOptions.enabled) return; setActivePanel(null); @@ -2467,12 +2459,14 @@ export function App({ if (activePanel) setActivePanel(null); if (modelDialogMode) setModelDialogMode(null); if (showApprovalModeDialog) setShowApprovalModeDialog(false); - // The Scheduled Tasks page is a full-pane overlay (position:absolute) that - // covers the chat footer too, so dismiss it for the same reason. The split - // view is deliberately NOT dismissed: each pane owns and renders its own - // session's approval, so an approval on the (outer) main session must not - // yank the user out of the panes they are working in. - if (mainView === 'scheduledTasks') setMainView('chat'); + // The Scheduled Tasks and Goals pages are full-pane overlays + // (position:absolute) that cover the chat footer too, so dismiss them for + // the same reason. The split view is deliberately NOT dismissed: each pane + // owns and renders its own session's approval, so an approval on the (outer) + // main session must not yank the user out of the panes they are working in. + if (mainView === 'scheduledTasks' || mainView === 'goals') { + setMainView('chat'); + } }, [ approvalOverlayActive, activePanel, @@ -2636,6 +2630,30 @@ export function App({ } | null>(null); const onSessionCreatedRef = useRef(onSessionCreated); onSessionCreatedRef.current = onSessionCreated; + /** + * The session a failed `/goal` submit left behind. + * + * Setting a goal starts a fresh session and then sends `/goal ` + * into it, but the daemon session is not created by the "new session" step — + * `ensureSessionForPrompt` creates it lazily *inside* `sendPrompt`. So a + * prompt that fails leaves a session that exists but never got its goal. + * + * The Goals form keeps the condition and lets the user retry. Without this + * ref every retry would abandon that session and create another, piling up + * blank chats in the sidebar. Remembering it lets the retry reuse it — no + * session is ever deleted. + * + * Only valid while the Goals page stays mounted. The moment the user leaves, + * that session is reachable from the composer and may stop being a scratch + * session, so the effect below forgets it: a later goal then starts a fresh + * session rather than landing on top of a conversation. + */ + const strandedGoalSessionRef = useRef(undefined); + useEffect(() => { + if (mainView !== 'goals') { + strandedGoalSessionRef.current = undefined; + } + }, [mainView]); const ensureSessionForPrompt = useCallback(() => { const currentSessionId = connectionRef.current.sessionId; if (createSessionPromiseRef.current) { @@ -3739,7 +3757,16 @@ export function App({ return request; }, []); const createNewSession = useCallback( - async (workspaceCwd?: string) => { + async ( + workspaceCwd?: string, + /** + * Leave `mainView` alone. The default is to switch to the chat, because a + * user who asks for a new chat wants to see it — but the Goals form has to + * stay mounted until its prompt is admitted, or a rejection has nowhere to + * render. Only that caller passes this. + */ + opts?: { keepView?: boolean }, + ) => { const targetWorkspaceCwd = lockedWorkspaceCwd ?? workspaceCwd; selectedWorkspaceCwdRef.current = targetWorkspaceCwd; setSelectedWorkspaceCwd(targetWorkspaceCwd); @@ -3749,7 +3776,7 @@ export function App({ // Starting a new chat means the user wants to see it — leave any open // Settings/Status panel so the fresh chat is visible (no-op when closed). closePanel(); - setMainView('chat'); + if (!opts?.keepView) setMainView('chat'); let focusRequest: number | undefined; try { const clearPromise = ( @@ -4262,8 +4289,7 @@ export function App({ commitComposerAccepted?: ComposerSubmitCommit; }, ) => { - const goalArg = text.replace(/^\/goal\b/i, '').trim(); - const lowerGoalArg = goalArg.toLowerCase(); + const goalArg = goalArgOf(text); const sendToDaemon = opts?.sendToDaemon ?? true; const sendGoalPrompt = () => { const deferComposerCommit = Boolean(onSubmitBeforeRef.current); @@ -4280,7 +4306,7 @@ export function App({ return clearComposerOnPromptStart ? false : true; }; - if (goalArg && GOAL_CLEAR_KEYWORDS.has(lowerGoalArg)) { + if (goalArg && isGoalClearKeyword(goalArg)) { if (!sendToDaemon) { store.appendLocalUserMessage(text); dispatchGoalCleared(activeGoalRef.current); @@ -4296,16 +4322,17 @@ export function App({ return sendGoalPrompt(); } - if (sendToDaemon) { - return sendGoalPrompt(); - } - store.appendLocalUserMessage(text); + // Bare `/goal` opens the Goals page instead of asking the daemon to print + // its status as text — the same move `/schedule` makes. Nothing is sent, + // so the composer is cleared by returning true. + openGoals(); return true; }, [ dispatchGoalCleared, dispatchGoalSet, handleBusyGoalClear, + openGoals, reportError, sendPrompt, store, @@ -4396,6 +4423,12 @@ export function App({ return true; } if (cmd === 'goal') { + // A bare `/goal` just opens the Goals page; it neither sends a + // prompt nor touches the session, so it works mid-turn too. + if (!goalArgOf(text)) { + openGoals(); + return true; + } if (promptBlocked) { if (isGoalClearCommand(text)) { return handleBusyGoalClear(text); @@ -5183,6 +5216,7 @@ export function App({ closePanel, openPanel, openScheduledTasks, + openGoals, createNewSession, handleBusyGoalClear, handleGoalSlashCommand, @@ -6216,6 +6250,10 @@ export function App({ closeMobileDrawer(); openScheduledTasks(); }} + onOpenGoals={() => { + closeMobileDrawer(); + openGoals(); + }} onOpenSessions={() => { closeMobileDrawer(); openPanel('sessions'); @@ -6558,6 +6596,109 @@ export function App({ )} + {mainView === 'goals' && ( +
+
+ +
+ {t('goals.title')} +
+
+
+ { + // Setting a goal registers the Stop hook AND kicks off + // the first turn, so it has to travel the prompt path. + // Start a FRESH session so the goal loop doesn't take + // over the conversation the user was already having. + // + // Unless a previous attempt in this same visit to the + // page already made one and then failed to send: that + // session never got its goal and is still current, so + // reuse it. Creating another would strand it, and a user + // retrying a few times would end up with a column of + // blank chats in the sidebar. + // + // Leaving the page forgets it (see the effect on + // `strandedGoalSessionRef`), so this can never reuse a + // session the user has since talked to. + const stranded = strandedGoalSessionRef.current; + const canReuseStranded = + stranded !== undefined && + connectionRef.current.sessionId === stranded; + if (!canReuseStranded) { + // `keepView`: createNewSession switches to the chat by + // default, which would unmount this form before the + // prompt is even sent and leave a later rejection with + // nowhere to render — the exact failure the deferred + // switch below exists to prevent. + const created = await createNewSession(undefined, { + keepView: true, + }); + // createNewSession already surfaced the failure; don't + // drop the goal into the wrong (still-current) session. + // `false` keeps the form open with the typed condition + // still in it — returning normally would read as + // "created" and reset it. + if (!created) return false; + onSessionIdChange?.(undefined); + } + // Switch to the chat only once the prompt is admitted. + // Switching first unmounts the Goals page, and a later + // rejection would then have nowhere to render: the user + // would land in an empty session with no explanation. + // Letting this reject keeps the error in the form the + // user is looking at. + try { + await sendPrompt(`/goal ${condition}`, undefined, { + clearComposerOnPromptStart: true, + }); + } catch (error) { + // `sendPrompt` creates the session lazily, so by now + // one may exist even though the prompt never landed. + // Remember it so the retry reuses it rather than + // stranding it. + strandedGoalSessionRef.current = + connectionRef.current.sessionId; + throw error; + } + strandedGoalSessionRef.current = undefined; + setMainView('chat'); + }} + onOpenSession={(sessionId) => { + // The goal's session transcript IS its history. + setMainView('chat'); + loadSidebarSession(sessionId).catch( + (error: unknown) => { + reportError(error, 'Failed to open session'); + }, + ); + }} + onError={reportError} + /> +
+
+ )} {mainView === 'split' && (
{/* The outer session's approval overlay is suppressed under the @@ -7082,6 +7223,7 @@ export function App({ onReturnToInput={handleReturnToEditor} tasks={backgroundTasks} activeGoal={activeGoal} + onOpenGoals={openGoals} hideSettings={hideSettings} onToggleShortcuts={handleToggleShortcuts} compact={true} diff --git a/packages/web-shell/client/components/StatusBar.module.css b/packages/web-shell/client/components/StatusBar.module.css index 223d6e37f8a..e810591c6d7 100644 --- a/packages/web-shell/client/components/StatusBar.module.css +++ b/packages/web-shell/client/components/StatusBar.module.css @@ -164,6 +164,23 @@ white-space: nowrap; } +.goalButton { + display: inline-flex; + align-items: center; + padding: 0; + margin: 0; + border: none; + background: none; + font: inherit; + color: inherit; + cursor: pointer; +} + +.goalButton:hover .goal, +.goalButton:focus-visible .goal { + text-decoration: underline; +} + .separator { color: var(--muted-foreground); } diff --git a/packages/web-shell/client/components/StatusBar.test.tsx b/packages/web-shell/client/components/StatusBar.test.tsx new file mode 100644 index 00000000000..55b2e8c38c5 --- /dev/null +++ b/packages/web-shell/client/components/StatusBar.test.tsx @@ -0,0 +1,115 @@ +// @vitest-environment jsdom +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +const { mockConnection } = vi.hoisted(() => ({ + mockConnection: { + sessionId: 'session-1' as string | undefined, + currentModel: undefined as string | undefined, + contextWindow: 0, + tokenCount: 0, + }, +})); + +vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ + useConnection: () => mockConnection, +})); + +const { StatusBar } = await import('./StatusBar'); +const { I18nProvider } = await import('../i18n'); + +let container: HTMLDivElement | null = null; +let root: Root | null = null; + +afterEach(() => { + act(() => root?.unmount()); + container?.remove(); + root = null; + container = null; + vi.clearAllMocks(); +}); + +function mount( + props: Partial[0]> = {}, +): HTMLDivElement { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + root!.render( + + + , + ); + }); + return container; +} + +const goalButton = () => + document.querySelector('button[aria-label^="Goals"]'); + +describe('StatusBar goal pill', () => { + it('names the active goal in its accessible label', () => { + // The visible pill is only "◎ Goal (2m)" — the condition never appears in + // it, and `title` is a hover tooltip screen readers do not reliably + // announce. Without the condition here, a screen-reader user cannot tell + // which goal is running without opening the Goals page. + mount({ + activeGoal: { condition: 'all tests pass', setAt: Date.now() - 5000 }, + onOpenGoals: vi.fn(), + }); + + expect(goalButton()?.getAttribute('aria-label')).toBe( + 'Goals: all tests pass', + ); + // The purpose stays in front of the condition: a bare condition string + // gives no hint that activating this opens anything. + expect(goalButton()?.getAttribute('aria-label')).toMatch(/^Goals: /); + }); + + it('falls back to the plain label when no goal is active', () => { + mount({ onOpenGoals: vi.fn() }); + expect(goalButton()).toBeNull(); + }); + + it('opens the Goals page when activated', () => { + const onOpenGoals = vi.fn(); + mount({ + activeGoal: { condition: 'ship it', setAt: Date.now() }, + onOpenGoals, + }); + + act(() => { + goalButton()?.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true }), + ); + }); + + expect(onOpenGoals).toHaveBeenCalledTimes(1); + }); + + it('renders the goal as static text when there is nowhere to open', () => { + // No `onOpenGoals` (e.g. embedded without the Goals page): the pill must + // not pretend to be interactive. + mount({ activeGoal: { condition: 'ship it', setAt: Date.now() } }); + + expect(goalButton()).toBeNull(); + expect(document.body.textContent).toContain('/goal active'); + }); +}); diff --git a/packages/web-shell/client/components/StatusBar.tsx b/packages/web-shell/client/components/StatusBar.tsx index 21f1c86b510..e5e7cccb00c 100644 --- a/packages/web-shell/client/components/StatusBar.tsx +++ b/packages/web-shell/client/components/StatusBar.tsx @@ -54,6 +54,8 @@ interface StatusBarProps { condition: string; setAt: number; } | null; + /** Open the Goals page. When omitted the goal pill stays a plain label. */ + onOpenGoals?: () => void; /** Hide the settings gear button (e.g. when /settings is in hiddenSlashCommands). */ hideSettings?: boolean; /** Toggle the keyboard-shortcuts panel (same as typing `?` in the editor). */ @@ -166,6 +168,7 @@ export const StatusBar = forwardRef( onReturnToInput, tasks, activeGoal, + onOpenGoals, hideSettings, onToggleShortcuts, compact = false, @@ -356,11 +359,31 @@ export const StatusBar = forwardRef( )} - {goalLabel && ( - - {goalLabel} - - )} + {goalLabel && + (onOpenGoals ? ( + + ) : ( + + {goalLabel} + + ))}
); diff --git a/packages/web-shell/client/components/dialogs/GoalsDialog.module.css b/packages/web-shell/client/components/dialogs/GoalsDialog.module.css new file mode 100644 index 00000000000..9374113be69 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GoalsDialog.module.css @@ -0,0 +1,310 @@ +.root { + display: flex; + flex-direction: column; + gap: 16px; + min-width: 0; +} + +.intro { + font-size: 13px; + line-height: 1.5; + color: var(--muted-foreground); +} + +.toolbar { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + flex-wrap: wrap; +} + +.count { + font-size: 13px; + font-weight: 600; + color: var(--foreground); +} + +.toolbarActions { + display: flex; + align-items: center; + gap: 8px; +} + +.primaryButton, +.secondaryButton { + appearance: none; + border-radius: 8px; + padding: 6px 14px; + font-size: 13px; + font-weight: 500; + cursor: pointer; + border: 1px solid var(--border); + transition: + background 0.15s ease, + opacity 0.15s ease; +} + +.primaryButton { + background: var(--primary); + color: var(--background); + border-color: var(--primary); +} + +.secondaryButton { + background: transparent; + color: var(--foreground); +} + +/* Same ring the form controls use (`.textarea:focus` below), but offset + outwards: `.primaryButton` is already filled with `--primary`, so an inset + ring in that colour would be invisible on it. */ +.primaryButton:focus-visible, +.secondaryButton:focus-visible { + outline: 2px solid var(--primary); + outline-offset: 2px; +} + +.primaryButton:disabled, +.secondaryButton:disabled { + opacity: 0.55; + cursor: default; +} + +.secondaryButton:hover:not(:disabled) { + background: var(--muted); +} + +/* ── Create form ─────────────────────────────────────────────── */ + +.formFields { + display: flex; + flex-direction: column; + gap: 14px; +} + +.field { + display: flex; + flex-direction: column; + gap: 6px; + min-width: 0; +} + +/* `--muted-foreground`, matching `.fieldLabel` in ScheduledTasksDialog: these two + dialogs sit side by side in the same sidebar and a field label is secondary + text in both. */ +.fieldLabel { + font-size: 12px; + font-weight: 600; + color: var(--muted-foreground); +} + +.required { + color: var(--error-color); + margin-left: 2px; +} + +.textarea { + width: 100%; + box-sizing: border-box; + border: 1px solid var(--border); + border-radius: 8px; + background: var(--background); + color: var(--foreground); + font-size: 13px; + font-family: inherit; + padding: 8px 10px; + resize: vertical; + min-height: 84px; +} + +.textarea:focus { + outline: 2px solid var(--primary); + outline-offset: -1px; +} + +.formHint { + font-size: 12px; + line-height: 1.5; + color: var(--muted-foreground); +} + +.formError, +.loadError { + font-size: 12px; + color: var(--error-color); +} + +/* The list is incomplete because some sessions could not be probed — distinct + * from a hard load error, which shows no list at all. */ +.degraded { + font-size: 12px; + color: var(--muted-foreground); + border: 1px solid var(--border); + border-radius: 8px; + padding: 8px 10px; +} + +.formActions { + display: flex; + justify-content: flex-end; + gap: 8px; +} + +/* ── List ────────────────────────────────────────────────────── */ + +.empty { + padding: 32px 16px; + text-align: center; + color: var(--muted-foreground); + font-size: 13px; +} + +.list { + display: flex; + flex-direction: column; + gap: 12px; +} + +.card { + display: flex; + flex-direction: column; + gap: 10px; + padding: 14px; + border: 1px solid var(--border); + border-radius: 12px; + background: var(--background); + min-width: 0; +} + +.cardHeader { + display: flex; + align-items: center; + gap: 10px; +} + +/* Idle: the goal is registered but its session is between turns. Running: the + * loop is mid-turn right now. */ +.statusDot { + flex: 0 0 auto; + width: 8px; + height: 8px; + border-radius: 50%; + background: var(--muted-foreground); +} + +.statusDotRunning { + background: var(--agent-blue-500); +} + +.cardTitle { + flex: 1 1 auto; + min-width: 0; + font-size: 14px; + font-weight: 600; + color: var(--foreground); + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.cardMenu { + display: flex; + align-items: center; + gap: 4px; + flex: 0 0 auto; +} + +.iconAction { + appearance: none; + background: transparent; + border: none; + color: var(--muted-foreground); + cursor: pointer; + border-radius: 6px; + width: 26px; + height: 26px; + font-size: 12px; + display: inline-flex; + align-items: center; + justify-content: center; +} + +.iconAction:hover:not(:disabled) { + background: var(--muted); + color: var(--foreground); +} + +/* Keyboard users get the same affordance as the mouse hover; matches + `.iconButton:focus-visible` in DialogShell.module.css. */ +.iconAction:focus-visible:not(:disabled) { + background: var(--muted); + color: var(--foreground); +} + +.iconAction:disabled { + opacity: 0.5; + cursor: default; +} + +.cardReason { + font-size: 12px; + line-height: 1.5; + color: var(--muted-foreground); + display: -webkit-box; + -webkit-line-clamp: 3; + -webkit-box-orient: vertical; + overflow: hidden; + white-space: pre-wrap; + word-break: break-word; +} + +.reasonLabel { + font-weight: 600; +} + +.cardFooter { + display: flex; + align-items: center; + gap: 10px; + flex-wrap: wrap; + margin-top: auto; +} + +.statusPill { + display: inline-flex; + align-items: center; + font-size: 12px; + font-weight: 500; + color: var(--foreground); + background: var(--muted); + border-radius: 999px; + padding: 3px 10px; +} + +.meta { + font-size: 12px; + color: var(--muted-foreground); +} + +.sessionLink { + appearance: none; + background: transparent; + border: none; + padding: 0; + margin-left: auto; + font: inherit; + font-size: 12px; + color: var(--muted-foreground); + cursor: pointer; + max-width: 40%; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.sessionLink:hover, +.sessionLink:focus-visible { + color: var(--foreground); + text-decoration: underline; +} diff --git a/packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx b/packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx new file mode 100644 index 00000000000..712358a0e13 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx @@ -0,0 +1,578 @@ +// @vitest-environment jsdom +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +interface MockGoal { + sessionId: string; + displayName: string | null; + condition: string; + iterations: number; + setAt: number; + lastReason?: string; + hasActivePrompt: boolean; +} + +const { actions } = vi.hoisted(() => ({ + actions: { + listGoals: vi.fn(), + clearGoal: vi.fn(), + }, +})); + +vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ + useWorkspaceActions: () => actions, +})); + +const { GoalsDialog } = await import('./GoalsDialog'); +const { I18nProvider } = await import('../../i18n'); + +let container: HTMLDivElement | null = null; +let root: Root | null = null; + +async function flush() { + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); +} + +function click(el: Element | null | undefined) { + if (!el) throw new Error('click target not found'); + act(() => { + el.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true }), + ); + }); +} + +function findButton(label: string): HTMLButtonElement | undefined { + return Array.from(document.querySelectorAll('button')).find( + (b) => b.textContent?.trim() === label, + ); +} + +/** Set the condition textarea the way React's onChange expects. */ +function setTextarea(value: string) { + const textarea = document.querySelector('textarea'); + if (!textarea) throw new Error('textarea not found'); + act(() => { + const setter = Object.getOwnPropertyDescriptor( + window.HTMLTextAreaElement.prototype, + 'value', + )!.set!; + setter.call(textarea, value); + textarea.dispatchEvent(new Event('input', { bubbles: true })); + }); +} + +async function mount( + goals: MockGoal[], + opts: { + onCreateGoal?: ( + condition: string, + ) => boolean | void | Promise; + onOpenSession?: (sessionId: string) => void; + onError?: (error: unknown, message: string) => void; + droppedCount?: number; + } = {}, +) { + actions.listGoals.mockResolvedValue({ + goals, + droppedCount: opts.droppedCount ?? 0, + }); + actions.clearGoal.mockResolvedValue({ cleared: true }); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root!.render( + + + , + ); + }); + await flush(); +} + +const baseGoal = (over: Partial = {}): MockGoal => ({ + sessionId: 'sess-1', + displayName: 'fix-ci', + condition: 'all tests pass', + iterations: 0, + setAt: Date.now() - 5000, + hasActivePrompt: false, + ...over, +}); + +beforeEach(() => { + vi.spyOn(window, 'confirm').mockReturnValue(true); +}); + +afterEach(() => { + // Unconditionally, not just at the end of each fake-timer test: a failing + // assertion skips the inline restore, and fake timers would then leak into + // every test after it as unrelated-looking hangs. + vi.useRealTimers(); + act(() => root?.unmount()); + container?.remove(); + root = null; + container = null; + vi.restoreAllMocks(); + vi.clearAllMocks(); +}); + +describe('GoalsDialog', () => { + it('shows the empty state when no goal is active', async () => { + await mount([]); + expect(document.body.textContent).toContain('No active goals'); + }); + + it('warns that the list is incomplete when sessions could not be probed', async () => { + // Otherwise a brownout is indistinguishable from an empty workspace, and + // the user re-creates goals that are already running. + await mount([], { droppedCount: 2 }); + + expect( + document.querySelector('[data-testid="goals-dropped"]'), + ).not.toBeNull(); + expect(document.body.textContent).toContain( + '2 sessions could not be reached', + ); + }); + + it('shows no degradation notice when every session was probed', async () => { + await mount([baseGoal()]); + expect(document.querySelector('[data-testid="goals-dropped"]')).toBeNull(); + }); + + it('renders a goal with its condition, turn count and judge verdict', async () => { + await mount([ + baseGoal({ iterations: 3, lastReason: 'two tests still fail' }), + ]); + + const text = document.body.textContent ?? ''; + expect(text).toContain('all tests pass'); + expect(text).toContain('3 turns'); + expect(text).toContain('two tests still fail'); + expect(text).toContain('fix-ci'); + }); + + it('says "not yet evaluated" before the first judge turn', async () => { + await mount([baseGoal({ iterations: 0 })]); + expect(document.body.textContent).toContain('not yet evaluated'); + }); + + it('distinguishes a working goal from a waiting one', async () => { + await mount([baseGoal({ hasActivePrompt: true })]); + expect(document.body.textContent).toContain('Working'); + + act(() => root?.unmount()); + container?.remove(); + await mount([baseGoal({ hasActivePrompt: false })]); + expect(document.body.textContent).toContain('Waiting'); + }); + + it('falls back to the session id when the session has no name', async () => { + await mount([baseGoal({ displayName: null, sessionId: 'abc-123' })]); + expect(findButton('abc-123')).toBeDefined(); + }); + + it('opens the goal session when its label is clicked', async () => { + const onOpenSession = vi.fn(); + await mount([baseGoal()], { onOpenSession }); + + click(findButton('fix-ci')); + + expect(onOpenSession).toHaveBeenCalledWith('sess-1'); + }); + + it('clears a goal after confirmation and reloads the list', async () => { + await mount([baseGoal()]); + actions.listGoals.mockResolvedValue({ goals: [], droppedCount: 0 }); + + click(document.querySelector('button[aria-label="Clear goal"]')); + await flush(); + + expect(window.confirm).toHaveBeenCalled(); + expect(actions.clearGoal).toHaveBeenCalledWith('sess-1'); + expect(document.body.textContent).toContain('No active goals'); + }); + + it('does not clear when the confirmation is declined', async () => { + vi.mocked(window.confirm).mockReturnValue(false); + await mount([baseGoal()]); + + click(document.querySelector('button[aria-label="Clear goal"]')); + await flush(); + + expect(actions.clearGoal).not.toHaveBeenCalled(); + }); + + it('surfaces a clear failure through onError', async () => { + const onError = vi.fn(); + await mount([baseGoal()], { onError }); + actions.clearGoal.mockRejectedValue(new Error('session is gone')); + + click(document.querySelector('button[aria-label="Clear goal"]')); + await flush(); + + expect(onError).toHaveBeenCalled(); + }); + + it('reloads the list when Refresh is clicked', async () => { + // The poll is on a 10s lane, so Refresh is the only way to see a goal you + // just set from another window without waiting. + await mount([]); + expect(actions.listGoals).toHaveBeenCalledTimes(1); + + actions.listGoals.mockResolvedValue({ + goals: [baseGoal({ condition: 'freshly appeared' })], + droppedCount: 0, + }); + click(findButton('Refresh')); + await flush(); + + expect(actions.listGoals).toHaveBeenCalledTimes(2); + expect(document.body.textContent).toContain('freshly appeared'); + }); + + it("disables a goal's clear button while its clear is in flight", async () => { + // Without this, a double-click fires two concurrent clears at the same + // session — the second racing a goal that is already gone. + await mount([baseGoal()]); + // After mount: the helper itself stubs clearGoal with a resolved value. + let release: (() => void) | undefined; + actions.clearGoal.mockImplementation( + () => + new Promise((resolve) => { + release = () => resolve({ cleared: true }); + }), + ); + + const clearButton = () => + document.querySelector( + 'button[aria-label="Clear goal"]', + ); + expect(clearButton()?.disabled).toBe(false); + + click(clearButton()); + await flush(); + + expect(actions.clearGoal).toHaveBeenCalledTimes(1); + expect(clearButton()?.disabled).toBe(true); + + // A second click while the first is still in flight must do nothing. + click(clearButton()); + await flush(); + expect(actions.clearGoal).toHaveBeenCalledTimes(1); + + await act(async () => { + release?.(); + await Promise.resolve(); + }); + await flush(); + }); + + it('rejects an empty condition instead of submitting it', async () => { + const onCreateGoal = vi.fn(); + await mount([], { onCreateGoal }); + + click(findButton('New goal')); + click(findButton('Set goal')); + await flush(); + + expect(onCreateGoal).not.toHaveBeenCalled(); + expect(document.body.textContent).toContain('Enter a condition'); + // Announced, not just painted: a screen-reader user gets no other signal + // that the submit was rejected, and would believe the goal was created. + const alert = document.querySelector('[role="alert"]'); + expect(alert?.textContent).toContain('Enter a condition'); + }); + + it('accepts a condition far longer than the old 4,000-char cap', async () => { + // `/goal` takes a condition of any length (#6665). Rejecting one here that + // the daemon would accept splits the two surfaces, and the textarea used to + // silently truncate at `maxLength` before the user could even submit it. + const onCreateGoal = vi.fn(); + await mount([], { onCreateGoal }); + + click(findButton('New goal')); + const condition = 'x'.repeat(10_000); + setTextarea(condition); + click(findButton('Set goal')); + await flush(); + + expect(onCreateGoal).toHaveBeenCalledWith(condition); + }); + + it('rejects a clear keyword, which would drop the goal instead of setting it', async () => { + const onCreateGoal = vi.fn(); + await mount([], { onCreateGoal }); + + click(findButton('New goal')); + // `/goal clear` clears; a form that accepted it would spawn a session that + // immediately drops its own goal. + setTextarea(' Clear '); + click(findButton('Set goal')); + await flush(); + + expect(onCreateGoal).not.toHaveBeenCalled(); + expect(document.body.textContent).toContain('clears a goal rather than'); + }); + + it('discards the typed condition when the form is cancelled', async () => { + // Cancel is the only way out of the form without submitting; if its wiring + // breaks there is no escape but a page reload. + const onCreateGoal = vi.fn(); + await mount([], { onCreateGoal }); + + click(findButton('New goal')); + setTextarea('ship it'); + click(findButton('Cancel')); + await flush(); + + expect(onCreateGoal).not.toHaveBeenCalled(); + expect(document.querySelector('textarea')).toBeNull(); + + // Re-opening must not resurrect the abandoned condition. + click(findButton('New goal')); + expect(document.querySelector('textarea')?.value).toBe(''); + }); + + it('submits a trimmed condition and closes the form', async () => { + const onCreateGoal = vi.fn(); + await mount([], { onCreateGoal }); + + click(findButton('New goal')); + setTextarea(' ship it '); + click(findButton('Set goal')); + await flush(); + + expect(onCreateGoal).toHaveBeenCalledWith('ship it'); + expect(document.querySelector('textarea')).toBeNull(); + }); + + it('never lets a slow /goals poll overlap itself', async () => { + // `GET /goals` fans out one probe per live session and a wedged child can + // hold it for the bridge's ext-method timeout, which is the same order as + // the poll interval. A fixed setInterval would stack fan-outs, and the + // action timeout rejects the wait without aborting the request. + vi.useFakeTimers(); + let release: (() => void) | undefined; + actions.listGoals.mockImplementation( + () => + new Promise((resolve) => { + release = () => resolve({ goals: [], droppedCount: 0 }); + }), + ); + + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root!.render( + + + , + ); + }); + + // The mount load is in flight and never settles. + expect(actions.listGoals).toHaveBeenCalledTimes(1); + + // Well past several intervals: still exactly one request. + await act(async () => { + await vi.advanceTimersByTimeAsync(45_000); + }); + expect(actions.listGoals).toHaveBeenCalledTimes(1); + + // Once it settles, the next poll is scheduled one interval later. + await act(async () => { + release?.(); + await vi.advanceTimersByTimeAsync(0); + }); + expect(actions.listGoals).toHaveBeenCalledTimes(1); + + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + expect(actions.listGoals).toHaveBeenCalledTimes(2); + + vi.useRealTimers(); + }); + + it('stops polling once unmounted', async () => { + vi.useFakeTimers(); + actions.listGoals.mockResolvedValue({ goals: [], droppedCount: 0 }); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root!.render( + + + , + ); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + const afterMount = actions.listGoals.mock.calls.length; + + act(() => root?.unmount()); + root = null; + + await act(async () => { + await vi.advanceTimersByTimeAsync(60_000); + }); + expect(actions.listGoals).toHaveBeenCalledTimes(afterMount); + + vi.useRealTimers(); + }); + + it('routes a creation failure to a toast when the page closed mid-flight', async () => { + const onError = vi.fn(); + let reject: ((e: Error) => void) | undefined; + const onCreateGoal = vi.fn( + () => + new Promise((_resolve, rj) => { + reject = rj; + }), + ); + await mount([], { onCreateGoal, onError }); + + click(findButton('New goal')); + setTextarea('ship it'); + click(findButton('Set goal')); + await flush(); + + // Navigating away unmounts the page while the prompt is still in flight; + // an inline form error would never be seen. + act(() => root?.unmount()); + root = null; + + await act(async () => { + reject?.(new Error('daemon says no')); + await Promise.resolve(); + }); + + expect(onError).toHaveBeenCalled(); + }); + + it('renders the load error and keeps the list usable', async () => { + actions.listGoals.mockRejectedValue(new Error('daemon unreachable')); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root!.render( + + + , + ); + }); + await flush(); + + expect(document.body.textContent).toContain('daemon unreachable'); + // The list goes stale on a poll that fails after the page is already up; + // nothing else on screen changes, so this has to announce itself. + expect(document.querySelector('[role="alert"]')?.textContent).toContain( + 'daemon unreachable', + ); + }); + + it('drops a stale dropped-session count when the next load fails outright', async () => { + // The banner describes a partial probe. A hard `GET /goals` failure is a + // different state, and pinning the old count reports a partial probe that + // did not happen on this load. + vi.useFakeTimers(); + actions.listGoals.mockResolvedValue({ goals: [], droppedCount: 2 }); + actions.clearGoal.mockResolvedValue({ cleared: true }); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root!.render( + + + , + ); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect( + document.querySelector('[data-testid="goals-dropped"]'), + ).not.toBeNull(); + + // The next poll reaches nothing at all. + actions.listGoals.mockRejectedValue(new Error('daemon unreachable')); + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + + expect(document.querySelector('[data-testid="goals-dropped"]')).toBeNull(); + expect(document.body.textContent).toContain('daemon unreachable'); + vi.useRealTimers(); + }); + + it('keeps the form open with the condition when creation reports failure', async () => { + // `onCreateGoal` returning false means no goal was started and the caller + // already surfaced why. Resetting would close the form and silently throw + // away what the user typed. + const onCreateGoal = vi.fn().mockResolvedValue(false); + await mount([], { onCreateGoal }); + + click(findButton('New goal')); + setTextarea('ship it'); + click(findButton('Set goal')); + await flush(); + + expect(onCreateGoal).toHaveBeenCalledWith('ship it'); + const textarea = document.querySelector('textarea'); + expect(textarea).not.toBeNull(); + expect(textarea!.value).toBe('ship it'); + }); + + it('closes the form when creation resolves with no explicit result', async () => { + // The common case: a void-returning callback still means success. + const onCreateGoal = vi.fn().mockResolvedValue(undefined); + await mount([], { onCreateGoal }); + + click(findButton('New goal')); + setTextarea('ship it'); + click(findButton('Set goal')); + await flush(); + + expect(document.querySelector('textarea')).toBeNull(); + }); +}); diff --git a/packages/web-shell/client/components/dialogs/GoalsDialog.tsx b/packages/web-shell/client/components/dialogs/GoalsDialog.tsx new file mode 100644 index 00000000000..ff65bb65e45 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GoalsDialog.tsx @@ -0,0 +1,362 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useCallback, useEffect, useRef, useState } from 'react'; +import { + useWorkspaceActions, + type DaemonGoal, +} from '@qwen-code/webui/daemon-react-sdk'; +import { useI18n } from '../../i18n'; +import { DialogShell } from './DialogShell'; +import { formatRuntime } from '../../utils/formatRuntime'; +import { isGoalClearKeyword } from '../../utils/goalCondition'; +import styles from './GoalsDialog.module.css'; + +/** + * Gap between the end of one refetch and the start of the next. Unlike + * scheduled tasks there is no `nextRunAt` to schedule against: a goal advances + * whenever its session finishes a turn, which the page can't predict, so it + * polls on a slow lane. + */ +const RELOAD_INTERVAL_MS = 10_000; +/** The elapsed-time column ticks independently of the refetch. */ +const TICK_INTERVAL_MS = 1000; + +interface GoalsDialogProps { + /** Send `/goal ` into a brand-new session and switch to it. Setting + * a goal is not a pure write — the daemon registers the Stop hook AND kicks + * off the first turn — so it has to travel the prompt path, not a REST POST. + * + * Return `false` to report a failure this form must not treat as a creation — + * the condition stays in the box. Reserved for failures already surfaced + * elsewhere; throw to have the message rendered inline instead. */ + onCreateGoal: (condition: string) => boolean | void | Promise; + /** Open the session driving a goal — its transcript IS the goal's history. */ + onOpenSession: (sessionId: string) => void; + onError: (error: unknown, fallback: string) => void; +} + +export function GoalsDialog({ + onCreateGoal, + onOpenSession, + onError, +}: GoalsDialogProps) { + const { t } = useI18n(); + const actions = useWorkspaceActions(); + + const [goals, setGoals] = useState(null); + /** Sessions the daemon could not probe; their goals are missing from `goals`. */ + const [droppedCount, setDroppedCount] = useState(0); + const [loadError, setLoadError] = useState(null); + const [busySessionId, setBusySessionId] = useState(null); + + const [showForm, setShowForm] = useState(false); + const [condition, setCondition] = useState(''); + const [submitting, setSubmitting] = useState(false); + const [formError, setFormError] = useState(null); + + const [now, setNow] = useState(() => Date.now()); + + const mountedRef = useRef(true); + // Monotonic reload id: a slow poll that resolves after a clear's reload must + // not resurrect the cleared goal. Only the latest reload may apply. + const reloadSeqRef = useRef(0); + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + }; + }, []); + + const reload = useCallback(async () => { + const seq = ++reloadSeqRef.current; + try { + const list = await actions.listGoals(); + if (!mountedRef.current || seq !== reloadSeqRef.current) return; + setGoals(list.goals); + setDroppedCount(list.droppedCount); + setLoadError(null); + } catch (err) { + if (!mountedRef.current || seq !== reloadSeqRef.current) return; + setLoadError(err instanceof Error ? err.message : String(err)); + setGoals((prev) => prev ?? []); + // The count described the previous, partially-probed list. This load + // reached nothing at all, so keeping it would pin a degraded banner + // reporting a partial probe that no longer happened. + setDroppedCount(0); + } + }, [actions]); + + // One self-chaining loop owns both the initial load and the polling: each + // fetch is scheduled only once the previous one has settled. `GET /goals` + // probes every live session, and a wedged child holds it for the bridge's + // ext-method timeout — the same order as this interval — so a fixed + // setInterval would stack overlapping fan-outs. `withActionTimeout` rejects + // the wait but does not abort the request, so those would keep running. + useEffect(() => { + let cancelled = false; + let timer = 0; + const run = async () => { + await reload(); + if (cancelled) return; + timer = window.setTimeout(() => void run(), RELOAD_INTERVAL_MS); + }; + void run(); + return () => { + cancelled = true; + window.clearTimeout(timer); + }; + }, [reload]); + + // Only tick the elapsed column while something is actually elapsing. + const hasGoals = !!goals?.length; + useEffect(() => { + if (!hasGoals) return; + const id = window.setInterval(() => setNow(Date.now()), TICK_INTERVAL_MS); + return () => window.clearInterval(id); + }, [hasGoals]); + + const resetForm = useCallback(() => { + setCondition(''); + setFormError(null); + setShowForm(false); + }, []); + + const handleSubmit = useCallback(async () => { + const trimmed = condition.trim(); + if (trimmed.length === 0) { + setFormError(t('goals.error.emptyCondition')); + return; + } + // No length cap: `/goal` accepts a condition of any length, and refusing + // one here that the daemon would accept only splits the two surfaces. + // + // The condition travels to the daemon as `/goal `, so a bare + // clear keyword arrives as a clear command: the fresh session would drop + // the goal the instant it was set, with nothing to show for it. + if (isGoalClearKeyword(trimmed)) { + setFormError(t('goals.error.clearKeyword', { word: trimmed })); + return; + } + setSubmitting(true); + setFormError(null); + try { + const created = await onCreateGoal(trimmed); + if (!mountedRef.current) return; + // No goal was started, and the caller already said why. Resetting here + // would close the form and drop the condition the user typed. + if (created === false) return; + resetForm(); + } catch (err) { + if (!mountedRef.current) { + // The page closed while the prompt was in flight, so the inline form + // error has nowhere to render. Toast rather than swallow it. + onError(err, t('goals.error.createFailed')); + return; + } + setFormError(err instanceof Error ? err.message : String(err)); + } finally { + if (mountedRef.current) setSubmitting(false); + } + }, [condition, onCreateGoal, onError, resetForm, t]); + + const handleClear = useCallback( + async (goal: DaemonGoal) => { + const label = + goal.condition.length > 60 + ? `${goal.condition.slice(0, 57)}…` + : goal.condition; + if (!window.confirm(t('goals.clearConfirm', { condition: label }))) { + return; + } + setBusySessionId(goal.sessionId); + try { + await actions.clearGoal(goal.sessionId); + await reload(); + } catch (err) { + onError(err, t('goals.error.clearFailed')); + } finally { + if (mountedRef.current) setBusySessionId(null); + } + }, + [actions, onError, reload, t], + ); + + return ( +
+
{t('goals.subtitle')}
+ +
+
+ {goals === null + ? t('goals.loading') + : t('goals.count', { count: goals.length })} +
+
+ + +
+
+ + {showForm && ( + +
+