feat(coding-agent): add sandbox-backed session foundations - #2025
Draft
sethkarten wants to merge 311 commits into
Draft
feat(coding-agent): add sandbox-backed session foundations#2025sethkarten wants to merge 311 commits into
sethkarten wants to merge 311 commits into
Conversation
…th, branding cleanup
…views, URL IP rejection
…t check, intrinsic detached detection, array rejection
…strap FD reader - readSandboxBootstrapFrame: uint32BE length + 1..64KiB payload + EOF - One total timeout (30s default) + close-confirm timeout (2s default) - Async fs.read/fs.close callbacks only, injectable FsFdAdapter - Short-read loops, caller-owned fresh payload, buffer erasure on terminal - Timeout/cancellation buffer lifecycle: retain pending-read buffers, erase on late callback - consumeSandboxBootstrapFrame wrapper with consumer erasure - 41 focused Vitest tests covering 1-byte splits, 1/64KiB payloads, empty/oversize/trailing/close-error/never-callback/double-callbacks
…strap FD reader - Strict options preflight: own enumerable data descriptors via Object.getOwnPropertyDescriptors, reject Proxy/getter/symbol/ non-enumerable/unknown keys, frozen config result - adapter.read/close sync throws caught and mapped to INTERNAL/CLOSE_FAILED - close confirm timer registered BEFORE adapter.close to prevent orphan timer on synchronous callback; cleared on every sync/async path - Per-read unique tokens with owned buffer ref + requested byte count; stale/double callbacks cannot touch current state - bytesRead validated: safe int 0..requested; negative/too-large/undefined values do not advance (error or retry) - Synchronous callback recursion capped at 128 depth; setImmediate deferral prevents stack overflow (tested with sync 1-byte 64KiB) - DataView.getUint32 for unsigned header parse (avoids signed 0xFFFFFFFF bug) - fd rejected for 0/1/2 (stdin/stdout/stderr); safe integer >= 3 required - Frozen ReadResult/ConsumeResult DTOs - consumeSandboxBootstrapFrame with consumer erasure and caught-throw 55 focused Vitest tests covering all regressions
- copyOptions rejects non-plain prototypes, symbol keys, non-enumerable, getter properties; adapter read/close validated via own data descriptors - Bounded integer timers: total 1..120000, close 1..10000; zero/huge rejected as INVALID_OPTIONS - bytesRead: undefined/non-integer/negative/>requested = phase error; zero in header/payload = premature EOF (READ_HEADER/READ_PAYLOAD); zero in trailing = successful EOF - Continuation scheduling: setImmediate deferred callbacks re-check cancelled/settled before calling onOk - Double close silently returns (no INTERNAL cast) - 65 focused Vitest regressions for all boundary conditions
… public-throw fix - copyOptions: snapshot getPrototypeOf, getOwnPropertyDescriptors, getOwnPropertySymbols in single try/catch; also snapshot adapter in guarded pass (so hostile Proxy traps cannot throw past Promise) - Adapter must be plain (Object.prototype or null), free of symbol keys, exactly two own enumerable data descriptors (read, close), each a function; no getters, no extras - Total deadline wins when a normal terminal close is pending: timeout replaces closeDispatch so that CLOSE_OK maps to TIMEOUT, not success - 71 focused Vitest regressions including timeline deadline win and hostile getPrototypeOf/ownKeys/adapter-descriptor Proxy traps
…ad erasure + fresh frozen adapter
- settle() now erases and nulls freshPayload for every non-ok result;
only ok:true success preserves it
- Adapter built as freshObject.freeze({read:aDescs.read.value, close:aDescs.close.value})
so a Proxy source cannot be re-read after validation
- ReadOptions fields marked readonly
- Regression: exact EOF + delayed close past total deadline yields TIMEOUT
and all allocated payload bytes zeroed
72 tests passing, npm run check clean
…dapter + proxy get-trap isolation
- adapter built as Object.freeze({read:aDescs.read.value, close:aDescs.close.value})
from already-validated descriptor function values; Proxy source not re-read
- Regression: Proxy get-trap counter test and null-prototype adapter acceptance
74 tests passing, npm run check clean
Import protocol constants from remote-agent-host-protocol; runtime- freeze PAAR_ERRORS; readonly DTOs; freeze outer result containers; descriptor-snapshot input copies (no `in`/direct Proxy reads); reject non-plain prototypes, class instances, inherited props, symbols, array extra properties; alias detection; DataView uint32BE; erase own buffers on all failure paths; drop custom trailing/-0 parsers; fixed-map hostile Uint8Array views to INVALID_INPUT.
PaarErrorCode: closed literal union; PAAR_ERRORS: frozen literal-type Alias: raw ref added to seen before snapshotOwnData Files: snapshotArrayIndices with strict canonical index verification Decoder: exact non-shared Uint8Array guard, byteOffset=0, reject Buffer/subclass/SharedArrayBuffer/detached/subview; same for throws bytes.byteLength <= totalArchiveSize; DataView.setUint32/writeUint32BE All hostile `in` removed; descriptor .value snapshots only Raw-byte canonical equality replaces custom trailing/-0 parsers 95 tests
isGenuineUint8Array: require b.byteLength === buffer.byteLength (reject short subview over large buffer); check Object.getPrototypeOf(buffer) === ArrayBuffer.prototype; detect detachment via ArrayBuffer.prototype.slice. call(buffer,0,0) — caught throw maps back to INVALID_INPUT, not SHORT_HEADER. UTF-8 sort comparison Buffers: erase (fill(0)) in finally after every Buffer.compare call (both encode and decode paths). Regressions: offset-zero short subview (exact error code), detached ArrayBuffer INVALID_INPUT assertion (not SHORT_HEADER).
…, query validation
…lete schema, atomic commit, never throw
Reads exactly one uint32BE length-prefixed frame from an injected Readable-like event source. Uses one fixed 4-byte header buffer and one exact payload allocation; no Buffer.concat, no arbitrary chunk accumulation, no O(n^2). Pre-copy bounds remaining bytes; any byte after payload is TRAILING. Rejects non-genuine Uint8Array chunks, subclasses, shared/detached buffers, and proxies with fixed error codes. Never mutates source-owned chunks. Total wall deadline 1..120000 ms kept referenced. Registers listeners/timer before resume; strict exact descriptor-copied options/source adapter returning fresh frozen object. Public frozen results never throw/raw reject. On every terminal path removes all listeners and clears timer. On input error/end-before-complete/timeout/trailing erases every owned header/payload buffer. On success returns fresh caller-owned payload only after EOF, with header erased and no alias to chunks. Late/ stale/double events after settle cannot mutate returned payload. Handles synchronous/reentrant data/end/error emission without recursion, double settlement, or listener races. No strings ever contain payload. Includes consume helper that awaits callback and erases payload in finally with fixed CALLBACK_FAILED. No any/dynamic/inline import/require/sync APIs/unref/raw error inspection. Exports readonly types. 92 focused Vitest regressions including bytewise split, header 0/FFFFFFFF/max/max+1, exact EOF, trailing same/later chunk, huge chunk pre-copy bound, premature end, source error, timeout, sync data/end/error during on(), unrelated listener preservation, copySource Proxy get trap avoid, no-timer-after-reentrant-settle, stale events, hostile options/source/chunks, result freeze/no alias, owned erasure via test allocation hook, source scan no concat/secret stringify.
… assertions - Set registeredData/End/Error flags BEFORE each src.on() call, so a synchronous callback that settles the promise can still be cleaned up by removeOwnListeners. On throw during on(), set flag false and run cleanup because registration outcome is uncertain. - copySource returns a fresh frozen adapter from descriptor values; never re-reads from the raw source, so Proxy get traps are not invoked by the internal adapter. - Remove _source from options (source is already a required arg). - Remove removeAllListeners() — only removeOwnListeners removes this reader's registered callbacks, preserving unrelated listeners. - Timer created only after all three on() registrations succeed without settlement. - Mark ParsedOptions.totalTimeoutMs readonly. - Fix Uint8Array adapter comment: constructing a view over a Buffer's backing ArrayBuffer is unsafe (Node retains pooled backing); the real adapter must allocate/copy fresh Uint8Array and erase after callback. - Tests assert listener tables are empty after synchronous data/end/error during on().
…during-on tests On src.on() throw, leave the pre-set registered*/true flag so removeOwnListeners (called via settleWithCode) will attempt removal of the possibly-installed listener. Previously the flag was set false before cleanup, causing cleanup to skip exactly the installed listener. Tests: three new regressions that install a callback into the source's listener table and THEN throw from each on(data/end/error). After settle, every installed owned listener is removed and unrelated ones preserved. 95/95 passing.
…ader integration - Add StdinSource.removeListener overloads matching on() event-specific callback types, fixing variance errors in source and tests - Guard removeOwnListeners with early return when src is null - Replace ArrayBuffer.transfer() (ES2024) with MessageChannel-based detachment compatible with ES2022 library target - Fix _getTrapCount typo (underscore prefix, const) in FD3 test - Cast cb to (...args: unknown[]) => void in test handler arrays where overloaded cb union types are stored by reference identity - Remove unused @ts-expect-error directives
- Header-only decode: accept exactly header+manifest bytes, reject payload as TRAILING_BYTES (streaming verifier owns payload hash validation) - Required PAWS_TA_SUBARRAY in isGenuineUint8Array; removed indexed-loop fallback - All live .byteLength reads replaced: reencodedLen captured once, manifestLen used for iteration, no bytes.byteLength/manifestSlice.byteLength live reads - Added explicit test: even declared payload bytes cause TRAILING_BYTES - Archive bound (headerSize+payloadSize <= 500MiB) enforced at metadata level
…n conflict - validateEntryOrder: early return if paths.length < 2 - Every early conflict/duplicate/unsorted return erases BOTH prevBytes and currBytes via a deferred result variable, avoiding leaked temp allocations - Added tests: single-entry ordering, conflict erasure instrumentation
…te compliance - encodeChangesetImpl: types.isProxy + prototype check before Object.getOwnPropertyDescriptors - decodeChangeset: same proxy/prototype safety before operation read - isGenuineUint8Array: reject backing ArrayBuffer with extra own properties/symbols - All tests: use discriminant narrowing for union-typed manifest/identity access - Tests: proxy trap instrumentation, buffer extra property coverage - Biome strict + tsgo --noEmit + 72 vitest tests all green
- hasInvalidPathChar: add C1 range check between C0 and DEL rejection - Add test for U+0085 (NEL) control character path rejection - 73 tests pass, all strict gates green
…inate subarray fallback - decodeSnapshot: re-encode final manifest and compare byte-for-byte against original manifest bytes; reject with NON_CANONICAL on mismatch (whitespace, key reorder, etc.) - decodeChangeset: same canonical JSON verification before returning - Subarray fallback removed: PAWS_TA_SUBARRAY required (admission checked in isGenuineUint8Array), no else/Reflect.get fallback - Dead freshBytes allocation removed from decodeSnapshot - 78 tests (5 new canonical JSON rejection tests), all strict gates green
…ay byte comparisons - Pass manifestStr (string) to decodeSnapshot/decodeChangeset instead of manifestSlice (Uint8Array alias) - Canonical check: build expectedJson string, compare expectedJson === manifestStr (string equality catches whitespace, key reorder, escape differences, number spelling) - Removed expectedBytes/expectedChgBytes allocations and .byteLength live reads - Only reencoded.byteLength remains as captured const reencodedLen - 78 tests pass, zero live .byteLength reads on temp typed arrays
- Use PAWS_TA_BYTE_LENGTH_GETTER via Reflect.apply instead of .byteLength - Grep shows zero .byteLength reads on reencoded/bytes/freshBytes/manifestSlice - All typed-array length reads use captured intrinsic getters - 78 tests pass, Biome strict + tsgo --noEmit both clean
…essions # Conflicts: # packages/coding-agent/src/modes/daemon/daemon-protocol.ts
…and uncertainty tests ordered-durable-relay.ts: replace Promise.all in closeAll with sequential reverse-acquisition-order for loop. The Set dedup preserves function-reference identity so aliased owners close once. A single close failure results in false and uncertainty dominates. target-inbox-registry.ts: close() already closes entries, factory, catalog in correct reverse sequence. New tests cover factory and catalog close failure returning CLOSE_UNCERTAIN on normal close. Tests added: - ordered-durable-relay: reverse-close-order, transport-failure-uncertainty - target-inbox-registry: factory-failure-uncertainty, catalog-failure-uncertainty
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tracking ticket: RES-1264
Status
Draft. Do not merge yet. This PR is the tracking branch for sandbox-backed Prime Agent sessions. Hosted session creation remains deliberately disabled until the remaining provider, workspace, transport-composition, recovery, and deletion boundaries are complete.
Implemented foundations
AgentSessioncommand effects and sandbox command applicationagent_messagedeliveryCurrent validation
tsgo --noEmitpass at the latest reviewed implementation milestonesRemaining before review
sandbox_sessionsonly after full composition succeedsmain, run the complete repository checks, security/artifact scans, and verify CISANDBOX_SESSIONS_PLAN.mdis the in-repository progress log and implementation record. Only reviewed integration commits are pushed to this branch; rejected experiments stay in isolated local worktrees.Note
Add sandbox-backed session foundations for coding-agent
prime-agent-runtime)AgentSessionto a brandedWeakSetidentity model; RLM child runtime now returns a frozen local-or-hosted runtime union handle instead of a raw session; registration is async and rejects unbranded sessions before callback or map mutationAgentSession.registerRlmChildSessionanddeleteRlmSubagentRuntimenow require brandedRlmSubagentRuntimehandles; callers passing rawAgentSessionobjects or synchronous registration callbacks will break. TheAgentSessionconstructor registers every instance in a module-privateWeakSet; out-of-tree code that constructs sessions indirectly is unaffected, but code that previously passed sessions directly to deletion APIs must migrate to the runtime-handle contract in agent-session-runtime.ts and rlm-runtime.tsMacroscope summarized d093e7e. (Automatic summaries will resume when PR exits draft mode or review begins).