From 93e5b017bf04184875141ddbddc44a98454d23ae Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 11 Jul 2026 15:14:02 -0700 Subject: [PATCH] =?UTF-8?q?test(channels):=20trace-harness=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20scenario=20names,=20fault=20vocabulary,=20lane=20sy?= =?UTF-8?q?nc=20guard=20(#104653)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- extensions/feishu/src/delivery-trace.test.ts | 3 + extensions/matrix/src/delivery-trace.test.ts | 89 ++++++++++--------- extensions/msteams/src/delivery-trace.test.ts | 3 + extensions/qqbot/src/delivery-trace.test.ts | 66 +++++++------- extensions/slack/src/delivery-trace.test.ts | 11 +-- .../telegram/src/delivery-trace.test.ts | 3 + scripts/test-projects.test-support.d.mts | 1 + scripts/test-projects.test-support.mjs | 6 +- .../plugins/contracts/trace/delivery-trace.ts | 22 ++++- test/scripts/test-projects.test.ts | 22 +++++ 10 files changed, 136 insertions(+), 90 deletions(-) diff --git a/extensions/feishu/src/delivery-trace.test.ts b/extensions/feishu/src/delivery-trace.test.ts index 5c7793974314..9873efe126e5 100644 --- a/extensions/feishu/src/delivery-trace.test.ts +++ b/extensions/feishu/src/delivery-trace.test.ts @@ -398,6 +398,9 @@ function setupFeishuTrace(recorder: WireRecorder, scenario: DeliveryTraceScenari options.onCleanup?.(); break; case "wire-fault": + if (step.fault !== "rate-limit") { + throw new Error("feishu trace scenarios script only rate-limit wire faults"); + } traceState.wireFaults.push({ fault: step.fault, retryAfterMs: step.retryAfterMs }); break; } diff --git a/extensions/matrix/src/delivery-trace.test.ts b/extensions/matrix/src/delivery-trace.test.ts index 086b88b734d3..67dd2dc4722c 100644 --- a/extensions/matrix/src/delivery-trace.test.ts +++ b/extensions/matrix/src/delivery-trace.test.ts @@ -17,8 +17,6 @@ import { runDeliveryTraceScenario, type DeliveryTraceInStep, type DeliveryTraceScenario, - type DeliveryTraceScenarioName, - type DeliveryTraceStep, type WireRecorder, } from "openclaw/plugin-sdk/channel-contract-testing"; import { @@ -277,14 +275,6 @@ async function setupMatrixTrace(recorder: WireRecorder) { }; } -// Matrix records generation-scoped edit-streaming shapes the shared v1 -// library does not model; the runner treats the scenario name as opaque data -// (it only keys the golden filename), so widening the closed union stays -// test-local instead of leaking matrix names into the core scenario library. -function matrixScenario(name: string, steps: readonly DeliveryTraceStep[]): DeliveryTraceScenario { - return { name: name as DeliveryTraceScenarioName, steps }; -} - const GEN_ONE_PARTIAL = "Build check:"; const GEN_ONE_BLOCK = "Build check: all suites green."; const GEN_TWO_PARTIAL = "Deploying to production now."; @@ -297,50 +287,61 @@ const MENTION_FINAL = "Paging @oncall:example.org for the deploy review."; // The draft stream throttles edits at 1000ms; each generation streams one // create plus one throttled m.replace edit before its boundary settles it. +// Matrix records generation-scoped edit-streaming shapes the shared v1 +// library does not model; the scenario name only keys the golden filename. const MATRIX_TRACE_SCENARIOS: readonly DeliveryTraceScenario[] = [ // Two generations separated by a tool round: one preview event per // generation, cumulative slices, block finalize-in-place for generation one, // and a final replace-edit because the final text differs from the last // rendered preview of generation two. - matrixScenario("streaming-happy-multi-generation", [ - { kind: "reply-start" }, - { kind: "partial", text: GEN_ONE_PARTIAL }, - { kind: "advance", ms: 400 }, - { kind: "partial", text: GEN_ONE_BLOCK }, - { kind: "advance", ms: 700 }, - { kind: "block-final", text: GEN_ONE_BLOCK }, - { kind: "tool-progress", name: "deploy", phase: "start" }, - { kind: "tool-progress", name: "deploy", phase: "result" }, - { kind: "partial", text: GEN_TWO_PARTIAL }, - { kind: "advance", ms: 400 }, - { kind: "partial", text: GEN_TWO_PREVIEW }, - { kind: "advance", ms: 700 }, - { kind: "final", text: GEN_TWO_FINAL }, - { kind: "idle" }, - ]), + { + name: "streaming-happy-multi-generation", + steps: [ + { kind: "reply-start" }, + { kind: "partial", text: GEN_ONE_PARTIAL }, + { kind: "advance", ms: 400 }, + { kind: "partial", text: GEN_ONE_BLOCK }, + { kind: "advance", ms: 700 }, + { kind: "block-final", text: GEN_ONE_BLOCK }, + { kind: "tool-progress", name: "deploy", phase: "start" }, + { kind: "tool-progress", name: "deploy", phase: "result" }, + { kind: "partial", text: GEN_TWO_PARTIAL }, + { kind: "advance", ms: 400 }, + { kind: "partial", text: GEN_TWO_PREVIEW }, + { kind: "advance", ms: 700 }, + { kind: "final", text: GEN_TWO_FINAL }, + { kind: "idle" }, + ], + }, // Final text equals the last rendered preview: the draft is finalized in // place with an edit that only clears the MSC4357 live marker. - matrixScenario("final-in-place", [ - { kind: "reply-start" }, - { kind: "partial", text: IN_PLACE_PARTIAL }, - { kind: "advance", ms: 400 }, - { kind: "partial", text: IN_PLACE_FINAL }, - { kind: "advance", ms: 700 }, - { kind: "final", text: IN_PLACE_FINAL }, - { kind: "idle" }, - ]), + { + name: "final-in-place", + steps: [ + { kind: "reply-start" }, + { kind: "partial", text: IN_PLACE_PARTIAL }, + { kind: "advance", ms: 400 }, + { kind: "partial", text: IN_PLACE_FINAL }, + { kind: "advance", ms: 700 }, + { kind: "final", text: IN_PLACE_FINAL }, + { kind: "idle" }, + ], + }, // Previews are mentions-inert and an edit cannot retro-notify, so a final // whose text would activate mentions redacts the preview and re-sends the // final as a fresh mention-bearing event. - matrixScenario("final-mentions-fresh", [ - { kind: "reply-start" }, - { kind: "partial", text: MENTION_PARTIAL }, - { kind: "advance", ms: 400 }, - { kind: "partial", text: MENTION_FINAL }, - { kind: "advance", ms: 700 }, - { kind: "final", text: MENTION_FINAL }, - { kind: "idle" }, - ]), + { + name: "final-mentions-fresh", + steps: [ + { kind: "reply-start" }, + { kind: "partial", text: MENTION_PARTIAL }, + { kind: "advance", ms: 400 }, + { kind: "partial", text: MENTION_FINAL }, + { kind: "advance", ms: 700 }, + { kind: "final", text: MENTION_FINAL }, + { kind: "idle" }, + ], + }, // Shared abandon shape: the handler's finally block flushes the pending // throttled edit, then redacts the unconsumed draft. deliveryTraceScenarios["cancel-mid-stream"], diff --git a/extensions/msteams/src/delivery-trace.test.ts b/extensions/msteams/src/delivery-trace.test.ts index 7b6c8e8a1275..34d2799157d4 100644 --- a/extensions/msteams/src/delivery-trace.test.ts +++ b/extensions/msteams/src/delivery-trace.test.ts @@ -307,6 +307,9 @@ function setupMSTeamsTrace(recorder: WireRecorder, traceCase: MSTeamsTraceCase) await created.markDispatchIdle(); break; case "wire-fault": + // The shared write-error fault vocabulary covers this shape, but a + // scripted step records a new IN event and would change the committed + // goldens; write-count arming at setup replays the same wire bytes. throw new Error("msteams trace scenarios arm stream write faults at setup instead"); } }; diff --git a/extensions/qqbot/src/delivery-trace.test.ts b/extensions/qqbot/src/delivery-trace.test.ts index 6524945b602b..b1dc10a3abae 100644 --- a/extensions/qqbot/src/delivery-trace.test.ts +++ b/extensions/qqbot/src/delivery-trace.test.ts @@ -29,7 +29,6 @@ import { runDeliveryTraceScenario, type DeliveryTraceInStep, type DeliveryTraceScenario, - type DeliveryTraceStep, type WireRecorder, } from "openclaw/plugin-sdk/channel-contract-testing"; import { chunkMarkdownText } from "openclaw/plugin-sdk/reply-runtime"; @@ -384,13 +383,6 @@ function setupQqbotTrace(recorder: WireRecorder, msgId: string) { }; } -// Channel-specific scenario names are not yet part of the shared -// DeliveryTraceScenarioName union; the runner only consumes steps and the -// golden is keyed by file name, so a local cast keeps the harness untouched. -function qqbotScenario(name: string, steps: readonly DeliveryTraceStep[]): DeliveryTraceScenario { - return { name: name as DeliveryTraceScenario["name"], steps }; -} - const MEDIA_INTERRUPT_FULL_TEXT = "Here is the chart:\nhttps://example.com/chart.png\nKey takeaways: ship it."; @@ -398,41 +390,47 @@ const QQBOT_TRACE_SCENARIOS: readonly DeliveryTraceScenario[] = [ // Official REPLACE-mode stream lifecycle over the shared streaming-happy // script: one stream session, cumulative GENERATING chunks, boundary joined // with "\n\n", DONE chunk sealing the full text. - qqbotScenario("streaming-happy-c2c", deliveryTraceScenarios["streaming-happy"].steps), + { name: "streaming-happy-c2c", steps: deliveryTraceScenarios["streaming-happy"].steps }, // Budget lifecycle against one msg_id: initial input_notify plus exactly // TYPING_RENEWAL_LIMIT (3) renewals, then a renewal-free tick proving the // reserved final reply; five message-tool sends where only the first can // claim the fifth passive slot; then a static final. Every later text send // falls back to a proactive body without msg_id/msg_seq. - qqbotScenario("budget-exhaustion", [ - { kind: "reply-start" }, - { kind: "advance", ms: 5000 }, - { kind: "advance", ms: 5000 }, - { kind: "advance", ms: 5000 }, - { kind: "advance", ms: 5000 }, - { kind: "tool-progress", name: "message", phase: "result" }, - { kind: "tool-progress", name: "message", phase: "result" }, - { kind: "tool-progress", name: "message", phase: "result" }, - { kind: "tool-progress", name: "message", phase: "result" }, - { kind: "tool-progress", name: "message", phase: "result" }, - { kind: "final", text: "Budget check complete." }, - { kind: "idle" }, - ]), - qqbotScenario("final-only", deliveryTraceScenarios["final-only"].steps), - qqbotScenario("cancel-mid-stream", deliveryTraceScenarios["cancel-mid-stream"].steps), + { + name: "budget-exhaustion", + steps: [ + { kind: "reply-start" }, + { kind: "advance", ms: 5000 }, + { kind: "advance", ms: 5000 }, + { kind: "advance", ms: 5000 }, + { kind: "advance", ms: 5000 }, + { kind: "tool-progress", name: "message", phase: "result" }, + { kind: "tool-progress", name: "message", phase: "result" }, + { kind: "tool-progress", name: "message", phase: "result" }, + { kind: "tool-progress", name: "message", phase: "result" }, + { kind: "tool-progress", name: "message", phase: "result" }, + { kind: "final", text: "Budget check complete." }, + { kind: "idle" }, + ], + }, + deliveryTraceScenarios["final-only"], + deliveryTraceScenarios["cancel-mid-stream"], // Media arriving mid-stream interrupts the session: DONE chunk for the text // before the tag, synchronous upload + media message (both budget spends), // then a fresh stream session with a new stream_msg_id and msg_seq resumes // the remaining text. - qqbotScenario("media-interrupt", [ - { kind: "reply-start" }, - { kind: "partial", text: "Here is the chart:" }, - { kind: "advance", ms: 300 }, - { kind: "partial", text: MEDIA_INTERRUPT_FULL_TEXT }, - { kind: "advance", ms: 300 }, - { kind: "final", text: MEDIA_INTERRUPT_FULL_TEXT }, - { kind: "idle" }, - ]), + { + name: "media-interrupt", + steps: [ + { kind: "reply-start" }, + { kind: "partial", text: "Here is the chart:" }, + { kind: "advance", ms: 300 }, + { kind: "partial", text: MEDIA_INTERRUPT_FULL_TEXT }, + { kind: "advance", ms: 300 }, + { kind: "final", text: MEDIA_INTERRUPT_FULL_TEXT }, + { kind: "idle" }, + ], + }, ]; const EXPECTED_REPLY_BUDGET_REMAINING: Readonly> = { diff --git a/extensions/slack/src/delivery-trace.test.ts b/extensions/slack/src/delivery-trace.test.ts index 2dc4ace6c32e..4ca1d7426aff 100644 --- a/extensions/slack/src/delivery-trace.test.ts +++ b/extensions/slack/src/delivery-trace.test.ts @@ -15,7 +15,6 @@ import { expectDeliveryTraceMatchesGolden, runDeliveryTraceScenario, type DeliveryTraceInStep, - type DeliveryTraceScenario, type DeliveryTraceStep, type TraceEvent, type TraceNormalizer, @@ -215,8 +214,8 @@ const BLOCKS_FINAL_PRESENTATION = { ], }; -// Slack-specific scenario scripts. The runner only consumes `steps`, so local -// names outside the shared scenario-library union are safe behind one cast. +// Slack-specific scenario scripts; the runner only consumes `steps` and the +// name (outside the shared scenario library) keys the golden filename. const slackTraceScenarios: Record = { "streaming-happy-native": [ { kind: "reply-start" }, @@ -271,10 +270,6 @@ const slackTraceScenarios: Record(); @@ -624,7 +619,7 @@ describe("slack delivery trace goldens", () => { for (const scenarioName of Object.keys(slackTraceScenarios) as SlackTraceScenarioName[]) { it(`records ${scenarioName}`, async () => { const events = await runDeliveryTraceScenario({ - scenario: asTraceScenario(scenarioName), + scenario: { name: scenarioName, steps: slackTraceScenarios[scenarioName] }, setup: (recorder) => setupSlackTrace(recorder, scenarioName), normalize: createSlackTsNormalizer(), }); diff --git a/extensions/telegram/src/delivery-trace.test.ts b/extensions/telegram/src/delivery-trace.test.ts index 39f6a8a5395e..5d8a53d76e94 100644 --- a/extensions/telegram/src/delivery-trace.test.ts +++ b/extensions/telegram/src/delivery-trace.test.ts @@ -292,6 +292,9 @@ async function setupTelegramTrace(recorder: WireRecorder) { // An aborted run stops emitting payloads; teardown happens on idle. break; case "wire-fault": + if (step.fault !== "rate-limit") { + throw new Error("telegram trace scenarios script only rate-limit wire faults"); + } state.wireFaults.push({ retryAfterMs: step.retryAfterMs }); armedRetryAfterMs = step.retryAfterMs; break; diff --git a/scripts/test-projects.test-support.d.mts b/scripts/test-projects.test-support.d.mts index 1f1d0a138b96..17adc6285c9d 100644 --- a/scripts/test-projects.test-support.d.mts +++ b/scripts/test-projects.test-support.d.mts @@ -40,6 +40,7 @@ export type ChangedTestTargetPlan = { export const DEFAULT_TEST_PROJECTS_VITEST_NO_OUTPUT_TIMEOUT_MS: string; export const DEFAULT_TEST_PROJECTS_VITEST_NO_OUTPUT_HEARTBEAT_MS: string; +export const CHANNEL_CONTRACT_CONFIG_PATTERNS: ReadonlyMap; export function orderFullSuiteSpecsForParallelRun( specs: T[], diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 3107cfb10426..66aae8dc736e 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -2337,7 +2337,11 @@ const RUNNABLE_VITEST_CONFIG_TARGETS = new Set([ ...Object.values(VITEST_CONFIG_BY_KIND), ...fullSuiteVitestShards.flatMap((shard) => [shard.config, ...shard.projects]), ]); -const CHANNEL_CONTRACT_CONFIG_PATTERNS = new Map([ +// Duplicates the per-config pattern arrays in test/vitest/vitest.contracts-shared.ts +// because this file must stay loader-free plain JS. Drift silently drops +// contract files from lane routing, so test/scripts/test-projects.test.ts +// asserts both enumerations stay identical. +export const CHANNEL_CONTRACT_CONFIG_PATTERNS = new Map([ [ CONTRACTS_CHANNEL_SURFACE_VITEST_CONFIG, [ diff --git a/src/channels/plugins/contracts/trace/delivery-trace.ts b/src/channels/plugins/contracts/trace/delivery-trace.ts index eab63767b875..88a7217674c9 100644 --- a/src/channels/plugins/contracts/trace/delivery-trace.ts +++ b/src/channels/plugins/contracts/trace/delivery-trace.ts @@ -54,7 +54,15 @@ export type TraceEvent = { /** Canonicalizes volatile fields (ids, timestamps) before compare/write. */ export type TraceNormalizer = (event: TraceEvent) => TraceEvent; -/** Agent-side lifecycle steps a channel maps onto its dispatcher wiring. */ +/** + * Agent-side lifecycle steps a channel maps onto its dispatcher wiring. + * + * `wire-fault` declares mocked wire-client misbehavior in the script instead + * of hand-armed mock state in setup: `rate-limit` scripts a retryable + * 429-shaped rejection honored after `retryAfterMs`; `write-error` makes the + * next and every later wire write throw an error whose `name` is `errorName` + * (e.g. the msteams SDK's StreamCancelledError on user Stop). + */ export type DeliveryTraceInStep = | { kind: "reply-start" } | { kind: "partial"; text: string } @@ -63,15 +71,23 @@ export type DeliveryTraceInStep = | { kind: "final"; text?: string; mediaUrls?: string[]; isError?: boolean } | { kind: "cancel" } | { kind: "idle" } - | { kind: "wire-fault"; fault: "rate-limit"; retryAfterMs: number }; + | { kind: "wire-fault"; fault: "rate-limit"; retryAfterMs: number } + | { kind: "wire-fault"; fault: "write-error"; errorName: string }; export type DeliveryTraceStep = DeliveryTraceInStep | { kind: "advance"; ms: number }; export type DeliveryTraceScenario = { - name: DeliveryTraceScenarioName; + /** + * Golden filename key. Shared-library scenarios use the closed + * DeliveryTraceScenarioName union; channels may record channel-specific + * scenarios under their own names. `string & {}` keeps completion for the + * shared names without closing the set — the runner never branches on it. + */ + name: DeliveryTraceScenarioName | (string & {}); steps: readonly DeliveryTraceStep[]; }; +/** Names in the shared scenario library (`deliveryTraceScenarios`). */ export type DeliveryTraceScenarioName = | "streaming-happy" | "final-only" diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index a3f6fc715ec3..7f07cc087c5d 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -6,6 +6,7 @@ import path from "node:path"; import fg from "fast-glob"; import { beforeAll, describe, expect, it, vi } from "vitest"; import { + CHANNEL_CONTRACT_CONFIG_PATTERNS, DEFAULT_TEST_PROJECTS_VITEST_NO_OUTPUT_HEARTBEAT_MS, DEFAULT_TEST_PROJECTS_VITEST_NO_OUTPUT_TIMEOUT_MS, applyDefaultMultiSpecVitestCachePaths, @@ -29,6 +30,12 @@ import { } from "../../scripts/test-projects.test-support.mjs"; import { captureReaddirSyncCallsDuring } from "../../src/test-utils/fs-scan-assertions.js"; import { toRepoPath } from "../../src/test-utils/repo-files.js"; +import { + channelConfigContractPatterns, + channelRegistryContractPatterns, + channelSessionContractPatterns, + channelSurfaceContractPatterns, +} from "../vitest/vitest.contracts-shared.ts"; import { fullSuiteVitestShards } from "../vitest/vitest.test-shards.mjs"; const normalizeRepoPath = toRepoPath; @@ -4890,3 +4897,18 @@ describe("scripts/test-projects Vitest cache isolation", () => { expect(applyDefaultMultiSpecVitestCachePaths(watch, { cwd: "/repo", env: {} })).toBe(watch); }); }); + +describe("scripts/test-projects channel contract lane patterns", () => { + // test-projects.test-support.mjs must stay loader-free plain JS, so it + // duplicates the per-config channel-contract patterns instead of importing + // vitest.contracts-shared.ts. Drift silently drops contract files from lane + // routing (it happened once), so pin both enumerations to each other. + it("stays in sync with the vitest.contracts-shared lane enumerations", () => { + expect(Object.fromEntries(CHANNEL_CONTRACT_CONFIG_PATTERNS)).toEqual({ + "test/vitest/vitest.contracts-channel-surface.config.ts": channelSurfaceContractPatterns, + "test/vitest/vitest.contracts-channel-config.config.ts": channelConfigContractPatterns, + "test/vitest/vitest.contracts-channel-registry.config.ts": channelRegistryContractPatterns, + "test/vitest/vitest.contracts-channel-session.config.ts": channelSessionContractPatterns, + }); + }); +});