diff --git a/src/infra/vitest-e2e-config.test.ts b/src/infra/vitest-e2e-config.test.ts index b71232c2df23..8ec4bdd2d7b1 100644 --- a/src/infra/vitest-e2e-config.test.ts +++ b/src/infra/vitest-e2e-config.test.ts @@ -39,6 +39,7 @@ describe("e2e vitest config", () => { "src/tui/tui-pty-harness.e2e.test.ts", "src/tui/tui-session-identity-pty.e2e.test.ts", "src/tui/tui-reset-transition-pty.e2e.test.ts", + "src/tui/tui-task-suggestions-pty.e2e.test.ts", "src/tui/tui-pty-local.e2e.test.ts", ]; diff --git a/src/tui/tui-pty-harness-fixture-test-support.ts b/src/tui/tui-pty-harness-fixture-test-support.ts index 5d8ab66fccfc..f2097b7f01f5 100644 --- a/src/tui/tui-pty-harness-fixture-test-support.ts +++ b/src/tui/tui-pty-harness-fixture-test-support.ts @@ -14,6 +14,7 @@ import { TUI_PTY_RENDERING_FIXTURE_SCRIPT } from "./tui-pty-rendering-test-suppo import { TUI_PTY_RESET_FIXTURE } from "./tui-pty-reset-fixture-test-support.js"; import { TUI_PTY_STARTUP_SESSION_FIXTURE } from "./tui-pty-startup-session-fixture-test-support.js"; import { TUI_PTY_SESSION_SUBSCRIPTION_FIXTURE_SCRIPT } from "./tui-pty-subscription-fixture-test-support.js"; +import { TUI_PTY_TASK_FIXTURE } from "./tui-pty-task-fixture-test-support.js"; import { startPty, type PtyRun } from "./tui-pty-test-support.js"; export * from "./tui-pty-harness-assertion-test-support.js"; @@ -145,16 +146,7 @@ export async function writeTuiPtyFixtureScript(dir: string) { ? pluginApproval(initialPluginApprovalSessionKey) : null; let pendingPluginApprovalRun: { runId: string; sessionKey: string } | null = null; - let pendingTaskSuggestion: { - id: string; - title: string; - prompt: string; - tldr: string; - cwd: string; - sessionKey: string; - agentId: string; - createdAt: number; - } | null = null; + ${TUI_PTY_TASK_FIXTURE.variables} function record(method: string, payload?: unknown) { if (!actionLogPath) { @@ -644,30 +636,7 @@ export async function writeTuiPtyFixtureScript(dir: string) { return { ok: true }; } - async listTaskSuggestions() { - record("listTaskSuggestions", { pending: Boolean(pendingTaskSuggestion) }); - return pendingTaskSuggestion ? [pendingTaskSuggestion] : []; - } - - async acceptTaskSuggestion(taskId: string) { - record("acceptTaskSuggestion", { taskId }); - pendingTaskSuggestion = null; - this.onEvent?.({ - event: "task.suggestion", - payload: { action: "resolved", taskId, resolution: "accepted" }, - }); - return { taskId, key: "agent:main:task-pty" }; - } - - async dismissTaskSuggestion(taskId: string) { - record("dismissTaskSuggestion", { taskId }); - pendingTaskSuggestion = null; - this.onEvent?.({ - event: "task.suggestion", - payload: { action: "resolved", taskId, resolution: "dismissed" }, - }); - return { taskId, dismissed: true }; - } + ${TUI_PTY_TASK_FIXTURE.methods} } async function main() { diff --git a/src/tui/tui-pty-task-fixture-test-support.ts b/src/tui/tui-pty-task-fixture-test-support.ts new file mode 100644 index 000000000000..20fcf8e80eb6 --- /dev/null +++ b/src/tui/tui-pty-task-fixture-test-support.ts @@ -0,0 +1,52 @@ +// Keeps pending task actions controllable through the real terminal input loop. +export const TUI_PTY_TASK_FIXTURE = { + variables: ` + let pendingTaskSuggestion: { + id: string; + title: string; + prompt: string; + tldr: string; + cwd: string; + sessionKey: string; + agentId: string; + createdAt: number; + } | null = null; + + async function waitForTaskRelease() { + const releasePath = process.env.OPENCLAW_TUI_PTY_TASK_RELEASE_PATH; + if (releasePath) { + while (!existsSync(releasePath)) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + } + } + `, + methods: ` + async listTaskSuggestions() { + record("listTaskSuggestions", { pending: Boolean(pendingTaskSuggestion) }); + return pendingTaskSuggestion ? [pendingTaskSuggestion] : []; + } + + async acceptTaskSuggestion(taskId: string) { + record("acceptTaskSuggestion", { taskId }); + await waitForTaskRelease(); + pendingTaskSuggestion = null; + this.onEvent?.({ + event: "task.suggestion", + payload: { action: "resolved", taskId, resolution: "accepted" }, + }); + return { taskId, key: "agent:main:task-pty" }; + } + + async dismissTaskSuggestion(taskId: string) { + record("dismissTaskSuggestion", { taskId }); + await waitForTaskRelease(); + pendingTaskSuggestion = null; + this.onEvent?.({ + event: "task.suggestion", + payload: { action: "resolved", taskId, resolution: "dismissed" }, + }); + return { taskId, dismissed: true }; + } + `, +} as const; diff --git a/src/tui/tui-task-suggestions-pty.e2e.test.ts b/src/tui/tui-task-suggestions-pty.e2e.test.ts new file mode 100644 index 000000000000..05e46f4488c2 --- /dev/null +++ b/src/tui/tui-task-suggestions-pty.e2e.test.ts @@ -0,0 +1,57 @@ +import { writeFile } from "node:fs/promises"; +import path from "node:path"; +import { afterEach, expect, it } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; +import { + readFixtureLog, + startTuiFixture, + waitForSynchronizedFrameRows, +} from "./tui-pty-harness-fixture-test-support.js"; + +const STARTUP_TIMEOUT_MS = 20_000; +const TEST_TIMEOUT_MS = 25_000; +const tempDirs = useAutoCleanupTempDirTracker(afterEach); + +it.each(["accept", "dismiss"])( + "does not reopen a task during pending %s after returning to its session", + async (action) => { + const tempDir = tempDirs.make("openclaw-tui-task-action-"); + const releasePath = path.join(tempDir, "release"); + const fixture = await startTuiFixture({ + env: { OPENCLAW_TUI_PTY_TASK_RELEASE_PATH: releasePath }, + }); + try { + await fixture.run.waitForOutput("local ready", STARTUP_TIMEOUT_MS); + await fixture.run.write("/session pending-source\r"); + await fixture.run.waitForOutput("session agent:main:pending-source"); + await fixture.run.write("task suggestion proof\r"); + await fixture.run.waitForOutput("Suggested follow-up: Remove stale adapter"); + if (action === "accept") { + await fixture.run.write("\x1b[A\r", { delay: false }); + await fixture.run.waitForOutput("Press Enter again to start this task."); + } + await fixture.run.write("\r", { delay: false }); + await fixture.waitForLogEntry((entry) => entry.method === `${action}TaskSuggestion`); + + await fixture.run.write("/session pending-other\r"); + await fixture.run.waitForOutput("session agent:main:pending-other"); + await fixture.run.write("/session pending-source\r"); + const rows = await waitForSynchronizedFrameRows( + fixture.run, + (frame) => frame.some((row) => row.includes("session agent:main:pending-source")), + 5_000, + ); + expect(rows.join("\n")).not.toContain("Suggested follow-up:"); + + await writeFile(releasePath, "release"); + await fixture.run.waitForOutput( + action === "accept" ? "session agent:main:task-pty" : "follow-up task dismissed", + ); + const calls = await readFixtureLog(fixture.logPath); + expect(calls.filter((entry) => entry.method === `${action}TaskSuggestion`)).toHaveLength(1); + } finally { + await fixture.cleanup(); + } + }, + TEST_TIMEOUT_MS, +); diff --git a/src/tui/tui-task-suggestions.test.ts b/src/tui/tui-task-suggestions.test.ts index fcd032e4e981..701daa3b5ccc 100644 --- a/src/tui/tui-task-suggestions.test.ts +++ b/src/tui/tui-task-suggestions.test.ts @@ -2,6 +2,7 @@ import type { Component, OverlayHandle, SelectItem } from "@earendil-works/pi-tu import { expectDefined } from "@openclaw/normalization-core"; import { describe, expect, it, vi } from "vitest"; import { stripAnsi } from "../../packages/terminal-core/src/ansi.js"; +import { createDeferred as deferred } from "../../test/helpers/promise.js"; import { createTuiTaskSuggestionController } from "./tui-task-suggestions.js"; type TestSelector = Component & { @@ -26,14 +27,6 @@ function suggestionPayload(overrides: Record = {}) { }; } -function deferred() { - let resolve!: (value: T) => void; - const promise = new Promise((resolvePromise) => { - resolve = resolvePromise; - }); - return { promise, resolve }; -} - function createHarness() { const selectors: TestSelector[] = []; const addSystem = vi.fn(); @@ -163,7 +156,7 @@ describe("TUI task suggestions", () => { harness.selectors[0]?.onSelect?.(accept); await vi.waitFor(() => { - expect(harness.acceptTaskSuggestion).toHaveBeenCalledWith("task_1"); + expect(harness.acceptTaskSuggestion).toHaveBeenCalledWith("task_1", "worktree", undefined); expect(harness.onAccepted).toHaveBeenCalledWith("agent:main:task"); }); expect(harness.addSystem).toHaveBeenCalledWith("follow-up task started in agent:main:task"); @@ -188,7 +181,7 @@ describe("TUI task suggestions", () => { harness.selectors[0]?.onSelect?.(action); await vi.waitFor(() => { - expect(harness.acceptTaskSuggestion).toHaveBeenCalledWith("task_1", mode); + expect(harness.acceptTaskSuggestion).toHaveBeenCalledWith("task_1", mode, undefined); }); if (mode === "session") { expect(harness.onAccepted).not.toHaveBeenCalled(); @@ -442,6 +435,107 @@ describe("TUI task suggestions", () => { expect(harness.onAccepted).not.toHaveBeenCalled(); }); + it.each(["accept", "dismiss"])( + "keeps an in-flight %s hidden across navigation and refresh", + async (value) => { + const harness = createHarness(); + const pending = deferred<{ taskId: string; key: string; dismissed: boolean }>(); + const action = + value === "accept" ? harness.acceptTaskSuggestion : harness.dismissTaskSuggestion; + action.mockReturnValueOnce(pending.promise); + harness.listTaskSuggestions.mockResolvedValue([suggestionPayload()]); + await harness.controller.refresh(); + const item = expectDefined( + harness.selectors[0]?.items.find((entry) => entry.value === value), + "pending task action", + ); + harness.selectors[0]?.onSelect?.(item); + if (value === "accept") { + harness.selectors[0]?.onSelect?.(item); + } + expect(action).toHaveBeenCalledTimes(1); + + harness.setSessionKey("agent:main:other"); + harness.controller.sessionChanged(); + harness.setSessionKey("agent:main:main"); + harness.controller.sessionChanged(); + await harness.controller.refresh(); + harness.controller.handleEvent("task.suggestion", { + action: "created", + suggestion: suggestionPayload(), + }); + expect(harness.openOverlay).toHaveBeenCalledTimes(1); + + pending.resolve({ taskId: "task_1", key: "agent:main:task", dismissed: true }); + await new Promise(setImmediate); + expect(action).toHaveBeenCalledTimes(1); + expect(harness.openOverlay).toHaveBeenCalledTimes(1); + expect(harness.addSystem).toHaveBeenCalledWith( + value === "accept" + ? "follow-up task started in agent:main:task" + : "follow-up task dismissed", + ); + }, + ); + + it.each([ + { value: "accept", fails: false }, + { value: "accept", fails: true }, + { value: "dismiss", fails: false }, + { value: "dismiss", fails: true }, + ])("ignores $value completion after disposal (fails=$fails)", async ({ value, fails }) => { + const harness = createHarness(); + const pending = deferred<{ taskId: string; key: string; dismissed: boolean }>(); + const action = + value === "accept" ? harness.acceptTaskSuggestion : harness.dismissTaskSuggestion; + action.mockReturnValueOnce(pending.promise); + harness.controller.handleEvent("task.suggestion", { + action: "created", + suggestion: suggestionPayload(), + }); + const item = expectDefined( + harness.selectors[0]?.items.find((entry) => entry.value === value), + "pending task action", + ); + harness.selectors[0]?.onSelect?.(item); + if (value === "accept") { + harness.selectors[0]?.onSelect?.(item); + } + expect(action).toHaveBeenCalledTimes(1); + harness.controller.dispose(); + harness.requestRender.mockClear(); + if (fails) { + pending.reject(new Error("gateway unavailable")); + } else { + pending.resolve({ taskId: "task_1", key: "agent:main:task", dismissed: true }); + } + await new Promise(setImmediate); + + expect(harness.onAccepted).not.toHaveBeenCalled(); + expect(harness.addSystem).not.toHaveBeenCalled(); + expect(harness.listTaskSuggestions).not.toHaveBeenCalled(); + expect(harness.requestRender).not.toHaveBeenCalled(); + }); + + it("ignores a failed recovery refresh after disposal", async () => { + const harness = createHarness(); + const pending = deferred(); + harness.dismissTaskSuggestion.mockRejectedValueOnce(new Error("gateway unavailable")); + harness.listTaskSuggestions.mockReturnValueOnce(pending.promise); + harness.controller.handleEvent("task.suggestion", { + action: "created", + suggestion: suggestionPayload(), + }); + harness.selectors[0]?.onSelect?.({ value: "dismiss", label: "Dismiss" }); + await vi.waitFor(() => expect(harness.listTaskSuggestions).toHaveBeenCalledTimes(1)); + harness.controller.dispose(); + harness.addSystem.mockClear(); + pending.reject(new Error("refresh unavailable")); + await new Promise(setImmediate); + + expect(harness.addSystem).not.toHaveBeenCalled(); + }); + it("shows only suggestions for the active session", () => { const harness = createHarness(); harness.controller.handleEvent("task.suggestion", { diff --git a/src/tui/tui-task-suggestions.ts b/src/tui/tui-task-suggestions.ts index dffa7a7dcd86..be1c3228c639 100644 --- a/src/tui/tui-task-suggestions.ts +++ b/src/tui/tui-task-suggestions.ts @@ -247,6 +247,7 @@ export function createTuiTaskSuggestionController(deps: TaskSuggestionController ((items: SelectItem[]) => new SelectList(items, items.length, selectListTheme)); const suggestions = new Map(); const hiddenIds = new Set(); + const resolvingIds = new Set(); let activeId: string | null = null; let activeOverlay: OverlayHandle | null = null; let activeSelector: TaskSelector | null = null; @@ -305,7 +306,9 @@ export function createTuiTaskSuggestionController(deps: TaskSuggestionController } const suggestion = [...suggestions.values()] .toSorted((left, right) => left.createdAt - right.createdAt) - .find((entry) => !hiddenIds.has(entry.id) && matchesSession(entry)); + .find( + (entry) => !hiddenIds.has(entry.id) && !resolvingIds.has(entry.id) && matchesSession(entry), + ); if (!suggestion) { return; } @@ -328,27 +331,21 @@ export function createTuiTaskSuggestionController(deps: TaskSuggestionController return; } closeActive(); - hiddenIds.add(suggestion.id); + // Navigation clears manual dismissals, not ownership of an unfinished action. + resolvingIds.add(suggestion.id); deps.requestRender(); try { + let acceptedKey: string | undefined; if (action.kind === "accept") { if (!deps.client.acceptTaskSuggestion) { throw new Error("task suggestion acceptance is unavailable"); } - const result = action.cloudProfileId - ? await deps.client.acceptTaskSuggestion( - suggestion.id, - action.mode, - action.cloudProfileId, - ) - : action.mode === "worktree" - ? await deps.client.acceptTaskSuggestion(suggestion.id) - : await deps.client.acceptTaskSuggestion(suggestion.id, action.mode); - remove(suggestion.id); - deps.chatLog.addSystem(`follow-up task started in ${result.key}`); - if (action.mode !== "session" && matchesSession(suggestion)) { - await deps.onAccepted(result.key); - } + const result = await deps.client.acceptTaskSuggestion( + suggestion.id, + action.mode, + action.cloudProfileId, + ); + acceptedKey = result.key; } else { if (!deps.client.dismissTaskSuggestion) { throw new Error("task suggestion dismissal is unavailable"); @@ -357,17 +354,31 @@ export function createTuiTaskSuggestionController(deps: TaskSuggestionController if (!result.dismissed) { throw new Error("task suggestion is no longer pending"); } - remove(suggestion.id); - deps.chatLog.addSystem("follow-up task dismissed"); + } + if (disposed) { + return; + } + remove(suggestion.id); + deps.chatLog.addSystem( + acceptedKey ? `follow-up task started in ${acceptedKey}` : "follow-up task dismissed", + ); + if (acceptedKey && action.mode !== "session" && matchesSession(suggestion)) { + await deps.onAccepted(acceptedKey); } } catch (error) { - hiddenIds.delete(suggestion.id); + if (disposed) { + return; + } deps.chatLog.addSystem(`follow-up task failed: ${formatErrorMessage(error)}`); void refresh().catch((refreshError: unknown) => { - deps.chatLog.addSystem( - `task suggestion refresh failed: ${formatErrorMessage(refreshError)}`, - ); + if (!disposed) { + deps.chatLog.addSystem( + `task suggestion refresh failed: ${formatErrorMessage(refreshError)}`, + ); + } }); + } finally { + resolvingIds.delete(suggestion.id); } presentNext(); if (!disposed) { diff --git a/test/test-env.test.ts b/test/test-env.test.ts index 072422dd23a2..5cb9f9bfe163 100644 --- a/test/test-env.test.ts +++ b/test/test-env.test.ts @@ -389,6 +389,23 @@ describe("installTestEnv", () => { expect(process.env.OPENCLAW_HOME).toBe(configuredOpenClawHome); }); + it.each([ + "TWILIO_ACCOUNT_SID", + "TWILIO_AUTH_TOKEN", + "TWILIO_PHONE_NUMBER", + "TWILIO_SMS_FROM", + "TWILIO_MESSAGING_SERVICE_SID", + ])("isolates and restores the SMS activation variable %s", (key) => { + setTestEnvValue(key, "test-channel-value"); + + const testEnv = installTestEnv({ mode: "hermetic" }); + cleanupFns.push(testEnv.cleanup); + + expect(process.env[key]).toBeUndefined(); + testEnv.cleanup(); + expect(process.env[key]).toBe("test-channel-value"); + }); + it("does not load ~/.profile for normal isolated test runs", () => { const realHome = createTempHome(); writeFile(path.join(realHome, ".profile"), "export TEST_PROFILE_ONLY=from-profile\n"); diff --git a/test/test-env.ts b/test/test-env.ts index bb872392e017..cc2fe3ec8366 100644 --- a/test/test-env.ts +++ b/test/test-env.ts @@ -15,6 +15,21 @@ type InstallTestEnvOptions = | { mode: "hermetic" }; const LIVE_TEST_TRIGGER_ENV_KEYS = ["LIVE", "OPENCLAW_LIVE_TEST", "OPENCLAW_LIVE_GATEWAY"] as const; +const ISOLATED_TEST_CREDENTIAL_ENV_KEYS = [ + "TELEGRAM_BOT_TOKEN", + "DISCORD_BOT_TOKEN", + "SLACK_BOT_TOKEN", + "SLACK_APP_TOKEN", + "SLACK_USER_TOKEN", + "TWILIO_ACCOUNT_SID", + "TWILIO_AUTH_TOKEN", + "TWILIO_PHONE_NUMBER", + "TWILIO_SMS_FROM", + "TWILIO_MESSAGING_SERVICE_SID", + "COPILOT_GITHUB_TOKEN", + "GH_TOKEN", + "GITHUB_TOKEN", +] as const; const HERMETIC_TEST_ENV_KEYS = [ ...LIVE_TEST_TRIGGER_ENV_KEYS, "OPENCLAW_LIVE_USE_REAL_HOME", @@ -204,14 +219,7 @@ function resolveRestoreEntries(): RestoreEntry[] { { key: "OPENCLAW_CANVAS_HOST_PORT", value: process.env.OPENCLAW_CANVAS_HOST_PORT }, { key: "OPENCLAW_TEST_HOME", value: process.env.OPENCLAW_TEST_HOME }, { key: "OPENCLAW_AGENT_DIR", value: process.env.OPENCLAW_AGENT_DIR }, - { key: "TELEGRAM_BOT_TOKEN", value: process.env.TELEGRAM_BOT_TOKEN }, - { key: "DISCORD_BOT_TOKEN", value: process.env.DISCORD_BOT_TOKEN }, - { key: "SLACK_BOT_TOKEN", value: process.env.SLACK_BOT_TOKEN }, - { key: "SLACK_APP_TOKEN", value: process.env.SLACK_APP_TOKEN }, - { key: "SLACK_USER_TOKEN", value: process.env.SLACK_USER_TOKEN }, - { key: "COPILOT_GITHUB_TOKEN", value: process.env.COPILOT_GITHUB_TOKEN }, - { key: "GH_TOKEN", value: process.env.GH_TOKEN }, - { key: "GITHUB_TOKEN", value: process.env.GITHUB_TOKEN }, + ...ISOLATED_TEST_CREDENTIAL_ENV_KEYS.map((key) => ({ key, value: process.env[key] })), { key: "NODE_OPTIONS", value: process.env.NODE_OPTIONS }, ]; } @@ -242,15 +250,10 @@ function createIsolatedTestHome(restore: RestoreEntry[]): { deleteTestEnvValue("OPENCLAW_BRIDGE_HOST"); deleteTestEnvValue("OPENCLAW_BRIDGE_PORT"); deleteTestEnvValue("OPENCLAW_CANVAS_HOST_PORT"); - // Avoid leaking real GitHub/Copilot tokens into non-live test runs. - deleteTestEnvValue("TELEGRAM_BOT_TOKEN"); - deleteTestEnvValue("DISCORD_BOT_TOKEN"); - deleteTestEnvValue("SLACK_BOT_TOKEN"); - deleteTestEnvValue("SLACK_APP_TOKEN"); - deleteTestEnvValue("SLACK_USER_TOKEN"); - deleteTestEnvValue("COPILOT_GITHUB_TOKEN"); - deleteTestEnvValue("GH_TOKEN"); - deleteTestEnvValue("GITHUB_TOKEN"); + // Ambient channel credentials can activate real plugins even with an isolated HOME. + for (const key of ISOLATED_TEST_CREDENTIAL_ENV_KEYS) { + deleteTestEnvValue(key); + } // Avoid leaking local dev tooling flags into tests (e.g. --inspect). deleteTestEnvValue("NODE_OPTIONS"); diff --git a/test/vitest/vitest.test-shards.mjs b/test/vitest/vitest.test-shards.mjs index 200479f54229..d8f44eb06d4d 100644 --- a/test/vitest/vitest.test-shards.mjs +++ b/test/vitest/vitest.test-shards.mjs @@ -17,6 +17,7 @@ export const tuiPtyTestFiles = [ "src/tui/tui-pty-harness.e2e.test.ts", "src/tui/tui-session-identity-pty.e2e.test.ts", "src/tui/tui-reset-transition-pty.e2e.test.ts", + "src/tui/tui-task-suggestions-pty.e2e.test.ts", "src/tui/tui-pty-local.e2e.test.ts", ];