fix(web-shell): stop rendering unrecognized daemon events in transcripts #8812
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2522,6 +2522,118 @@ describe('daemon UI normalizer — Wave 3/4 event coverage (PR-A)', () => { | |
| expect(events).toEqual([]); | ||
| }); | ||
|
|
||
| it('stamps debugReason on unrecognized daemon events', () => { | ||
| const events = normalizeDaemonEvent( | ||
| envelopeOf('some_future_event', { sessionId: 's1' }), | ||
| ); | ||
|
|
||
| expect(events).toEqual([ | ||
| expect.objectContaining({ | ||
| type: 'debug', | ||
| debugReason: 'unrecognized_event', | ||
| }), | ||
| ]); | ||
| }); | ||
|
|
||
| it('stamps debugReason on unrecognized session_update kinds', () => { | ||
| const events = normalizeDaemonEvent( | ||
| envelopeOf('session_update', { | ||
| update: { sessionUpdate: 'some_future_kind', payload: { a: 1 } }, | ||
| }), | ||
| ); | ||
|
|
||
| expect(events).toEqual([ | ||
| expect.objectContaining({ | ||
| type: 'debug', | ||
| debugReason: 'unrecognized_session_update', | ||
| }), | ||
| ]); | ||
| }); | ||
|
|
||
| it('classifies a session_update with no usable discriminator as malformed', () => { | ||
| // `getSessionUpdatePayload` accepts any record, so these reach the default | ||
| // branch with `kind === undefined`. They are broken frames, not kinds from | ||
| // a newer daemon — marking them unrecognized would let renderers hide the | ||
| // only diagnostic they produce. | ||
| for (const update of [ | ||
| {}, | ||
| { sessionUpdate: 42 }, | ||
| { sessionUpdate: '' }, | ||
| // Truthy but no more usable than an empty string. | ||
| { sessionUpdate: ' ' }, | ||
| ]) { | ||
| expect( | ||
| normalizeDaemonEvent(envelopeOf('session_update', { update })), | ||
| ).toEqual([ | ||
| expect.objectContaining({ | ||
| type: 'debug', | ||
| debugReason: 'malformed_payload', | ||
| }), | ||
| ]); | ||
| } | ||
| }); | ||
|
|
||
| it('stamps debugReason on malformed payloads of known events', () => { | ||
| const events = normalizeDaemonEvent( | ||
| envelopeOf('memory_changed', { scope: 'not-a-scope' }), | ||
| ); | ||
|
|
||
| expect(events).toEqual([ | ||
| expect.objectContaining({ | ||
| type: 'debug', | ||
| debugReason: 'malformed_payload', | ||
| }), | ||
| ]); | ||
| }); | ||
|
|
||
| it('carries debugReason through the reducer onto the transcript block', () => { | ||
| // The normalizer tests above inspect events directly and the Web Shell | ||
| // adapter tests construct blocks by hand, so neither would notice if the | ||
| // reducer dropped the field on the way across. Production blocks would | ||
| // then lose their classification and Web Shell would render raw JSON | ||
| // again with both suites still green. | ||
| const state = reduceDaemonTranscriptEvents( | ||
| createDaemonTranscriptState({ now: 1 }), | ||
| normalizeDaemonEvent( | ||
| envelopeOf('some_future_event', { sessionId: 's1' }), | ||
| ), | ||
| ); | ||
|
|
||
| expect(state.blocks).toEqual([ | ||
| expect.objectContaining({ | ||
| kind: 'debug', | ||
| debugReason: 'unrecognized_event', | ||
| }), | ||
| ]); | ||
|
Comment on lines
+2602
to
+2607
Collaborator
There was a problem hiding this comment. [Suggestion] This round-trip test pins only the positive direction of the contract (normalizer event → block carries Suggested companion test next to this one (the dispatch shape mirrors it('keeps client-dispatched debug blocks free of debugReason', () => {
const state = reduceDaemonTranscriptEvents(
createDaemonTranscriptState({ now: 1 }),
[
{
type: 'debug',
text: 'Model switched to qwen3-coder-plus',
source: 'model_switch_summary',
},
],
);
expect(state.blocks).toHaveLength(1);
expect(state.blocks[0]).toEqual(expect.objectContaining({ kind: 'debug' }));
expect(state.blocks[0]).not.toHaveProperty('debugReason');
});中文说明这个往返测试只固定了契约的正向(normalizer 事件 → 块携带 — qwen3.8-max via Qwen Code /review (v0.21.8)
Collaborator
Author
There was a problem hiding this comment. Added in bc32742, and I reproduced your mutant first: |
||
| }); | ||
|
|
||
| it('leaves client-dispatched debug blocks without a debugReason', () => { | ||
| // The mirror of the test above, and the invariant that keeps Web Shell's | ||
| // model-switch summary visible. Without it, defaulting the field in | ||
| // `appendStatusBlock` (e.g. `event.debugReason ?? 'unrecognized_event'`) | ||
| // passes every other test in both suites while silently tagging the | ||
| // summary as unrecognized, which Web Shell then filters out. | ||
| const state = reduceDaemonTranscriptEvents( | ||
| createDaemonTranscriptState({ now: 1 }), | ||
| [ | ||
| { | ||
| type: 'debug', | ||
| text: 'Model switched to qwen3-coder-plus', | ||
| source: 'model_switch_summary', | ||
| }, | ||
| ], | ||
| ); | ||
|
|
||
| expect(state.blocks).toHaveLength(1); | ||
| expect(state.blocks[0]).toEqual( | ||
| expect.objectContaining({ | ||
| kind: 'debug', | ||
| source: 'model_switch_summary', | ||
| }), | ||
| ); | ||
| expect(state.blocks[0]).not.toHaveProperty('debugReason'); | ||
| }); | ||
|
|
||
| it('normalizes memory_changed with closed-enum scope + mode', () => { | ||
| const events = normalizeDaemonEvent( | ||
| envelopeOf('memory_changed', { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion]
DaemonUiDebugReasonis a new public type of@qwen-code/sdk/daemon, but nothing gates that public surface: no test imports it through this barrel, so if the re-export were dropped or the type renamed without updating the barrel, every suite stays green (the only in-repo referents aretypes.tsitself and the two re-export lines, and an esbuild-strippedexport typecan never fail at runtime) while external consumers lose the documented union type — the first signal would be theirtscerror after upgrade. — Failure scenario: a future refactor drops this re-export → all builds and tests pass → the advertised public API silently shrinks until a consumer's compile breaks. (Note: the innerdaemon/uibarrel IS transitively guarded — this outer barrel re-exports from./ui/index.js, so deleting the inner line fails the sdk build; only the outermost export needs an explicit guard.)中文说明
DaemonUiDebugReason是@qwen-code/sdk/daemon新增的公开类型,但这一公开面无任何守护:没有任何测试经公共 barrel 导入它,因此若某次重构丢失该再导出、或类型改名后忘了同步,所有构建与套件依旧全绿(仓内只有types.ts与两行再导出引用它,而export type会被 esbuild 擦除、运行时永不报错),外部消费者则无声地失去文档承诺的联合类型——第一个信号是他们升级后自己的tsc报错。——失败场景:未来的重构删掉了这一行再导出,构建测试全部通过,公开 API 静默缩水,直到消费者的编译中断。(说明:内层daemon/uibarrel 其实已被传递守护——外层 barrel 从./ui/index.js再导出,删除内层那行会让 sdk 构建直接报错;真正需要显式守护的只有这个最外层导出。)— Kimi-K3 via Qwen Code /review (v0.21.6)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, and the suggested guard turned out not to hold — worth flagging since it affects other type-only guards in this package.
I implemented it as written first, then mutation-checked by deleting the re-export from
src/daemon/index.ts. Bothvitest run(14 passed) andnpm -w packages/sdk-typescript run typecheckstayed green. Two reasons: vitest transpiles through esbuild, which erasesexport typewithout checking it, and this package's tsconfig isinclude: ["src/**/*.ts"]withexclude: [..., "test"], so nothing type-checks the test file at all.expectTypeOfalone cannot fence this surface here.So in bc32742 the union ships as a closed enum value instead, matching
DAEMON_ERROR_KINDS/DAEMON_APPROVAL_MODES:Re-exported as a value through both barrels, with a runtime
toEqualassertion next to theexpectTypeOfyou suggested. Mutation-checked: dropping the outer re-export now fails that test.