fix(tui): keep pending follow-up actions hidden after navigation (#130458)

* fix(tui): keep pending follow-up actions hidden after navigation

Separate pending task operations from manually hidden prompts and fence completion after controller disposal. Cover navigation and shutdown with controller and real-terminal regressions. Isolate SMS activation credentials in hermetic test environments exposed by the broad TUI QA run.

* test(tui): isolate pending task terminal regressions

* test(tui): include task suggestions in PTY routing assertion
This commit is contained in:
Peter Steinberger
2026-08-26 17:56:50 -07:00
committed by GitHub
parent 98017ad71e
commit 85c24bf05c
9 changed files with 288 additions and 83 deletions
+1
View File
@@ -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",
];
@@ -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() {
@@ -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;
@@ -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,
);
+104 -10
View File
@@ -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<string, unknown> = {}) {
};
}
function deferred<T>() {
let resolve!: (value: T) => void;
const promise = new Promise<T>((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<unknown[]>();
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", {
+33 -22
View File
@@ -247,6 +247,7 @@ export function createTuiTaskSuggestionController(deps: TaskSuggestionController
((items: SelectItem[]) => new SelectList(items, items.length, selectListTheme));
const suggestions = new Map<string, TaskSuggestion>();
const hiddenIds = new Set<string>();
const resolvingIds = new Set<string>();
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) {
+17
View File
@@ -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");
+20 -17
View File
@@ -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");
+1
View File
@@ -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",
];