Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/cli/src/ui/utils/goal-runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,36 @@ describe('waitForGoalRuntime', () => {
expect(getGoalRuntimeReady).toHaveBeenCalledTimes(1);
});

it('treats a synchronous persistence-unavailable throw as settled', async () => {
// Config.getGoalRuntimeReady() THROWS synchronously (rather than
// rejecting) when chat recording is disabled, e.g. --no-chat-recording.
// The startup gate must swallow that throw exactly like the rejected
// promise above; an escaped rejection kills the AppContainer init
// effect and freezes the TUI on the init banner forever (#10311).
const getGoalRuntimeReady = vi.fn((): Promise<GoalRuntime> => {
throw new GoalPersistenceUnavailableError();
});

await expect(waitForGoalRuntime({ getGoalRuntimeReady })).resolves.toBe(
true,
);
await expect(
waitForGoalRuntime({ getGoalRuntimeReady }, { timeoutMs: 100 }),
).resolves.toBe(true);
expect(getGoalRuntimeReady).toHaveBeenCalledTimes(2);
});

it('does not hide a synchronous throw of any other error', async () => {
const failure = new Error('unsupported Goal lifecycle record');
const getGoalRuntimeReady = vi.fn((): Promise<GoalRuntime> => {
throw failure;
});

await expect(waitForGoalRuntime({ getGoalRuntimeReady })).rejects.toBe(
failure,
);
});

it('does not hide malformed or unsupported persisted Goal state', async () => {
const failure = new Error('unsupported Goal lifecycle record');
const getGoalRuntimeReady = vi.fn().mockRejectedValue(failure);
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/ui/utils/goal-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ export async function waitForGoalRuntime(
config: Pick<Config, 'getGoalRuntimeReady'>,
options: { timeoutMs?: number } = {},
): Promise<boolean> {
const ready = config.getGoalRuntimeReady();
const awaitReady = async (): Promise<void> => {
try {
await ready;
// The call must stay inside the try: with chat recording disabled
// (--no-chat-recording, settings) getGoalRuntimeReady() THROWS
// synchronously instead of rejecting, and an escaped rejection kills
// the AppContainer init effect, freezing the TUI on the init banner
// forever.
await config.getGoalRuntimeReady();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified real against the code: Config.getGoalRuntimeReady() (packages/core/src/config/config.ts:8462) reaches a synchronous throw new GoalPersistenceUnavailableError() via getGoalRuntime() when chat recording is disabled, while the documented contract (packages/core/src/goals/goal-protocol.ts:224) says it rejects — and getGoalRuntimePrepared() carries the same shape. The fix (making both methods async so the sync throw becomes a rejection) lives in packages/core, outside this PR's footprint, and the finding itself marks it a maintainer follow-up — so it is deferred to the follow-up queue rather than implemented here. This PR's gate fix, the "must stay inside the try" comment, and the tests remain as defense-in-depth until that core change lands.

中文说明

已对照代码核实为真实问题:聊天记录被禁用时,Config.getGoalRuntimeReady()(packages/core/src/config/config.ts:8462)会经由 getGoalRuntime() 同步 throw new GoalPersistenceUnavailableError,而文档契约(packages/core/src/goals/goal-protocol.ts:224)写的是拒绝(reject)—— getGoalRuntimePrepared() 也带有同样的形态。修复方案(把两个方法改为 async,让同步抛出变成拒绝)位于 packages/core,超出本 PR 的足迹,且该意见本身也标注为维护者后续跟进事项 —— 因此将其延后到后续队列,而不在本 PR 实现。在 core 改动落地之前,本 PR 的门禁修复、“必须留在 try 内”注释与测试作为纵深防御保留。

} catch (error) {
if (!(error instanceof GoalPersistenceUnavailableError)) throw error;
}
Expand Down
Loading