Skip to content

Commit 1efc67c

Browse files
committed
refactor(kernel): own the response-warnings field contract beside the message reader
Four call sites read the composable warnings field with their own Array.isArray filters (the daemon append, the daemon attempt outcome, the CLI success line, the CLI/MCP error text), and the PR had grown a fifth. The field has one contract now: kernel/success-text readResponseWarnings, next to readCommandMessage, with the daemon append, the attempt outcome, messageWithWarningsText, and both error renderers as its named consumers. The CLI-side reader the PR introduced is deleted, not wrapped.
1 parent 4932e0f commit 1efc67c

9 files changed

Lines changed: 55 additions & 29 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import assert from 'node:assert/strict';
2+
import { describe, test } from 'vitest';
3+
import { readCommandMessage, readResponseWarnings } from './success-text.ts';
4+
5+
describe('readCommandMessage', () => {
6+
test('an empty message is absent, not an empty success line', () => {
7+
assert.equal(readCommandMessage({ message: '' }), null);
8+
assert.equal(readCommandMessage({ message: 42 }), null);
9+
assert.equal(readCommandMessage(undefined), null);
10+
assert.equal(readCommandMessage({ message: 'Replayed 7 steps' }), 'Replayed 7 steps');
11+
});
12+
});
13+
14+
describe('readResponseWarnings', () => {
15+
test('the shared warnings parser ignores anything that is not a note', () => {
16+
assert.deepEqual(readResponseWarnings({ warnings: ['a note', 42, { nested: true }, null] }), [
17+
'a note',
18+
]);
19+
assert.deepEqual(readResponseWarnings({}), []);
20+
assert.deepEqual(readResponseWarnings(undefined), []);
21+
});
22+
});

‎packages/kernel/src/success-text.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,16 @@ export function withSuccessText<T extends Record<string, unknown>>(
1212
export function readCommandMessage(data: Record<string, unknown> | undefined): string | null {
1313
return typeof data?.message === 'string' && data.message.length > 0 ? data.message : null;
1414
}
15+
16+
/**
17+
* The composable response-warnings channel (skipped `optional` steps, capture
18+
* degradations): every reader of a response or error record — daemon outcome,
19+
* CLI success line, CLI/MCP error text — goes through here, so one field
20+
* contract has one parser. Non-string entries are other producers' bugs.
21+
*/
22+
export function readResponseWarnings(data: Record<string, unknown> | undefined): string[] {
23+
const warnings = data?.warnings;
24+
return Array.isArray(warnings)
25+
? warnings.filter((warning): warning is string => typeof warning === 'string')
26+
: [];
27+
}

‎src/commands/output-common.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { readCommandMessage } from '@agent-device/kernel/success-text';
1+
import { readCommandMessage, readResponseWarnings } from '@agent-device/kernel/success-text';
22
import type { CommandProgressState } from './command-progress.ts';
33
import type { CliOutput } from './command-contract.ts';
44

@@ -37,9 +37,7 @@ export function messageCliOutput(result: Record<string, unknown>): CliOutput {
3737
*/
3838
export function messageWithWarningsText(result: Record<string, unknown>): string | null {
3939
const message = readCommandMessage(result);
40-
const warnings = Array.isArray(result.warnings)
41-
? result.warnings.filter((warning): warning is string => typeof warning === 'string')
42-
: [];
40+
const warnings = readResponseWarnings(result);
4341
if (warnings.length === 0) return message;
4442
return [message, ...warnings.map((warning) => `Warning: ${collapseWarningText(warning)}`)]
4543
.filter(Boolean)

‎src/commands/output/error.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
type ErrorCandidateView,
66
type NormalizedError,
77
} from '@agent-device/kernel/errors';
8+
import { readResponseWarnings } from '@agent-device/kernel/success-text';
89
import { formatReplayDivergenceReport } from '@agent-device/ad-replay/divergence';
910
import { collapseWarningText } from '../output-common.ts';
1011

@@ -54,13 +55,6 @@ export function printHumanError(
5455
}
5556
}
5657

57-
export function readResponseWarnings(details: Record<string, unknown> | undefined): string[] {
58-
const warnings = details?.warnings;
59-
return Array.isArray(warnings)
60-
? warnings.filter((warning): warning is string => typeof warning === 'string')
61-
: [];
62-
}
63-
6458
export function formatErrorCandidateViews(views: ErrorCandidateView[]): string[] {
6559
return views.flatMap((view) => {
6660
if (view.kind === 'element-match') {

‎src/daemon/replay/internal/session-test-outcome.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { readSnapshotDiagnosticsSummary } from '@agent-device/contracts/capture';
2+
import { readResponseWarnings } from '@agent-device/kernel/success-text';
23
import type { DaemonResponse } from '../../daemon-request.ts';
34
import { isReplayInfrastructureFailure } from './session-test-infrastructure.ts';
45
import type { ReplayTestAttemptFailed, ReplayTestAttemptOutcome } from '@agent-device/replay-test';
@@ -18,7 +19,7 @@ export function toReplayTestAttemptOutcome(response: DaemonResponse): ReplayTest
1819
status: 'failed',
1920
error: response.error,
2021
artifactPaths: readArtifactPaths(response.error.details?.artifactPaths),
21-
warnings: readStringArray(response.error.details?.warnings),
22+
warnings: readResponseWarnings(response.error.details),
2223
infrastructure: isReplayInfrastructureFailure(response),
2324
...snapshotDiagnostics(response.error.details?.snapshotDiagnostics),
2425
};
@@ -28,7 +29,7 @@ export function toReplayTestAttemptOutcome(response: DaemonResponse): ReplayTest
2829
status: 'passed',
2930
replayed: typeof data?.replayed === 'number' ? data.replayed : 0,
3031
healed: typeof data?.healed === 'number' ? data.healed : 0,
31-
warnings: readStringArray(data?.warnings),
32+
warnings: readResponseWarnings(data),
3233
artifactPaths: readArtifactPaths(data?.artifactPaths),
3334
...snapshotDiagnostics(data?.snapshotDiagnostics),
3435
};

‎src/daemon/session-lifecycle/internal/__tests__/session-open-warnings.test.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import assert from 'node:assert/strict';
22
import { test } from 'vitest';
3-
import { appendResponseWarning, readResponseWarnings } from '../session-open-warnings.ts';
3+
import { appendResponseWarning } from '../session-open-warnings.ts';
44

55
test('a producer adds its note without dropping the notes already on the response', () => {
66
const responseData: Record<string, unknown> = { warnings: ['the session is already open'] };
@@ -21,10 +21,12 @@ test('a response that carries no warnings yet starts from an empty list', () =>
2121
assert.deepEqual(responseData.warnings, ['the device was taken over']);
2222
});
2323

24-
test('reading warnings ignores anything that is not a note', () => {
25-
assert.deepEqual(readResponseWarnings({ warnings: ['a note', 42, { nested: true }, null] }), [
26-
'a note',
27-
]);
28-
assert.deepEqual(readResponseWarnings({}), []);
29-
assert.deepEqual(readResponseWarnings(undefined), []);
24+
test('accumulating onto non-note entries keeps only the notes and the new one', () => {
25+
const responseData: Record<string, unknown> = {
26+
warnings: ['a note', 42, { nested: true }, null],
27+
};
28+
29+
appendResponseWarning(responseData, 'the device was taken over');
30+
31+
assert.deepEqual(responseData.warnings, ['a note', 'the device was taken over']);
3032
});

‎src/daemon/session-lifecycle/internal/session-open-foreground.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type {
77
InspectDeviceRuntimeFacts,
88
} from '../../request-runtime-binding.ts';
99
import { errorResponse } from '../../response.ts';
10-
import { readResponseWarnings } from './session-open-warnings.ts';
10+
import { readResponseWarnings } from '@agent-device/kernel/success-text';
1111

1212
export type ForegroundOpenResolution =
1313
| { type: 'not-requested' }
Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { readResponseWarnings } from '@agent-device/kernel/success-text';
2+
13
/**
24
* Response-level warnings accumulate: every `open` producer adds its own note and keeps the ones
35
* already there, so a producer never has to know which other note ran first.
@@ -8,10 +10,3 @@ export function appendResponseWarning(
810
): void {
911
responseData.warnings = [...readResponseWarnings(responseData), warning];
1012
}
11-
12-
export function readResponseWarnings(responseData: Record<string, unknown> | undefined): string[] {
13-
const warnings = responseData?.warnings;
14-
return Array.isArray(warnings)
15-
? warnings.filter((warning): warning is string => typeof warning === 'string')
16-
: [];
17-
}

‎src/mcp/tool-error.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import {
44
type NormalizedError,
55
} from '@agent-device/kernel/errors';
66
import { formatReplayDivergenceReport } from '@agent-device/ad-replay/divergence';
7-
import { formatErrorCandidateViews, readResponseWarnings } from '../commands/output/error.ts';
7+
import { readResponseWarnings } from '@agent-device/kernel/success-text';
8+
import { formatErrorCandidateViews } from '../commands/output/error.ts';
89
import { collapseWarningText } from '../commands/output-common.ts';
910

1011
export function normalizeToolError(error: unknown): NormalizedError {

0 commit comments

Comments
 (0)