Skip to content

Commit d9727d4

Browse files
committed
fix(coding-agent): close three entry-line memo stamp gaps
- Gate the MCP spill-strip mirror on top-level toolResult details (custom/hook messages and nested details keep structuredContent in the line) - Brand-check boxed primitives with intrinsic valueOf calls, never reading Symbol.toStringTag user code the serializer ignores - Detect the BigInt.prototype.toJSON hook so bigint graphs with a stateful serializer stay uncached
1 parent f81c5ea commit d9727d4

2 files changed

Lines changed: 267 additions & 65 deletions

File tree

‎packages/coding-agent/src/session/session-manager.ts‎

Lines changed: 118 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,25 @@ function hasToJSONHook(value: object): boolean {
887887
}
888888
return false;
889889
}
890+
/**
891+
* Whether `JSON.stringify` routes bigint values through a `toJSON` hook. A
892+
* bigint carries no properties of its own — the primitive has no wrapper
893+
* object to hang a hook on — so the only hook serialization can consult is
894+
* one installed on `BigInt.prototype` itself (an extension hook, for example).
895+
* Answered through the property descriptor, never by reading the key: a
896+
* stateful `toJSON` getter on the prototype would otherwise run here and again
897+
* in `#lineFor`, shifting what serialization observes. Any accessor there —
898+
* or any callable data value, which the serializer invokes with the bigint as
899+
* its receiver — leaves a bigint-bearing graph uncached (see
900+
* {@link hasSerializerHook}).
901+
*/
902+
function hasBigIntToJSONHook(): boolean {
903+
const descriptor = Object.getOwnPropertyDescriptor(BigInt.prototype, "toJSON");
904+
if (descriptor === undefined) return false;
905+
if (typeof descriptor.get === "function" || typeof descriptor.set === "function") return true;
906+
return typeof descriptor.value === "function";
907+
}
908+
890909
/**
891910
* Whether any indexed slot of `value` resolves through an accessor — answered
892911
* through property descriptors along the prototype chain, never by reading a
@@ -934,20 +953,26 @@ function projectorGenerationResult(value: object): string | undefined {
934953
return undefined;
935954
}
936955
/**
937-
* Whether `value` is a toolResult `details` node whose `structuredContent` the
938-
* persistence projection removes before serialization — the mirror of
939-
* `stripSpilledMcpStructuredContent`'s predicate in
940-
* session-persistence.ts. The memo's three walks (hook probe, stamp, matcher)
941-
* must all apply it: the projected line never contains the field, so a hook
942-
* or cycle beneath it can neither reach the persisted bytes nor be allowed
943-
* to overflow a walk of the live graph. Reads through property descriptors
944-
* only where the answer must not invoke user code; the `meta.truncation`
945-
* spine is reached by key from a descriptor-read root because the hook probe
946-
* — which runs before every stamp — already leaves any accessor-backed
947-
* graph uncached.
956+
* Whether `value` is the top-level `details` of a `toolResult` message whose
957+
* `structuredContent` the persistence projection removes before serialization
958+
* — the mirror of `stripSpilledMcpStructuredContent` in
959+
* session-persistence.ts, which strips only that one node. A custom or hook
960+
* message — or any nested property merely named `details` — keeps its
961+
* `structuredContent` in the projected line, so the memo's three walks (hook
962+
* probe, stamp, matcher) must skip the field only here: the projected line
963+
* never contains it, so a hook or cycle beneath it can neither reach the
964+
* persisted bytes nor be allowed to overflow a walk of the live graph. The
965+
* `role` gate is answered through the own data descriptor of the message the
966+
* walk started from — never by reading a key — because the hook probe, which
967+
* runs before every stamp, already leaves any accessor-backed graph uncached.
968+
* Reads through property descriptors only where the answer must not invoke
969+
* user code; the `meta.truncation` spine is reached by key from a
970+
* descriptor-read root for the same reason.
948971
*/
949-
function isMcpSpillStrippedDetails(value: object, key?: string): boolean {
950-
if (key !== "details" || !Object.hasOwn(value, "structuredContent")) return false;
972+
function isMcpSpillStrippedDetails(value: object, key?: string, isTopLevelToolResultDetails?: boolean): boolean {
973+
if (key !== "details" || isTopLevelToolResultDetails !== true || !Object.hasOwn(value, "structuredContent")) {
974+
return false;
975+
}
951976
const serverName = Object.getOwnPropertyDescriptor(value, "serverName")?.value;
952977
const mcpToolName = Object.getOwnPropertyDescriptor(value, "mcpToolName")?.value;
953978
if (typeof serverName !== "string" || typeof mcpToolName !== "string") return false;
@@ -959,6 +984,19 @@ function isMcpSpillStrippedDetails(value: object, key?: string): boolean {
959984
return typeof artifactId === "string" && artifactId.length > 0;
960985
}
961986

987+
/**
988+
* Whether `childKey` of `value` is the top-level `details` of a `toolResult`
989+
* message — the only node `stripSpilledMcpStructuredContent` strips. True
990+
* only when the walk is still at the message root (`key` is `undefined`) and
991+
* the child's own `role` data descriptor reads `"toolResult"`: answered
992+
* through the descriptor, never by reading the key, so a stateful `role`
993+
* getter is left for the object loop to leave uncached rather than run here.
994+
*/
995+
function isTopLevelMessageChild(value: object, key: string | undefined, childKey: string): boolean {
996+
if (key !== undefined || childKey !== "details") return false;
997+
return Object.getOwnPropertyDescriptor(value, "role")?.value === "toolResult";
998+
}
999+
9621000
/**
9631001
* Whether `value`'s JSON graph passes through a `toJSON` serializer hook — or
9641002
* through any accessor whose read the probe must not perform.
@@ -983,7 +1021,13 @@ function isMcpSpillStrippedDetails(value: object, key?: string): boolean {
9831021
* walk could never escape; the projected line never contains it, so no hook
9841022
* beneath one can reach the persisted bytes.
9851023
*/
986-
function hasSerializerHook(value: unknown, key?: string): boolean {
1024+
function hasSerializerHook(value: unknown, key?: string, isTopLevelToolResultDetails?: boolean): boolean {
1025+
// A bigint is the one primitive `JSON.stringify` routes through a hook:
1026+
// when an extension installs `BigInt.prototype.toJSON`, serialization
1027+
// invokes it — so a stateful hook's first output cannot be stamped as a
1028+
// leaf. Leave the graph uncached exactly when such a hook exists (see
1029+
// `hasBigIntToJSONHook`); other primitives never consult one.
1030+
if (typeof value === "bigint") return hasBigIntToJSONHook();
9871031
if (value === null || typeof value !== "object") return false;
9881032
// `JSON.stringify` consults the hook before the value shape, so the probe
9891033
// does too — through the property descriptor, never by reading the key: a
@@ -1004,13 +1048,14 @@ function hasSerializerHook(value: unknown, key?: string): boolean {
10041048
return false;
10051049
}
10061050
// Mirror the persistence projection's MCP spill strip
1007-
// (`stripSpilledMcpStructuredContent`): when a toolResult's details carries
1008-
// the spill markers, `structuredContent` is removed before serialization,
1009-
// so a hook beneath it cannot reach the persisted bytes — and the live
1010-
// graph may be cyclic where the projection is not. A stateful `meta`
1011-
// getter would shift serialization if read, so the object loop below
1012-
// answers it through the descriptor and leaves the graph uncached.
1013-
const skipStructuredContent = isMcpSpillStrippedDetails(value, key);
1051+
// (`stripSpilledMcpStructuredContent`): only the top-level `details` of a
1052+
// `toolResult` message loses `structuredContent` before serialization, so
1053+
// only there can a hook beneath it not reach the persisted bytes — and
1054+
// only there may the live graph be cyclic where the projection is not. A
1055+
// stateful `meta` getter would shift serialization if read, so the object
1056+
// loop below answers it through the descriptor and leaves the graph
1057+
// uncached.
1058+
const skipStructuredContent = isMcpSpillStrippedDetails(value, key, isTopLevelToolResultDetails);
10141059
// Mirror the projection's stripping boundary: a hook beneath a stripped key
10151060
// cannot affect the persisted bytes — and the live value may be cyclic
10161061
// where the projection is not.
@@ -1042,7 +1087,7 @@ function hasSerializerHook(value: unknown, key?: string): boolean {
10421087
const descriptor = Object.getOwnPropertyDescriptor(value, childKey);
10431088
if (descriptor === undefined) continue;
10441089
if (typeof descriptor.get === "function" || typeof descriptor.set === "function") return true;
1045-
if (hasSerializerHook(descriptor.value, childKey)) return true;
1090+
if (hasSerializerHook(descriptor.value, childKey, isTopLevelMessageChild(value, key, childKey))) return true;
10461091
}
10471092
return false;
10481093
}
@@ -1074,41 +1119,39 @@ const STAMP_TRANSIENT = Symbol("stamp.transient");
10741119
* instead — hooked graphs are left uncached (see {@link hasSerializerHook}) —
10751120
* so only hookless wrappers reach here.
10761121
*
1077-
* The intrinsic call is the brand check, answered without invoking an
1078-
* override: only a genuine wrapper unboxes, and anything else falls through
1079-
* to the object walk. There is deliberately no `instanceof` gate: it answers
1080-
* the wrong question twice — a proxy around a wrapper or an object built on
1081-
* `Number.prototype` passes the chain check while carrying no `[[NumberData]]`
1082-
* slot, so `valueOf()` throws on it while the serializer accepts it as `{}`;
1083-
* and a wrapper from another realm (a `node:vm` context, an iframe) fails the
1084-
* local-realm chain check although `valueOf` succeeds and `JSON.stringify`
1085-
* emits the primitive. The `Symbol.toStringTag` dispatch below narrows the
1086-
* candidates first so plain objects never pay for a throwing intrinsic call.
1122+
* Each intrinsic call is the brand check, answered without invoking an
1123+
* override and without reading `Symbol.toStringTag`: only a genuine wrapper
1124+
* unboxes, and anything else falls through to the object walk. There is
1125+
* deliberately no `instanceof` gate: it answers the wrong question twice — a
1126+
* proxy around a wrapper or an object built on `Number.prototype` passes the
1127+
* chain check while carrying no `[[NumberData]]` slot, so `valueOf()` throws
1128+
* on it while the serializer accepts it as `{}`; and a wrapper from another
1129+
* realm (a `node:vm` context, an iframe) fails the local-realm chain check
1130+
* although `valueOf` succeeds and `JSON.stringify` emits the primitive. There
1131+
* is deliberately no `Symbol.toStringTag` dispatch either: `JSON.stringify`
1132+
* never reads the tag, but `Object.prototype.toString` runs a getter installed
1133+
* there — even a non-enumerable one — so dispatching on it would invoke user
1134+
* code the serializer itself ignores, letting a throwing getter fail a probe
1135+
* over a value that serializes normally, or a stateful one mutate fields
1136+
* before `#lineFor` observes them. Plain objects pay for up to three throwing
1137+
* intrinsic calls instead — a local cost that runs no user code.
10871138
*/
10881139
function unboxedPrimitive(value: object): number | string | boolean | undefined {
1089-
const tag = Object.prototype.toString.call(value);
1090-
if (tag === "[object Number]") {
1091-
try {
1092-
return Number.prototype.valueOf.call(value);
1093-
} catch {
1094-
return undefined;
1095-
}
1140+
try {
1141+
return Number.prototype.valueOf.call(value);
1142+
} catch {
1143+
// Not a genuine Number wrapper: fall through to the next brand.
10961144
}
1097-
if (tag === "[object String]") {
1098-
try {
1099-
return String.prototype.valueOf.call(value);
1100-
} catch {
1101-
return undefined;
1102-
}
1145+
try {
1146+
return String.prototype.valueOf.call(value);
1147+
} catch {
1148+
// Not a genuine String wrapper: fall through to the next brand.
11031149
}
1104-
if (tag === "[object Boolean]") {
1105-
try {
1106-
return Boolean.prototype.valueOf.call(value);
1107-
} catch {
1108-
return undefined;
1109-
}
1150+
try {
1151+
return Boolean.prototype.valueOf.call(value);
1152+
} catch {
1153+
return undefined;
11101154
}
1111-
return undefined;
11121155
}
11131156

11141157
/**
@@ -1152,7 +1195,12 @@ function rawJsonPayload(value: object): string | undefined {
11521195
* indexed slots with a snapshotted length, the way `JSON.stringify` reads
11531196
* them, never through the iterator protocol.
11541197
*/
1155-
function messageValueStamp(value: unknown, out: unknown[] = [], key?: string): unknown[] {
1198+
function messageValueStamp(
1199+
value: unknown,
1200+
out: unknown[] = [],
1201+
key?: string,
1202+
isTopLevelToolResultDetails?: boolean,
1203+
): unknown[] {
11561204
if (value === null || typeof value !== "object") {
11571205
out.push(value);
11581206
return out;
@@ -1185,11 +1233,14 @@ function messageValueStamp(value: unknown, out: unknown[] = [], key?: string): u
11851233
// its key-stripping loop, so transient keys inside one reach the line and
11861234
// must be stamped. Only non-atomic containers strip them.
11871235
const atomic = isAtomicPersistenceNode(value, key);
1188-
// The MCP spill strip removes `structuredContent` before serialization
1189-
// (see `isMcpSpillStrippedDetails`), so the field is skipped outright:
1190-
// unlike the transient keys below it forces no rebuild pass, and the
1191-
// live graph may be cyclic where the projection is not.
1192-
const skipStructuredContent = isMcpSpillStrippedDetails(value, key);
1236+
// Only the top-level `details` of a `toolResult` message loses
1237+
// `structuredContent` before serialization (see
1238+
// `isMcpSpillStrippedDetails`), so only there is the field skipped
1239+
// outright: unlike the transient keys below it forces no rebuild
1240+
// pass, and the live graph may be cyclic where the projection is
1241+
// not. Any other marker-shaped `details` keeps its field in the
1242+
// projected line and is stamped whole.
1243+
const skipStructuredContent = isMcpSpillStrippedDetails(value, key, isTopLevelToolResultDetails);
11931244
out.push(STAMP_OBJECT);
11941245
for (const childKey of Object.keys(fields)) {
11951246
if (skipStructuredContent && childKey === "structuredContent") continue;
@@ -1205,7 +1256,7 @@ function messageValueStamp(value: unknown, out: unknown[] = [], key?: string): u
12051256
continue;
12061257
}
12071258
out.push(childKey);
1208-
messageValueStamp(fields[childKey], out, childKey);
1259+
messageValueStamp(fields[childKey], out, childKey, isTopLevelMessageChild(value, key, childKey));
12091260
}
12101261
}
12111262
out.push(STAMP_END);
@@ -1222,7 +1273,7 @@ function messageValueStamp(value: unknown, out: unknown[] = [], key?: string): u
12221273
*/
12231274
function stampMatches(stamp: readonly unknown[], value: unknown): boolean {
12241275
let index = 0;
1225-
const match = (live: unknown, key?: string): boolean => {
1276+
const match = (live: unknown, key?: string, isTopLevelToolResultDetails?: boolean): boolean => {
12261277
const expected = stamp[index++];
12271278
// The stamp never invokes a `toJSON` hook: a hooked graph is left
12281279
// uncached (see {@link hasSerializerHook}), so reaching one here means
@@ -1260,10 +1311,12 @@ function stampMatches(stamp: readonly unknown[], value: unknown): boolean {
12601311
// inside atomic nodes (persistence keeps them) and as
12611312
// presence-only markers elsewhere.
12621313
const atomic = isAtomicPersistenceNode(live, key);
1263-
// The stamp skipped the MCP-stripped field outright (see
1264-
// {@link messageValueStamp}), so the matcher skips it too —
1265-
// the value was never recorded and may be cyclic.
1266-
const skipStructuredContent = isMcpSpillStrippedDetails(live, key);
1314+
// The stamp skipped the top-level toolResult's MCP-stripped
1315+
// field outright (see {@link messageValueStamp}), so the
1316+
// matcher skips it too — the value was never recorded and may
1317+
// be cyclic. Any other marker-shaped `details` was stamped
1318+
// whole and is matched whole.
1319+
const skipStructuredContent = isMcpSpillStrippedDetails(live, key, isTopLevelToolResultDetails);
12671320
for (const childKey of Object.keys(fields)) {
12681321
if (skipStructuredContent && childKey === "structuredContent") continue;
12691322
// Stripped before serialization by `truncateForPersistence`,
@@ -1275,7 +1328,7 @@ function stampMatches(stamp: readonly unknown[], value: unknown): boolean {
12751328
continue;
12761329
}
12771330
if (stamp[index++] !== childKey) return false;
1278-
if (!match(fields[childKey], childKey)) return false;
1331+
if (!match(fields[childKey], childKey, isTopLevelMessageChild(live, key, childKey))) return false;
12791332
}
12801333
}
12811334
return stamp[index++] === STAMP_END;

0 commit comments

Comments
 (0)