fix(agent,ai): eliminate the managed snapshot failure producers (root cause for #4578) - #4587
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ad leak The #4580 circuit breaker surfaced local snapshot failures once, but the producers that trip the circuit were still live. Two root causes are fixed so the circuit does not fire on benign payloads at all: packages/agent: a payload class carrying assistant/message-event fields on prototype getters clones into an empty record (structuredClone copies only own enumerable properties), so the live role/type checks passed while the detached snapshot lost the identity and deterministically failed as shell.role / event.unknownType. The shell and event snapshots now repair such roots (and readable proxies) through the existing guarded-read path, and the run-loop message_update replay normalizes through the managed event snapshot instead of a naive spread that dropped prototype-carried fields. packages/ai: cursor native tool calls attached raw protobuf-es payloads (bigint fields, $typeName markers, byte arrays) as toolCall arguments, defeating JSON.stringify in snapshot staging, transcript persistence, and replay. Arguments are now converted to plain JSON-safe data at the provider boundary. Lore-id: 61d94fea Constraint: hostile shapes (throwing get traps, sentinel-degraded content, non-string event types) keep named fail-fast diagnostics with no retry authority Constraint: repair reads stay guarded (managedProperty) so a hostile trap can only degrade a field to undefined Rejected: widening the sanitizer to accept unserializable staged values | hides producer defects behind lossy placeholders Rejected: repairing hostile get-trap proxies | unreadable roots must not gain retry authority Confidence: high Scope-risk: medium Reversibility: clean Tested: payload-class end-to-end managed run; descriptor-trap proxy repair; cursor protobuf argument conversion; full agent suite (811 pass) Not-tested: live Cursor provider session
The initial producer-boundary conversion still walked oversized containers and could throw on deeply nested or unreadable payload objects. Stop traversal at explicit node/depth limits and contain hostile enumeration while preserving object-shaped tool arguments. Lore-id: 92d31e6a Constraint: native tool arguments must remain plain JSON-safe records Rejected: rely on protobuf payloads always being shallow and readable | leaves the advertised total conversion vulnerable to malformed provider data Confidence: high Scope-risk: low Reversibility: clean Tested: cursor native toolcall JSON safety suite
f380a78 to
be83d77
Compare
|
Signed exact-head reconstruction evidence
Overlap audit:
Fresh exact-head verification:
Canonical verdict remains Signed-by: Yeachan-Heo — |
|
Terminal exact-head CI classification Authoritative Dev CI run: https://github.com/Yeachan-Heo/gajae-code/actions/runs/31883508312 Binding:
Product result: GREEN. State gates, affected plan, baseline+modern native build, CLI smoke, agent loop 38/38, managed transaction 51/51, Cursor safety 6/6, AI package check, coding-agent/stats builds, affected evidence/aggregate, and virtual integration all completed successfully. Platform-specific unrelated jobs were correctly skipped. The sole failure is Earlier runs Signed-by: Yeachan-Heo — |
OWNER_CONFIRMATION_REQUIRED — exact-head review is the only remaining gateCurrent immutable review binding:
Bounded review surface: six files only — managed assistant shell/event root repair and regression, Cursor native toolCall JSON-safety boundary and regression, plus the two package changelog entries. Please verify that benign payload-class/readable-proxy roots are repaired, unreadable roots retain fail-fast/no-retry behavior, Cursor bigint/bytes/type markers/cycles stay JSON-safe and bounded, and #4580's circuit breaker is not duplicated or weakened. Exact-head approvals are currently zero. Existing review requests remain active for write-authorized non-authors @probepark, @HaD0Yun, and @IYENTeam; this comment is the single refreshed notification and will not be repeated absent a head change. An No self-approval or stale-head review will be used. — |
probepark
left a comment
There was a problem hiding this comment.
Approve at be83d77e4
fix(agent,ai): eliminate the managed snapshot failure producer, head commit "bound cursor payload
conversion". 6 files, +339.
Differential
# base 804314081, with this head's test files applied
(fail) managed snapshot benign degradation (PR #4538 salvage) > repairs payload-class messages and events whose fields live on the prototype
(fail) managed snapshot benign degradation (PR #4538 salvage) > repairs descriptor-trap proxy events whose guarded gets stay readable
SyntaxError: Export named 'buildNativeToolCallBlock' not found in module '.../providers/cursor.ts'
49 pass 3 fail
# head be83d77e4
57 pass 0 fail
It attacks the producer rather than the symptom
The previous rounds in this area treated managed snapshot failures as something to classify
(#4550's named sites), then to surface immediately rather than retry (#4580). This one removes the
cause: payload objects whose fields live on the prototype, and descriptor-trap proxies whose gets
are still readable, are now repaired at conversion time instead of rejected downstream.
That ordering is right. Naming and routing a failure is worth doing while the failure is
unavoidable; once the producer can be fixed, the classification machinery should have less to do,
not more.
bound cursor payload conversion in the same PR is the necessary guard - repairing prototype-backed
and proxied payloads means walking structures you did not construct, so it needs an explicit bound
rather than trusting the shape.
The two base failures are prototype-field and descriptor-trap repair specifically, so the tests
target the new capability rather than the surrounding plumbing.
merge-approved.
Reviewed by @probepark - method: fresh-worktree run at the exact head, separate clean base worktree with the head's test files to prove the differential.
Signed terminal merge evidence and reproducible receiptPR #4587 is terminally merged. Approval and merge authority
Squash merge and concurrent-dev audit
Post-merge verification
Validated gajae receipt / reproducible recordReproduction: fetch Signed-by: Yeachan-Heo — |
gajae.pr-review-verdict.v1 merge-approved sha256:c2647d5f8c5f5ab1dffa3d4f066a9cf534f977c25d299d66b59d16055e0176d3 reviewer:human reviewer-id:probepark evidence:#4587 (review)
fix(agent,ai): eliminate managed snapshot failure producers
#4580 made residual local snapshot failures surface once instead of retrying. This PR removes the benign producers that trip that circuit breaker without duplicating or relaxing its retry policy.
Root causes fixed
Payload-class and readable-proxy roots (
packages/agent)Provider message/event classes can carry
role,type, and payload fields on prototype getters. Live guards see those fields, butstructuredClonecopies only own enumerable properties, so the detached snapshot loses its identity. Managed assistant shell/event reconstruction now re-reads only known fields through the existing guardedmanagedPropertypath when the detached root loses its discriminant. The run-loopmessage_updatereplay normalizes throughmanagedAssistantEventSnapshotinstead of spreading prototype-backed events.Readable proxies whose descriptor trap is hostile are repaired through guarded gets. Unreadable get traps, throwing role getters, malformed non-string event types, and sanitizer-degraded content keep named fail-fast local snapshot diagnostics and never gain managed retry authority.
Cursor protobuf argument leak (
packages/ai)Cursor native tool calls previously attached protobuf-es payloads directly as toolCall
arguments. Bigints,$typeNamemarkers, byte arrays, cycles, and non-data leaves could defeat JSON serialization in managed staging, JSONL persistence, and replay. The provider boundary now converts arguments to plain JSON-safe records: safe-range bigint to number, larger bigint to decimal string, bytes to base64, dates to ISO, markers stripped, cycles/functions/unreadable objects to null, with explicit shared node and depth limits.Scope and overlap
804314081fe9d3f4d34014d1385f09d8d49a7116(dev, including fix(sdk): keep Windows OS probes and slow syncs off the session-index lock (#4544) #4555).be83d77e46a8c92fdef1b6bb16f66b7969402d55.sha256:c2647d5f8c5f5ab1dffa3d4f066a9cf534f977c25d299d66b59d16055e0176d3.packages/ai/CHANGELOG.md; their source/test work is disjoint and all changelog entries are preserved.Fresh exact-head verification
packages/agent/test, Cursor AI suites, coding-agent fallback integration).Exact-head review is requested from @probepark, @HaD0Yun, and @IYENTeam.
needs-humanremains binding until one authorized non-author reviewer approves this exact head and product CI is terminal green. Earlier reviews and runs are stale by construction.—
[repo owner's gaebal-gajae (clawdbot) 🦞]