Conversation
Capability and error DTOs could be accepted with weaker meaning than the stage 1A contract claims: - AgentServiceCapabilitiesSchema required neither a complete nor a non-empty set, so an omitted Desktop-only capability was indistinguishable from a supported one. Every declared id is now required exactly once while unknown, duplicate, and over-limit sets stay rejected. - capability_unavailable did not require the capability, and any code could carry it. The code now owns the structured fields: capability_unavailable requires capability plus requiredClient (nullable, as in the capability DTO), and every other code rejects both, so a client never falls back to message. - The wire envelope inherited AppError's unbounded message and details. message is now non-empty and capped at 4096, and details is bounded in key count, key shape, key length, value depth, and value size, with credential-, path-, handle-, and identity-shaped keys refused. The mapping from service error codes onto LOCAL_CONTROL_ERROR_CODES is explicit in this file and total by construction; localControl.ts is unchanged.
- Measure the detail value budget on the value's own JSON UTF-8 encoding, so a nested key or array element can no longer hide bytes from the size limit, and run the depth walk plus the encoding check ahead of `JsonValueSchema` so an unmeasurable or pathologically deep value fails closed instead of escaping as a RangeError from the recursive JSON walk. - Drop the forbidden detail-key fragment and path blacklists. They rejected valid diagnostics such as `tokenCount` and `maxTokens` and promised a redaction no schema can perform; key syntax, key length, key count, and the depth/byte budgets stay, and the comment now states that sensitive-data redaction is the producer's responsibility. - State in the local-control mapping comment that `capability_unavailable` and `service_unavailable` both collapse onto `unavailable`, so the capability identity must be preserved by the adapter instead of read back from the code. - Extend the contract suite with the nested-key, large-array, boundary, fail-closed, and diagnostic-field cases.
An accessor- or proxy-bearing in-process value could be read more than once: the depth and size pre-checks measured one reading while the piped `JsonValueSchema` validated another, so a value whose later reads grew past the budget was accepted, and a getter or proxy trap that threw on a later read escaped `safeParse` as an uncaught exception. - Read each detail value once, through property descriptors, into a fresh plain JSON copy, and measure the size budget on that copy, so the bytes that were budgeted are the bytes a caller receives. - Reject whatever is not plain JSON data: accessors, array holes, non-Object.prototype prototypes, symbol keys, functions, bigints, undefined, and non-finite numbers, and turn any throwing read or measurement into a schema issue instead of an exception. - Extend the contract suite with the changing accessor, throwing accessor, throwing proxy, and non-plain object cases, plus the multi-byte UTF-8 size boundary.
Stage 0 record for the standalone agent service, at checked revision 7e758ab (plus the 4c07c5b architecture baseline and the four Stage 1A contract commits). baseline.md freezes the Desktop and direct ACP flows, the identifier and ownership matrices, the capability classification with the first-version allowlist, resource ownership through shutdown, the portable-import probe evidence, the invariants, and the Stage 1 handoff. Two blockers are recorded instead of claimed as passes: - The minimum two-turn/tool-continuation scenario was not completed: Node 22.22.0 is outside the declared >=24.18.0 <25, the installed Electron 41.10.4 does not match the lockfile 43.6.0, and there is no headless composition root to run it against. - safeStorage is unavailable outside a full Electron runtime and is imported at module scope by the credential store, so credential access blocks Stage 3. plan.md Stage 0 checkboxes are flipped per evidence, the unproven scenario is satisfied only through its blocker branch, and the reviewed-inventory line stays unchecked pending review. A Stage 1 sub-slice rule records 1A DTO-only, 1B events/interaction/cancellation, 1C client adapters, and 1D compatibility mapping so the delegation boundary lives in the repository. Documentation only: no production code, test, or configuration change.
Adds the Stage 1B client-facing DTOs and nothing else: the typed event envelope, bounded event subscription with epoch:seq cursor replay, overflow and resync semantics, the authoritative snapshot, owned artifact references, interaction/approval request-response DTOs, and three separate cancellation layers. Events reuse LocalControlEventCursorSchema and the Stage 1A ids. Every object is strict and every union discriminated, so unknown fields, wrong discriminators, missing cursors or decisions, illegal cancel layers, and oversized payloads fail closed; no principal, approver, renderer, handle, path, or runtime object is representable. Nothing imports these modules. Verification: test/main/contracts (71 passed, 14 new), targeted tsc over the new files, oxfmt --check, oxlint. Full typecheck:node/web fails only on pre-existing dependency drift (tokenx, @ai-sdk/open-responses) reproduced in the untouched main checkout; host Node is 22.22.0 against engines >=24.18.0.
Event data is read into a bounded plain copy before `JsonValueSchema` sees it. The recursive schema threw `RangeError` out of `safeParse` for a payload deep enough to exhaust the stack (a `JSON.parse` result at depth 4000 or 40000), which is not fail-closed. The read is iterative and bounded in depth, node count, and encoded bytes, refuses cycles, array holes, accessors, symbol keys, non-plain prototypes, and non-JSON values, and measures the copy it returns, so the byte budget is spent on what a client receives. Replay validation requires every replayed event to stay in the requested cursor epoch, not only the first: a switch in the middle with contiguous sequences and a matching `initialCursor` used to pass as one gap-free catch-up. A snapshot's pending interactions must belong to the snapshot's session, and `expired` is no longer accepted with `resumed: true`, because an expiry records a response that was not applied. Verification: test/main/contracts 74 passed (17 in the event contract; 3 new tests); the new cases fail against the previous sources (RangeError, mid-replay epoch switch, cross-session pending interaction, expired+resumed). oxfmt --check and oxlint clean. typecheck:node/web fail only on pre-existing dependency drift (tokenx, @ai-sdk/open-responses) plus pre-existing renderer errors; host Node is 22.22.0 against engines >=24.18.0.
The event-data read pushed a frame for every element or key of a container before it looked at the node budget, so the input's width, not the budget, decided how much memory the read materialized: a five-million-element array was enumerated in full (10,000,002 own-key and descriptor reads over 13.25s) before being refused as oversized, and the comment claiming the node check "stops an oversized payload from being copied in full first" was not true for width. A container is now refused before it is expanded when its width cannot fit the node budget next to the nodes already counted. The node budget is the byte budget — every node costs at least one encoded byte plus the delimiter joining it to its parent — so this is a work bound, not a second acceptance rule: a payload the byte check accepts has at most ~131k nodes, and the node check could never have refused one the byte check accepts. An array's width comes from `length`, so a wide array is refused without even listing its keys; an object's key list is what `Object.keys` returns, so only that list is still built at the input's own width. Frames held at once are now bounded by the depth budget times the node budget instead of by the input. An own `__proto__` key is refused at any depth instead of being copied. `Object.defineProperty` kept the key in the measured copy, but the record stage the copy is piped into writes keys by assignment, and `__proto__` is the one key where that sets a prototype instead of creating an own property, so the returned DTO was a different value from the one that was measured: `JSON.parse` of a document with that key was accepted and came back without it. `-0` is refused too: it is encodable but not preserved (`JSON.stringify(-0)` is `'0'`), so it does not survive the round trip the byte budget measures. Every other number semantic is unchanged, and -0 is the only finite double JSON does not reproduce. Verification: test/main/contracts 77 passed (20 in the event contract, 3 new cases). Against the previous sources the new cases fail: `__proto__` and `-0` are "accepted", the wide-array case reads all 10,000,002 keys and descriptors over 13.25s where the new suite runs in 173ms, and the nested budget case visits 131,071 children before refusing where it now visits none. oxfmt --check and oxlint clean. typecheck:node (9) and typecheck:web (5) report only the pre-existing dependency drift (tokenx, @ai-sdk/open-responses) and pre-existing renderer errors, with host Node 22.22.0 against engines >=24.18.0.
Adds the Stage 1C client-facing adapter boundary and nothing else: the typed handshake, submission, and submission-query DTOs, one closed operation vocabulary, the operation-to-capability mapping, one pure refusal resolver, and the adapter surface a binding implements. Stage 1A/1B are imported unchanged and no runtime, transport, Electron, ACP runtime, or CLI implementation is added. The submission DTO is the piece 1A/1B left open: 1A defined the receipt but no request, so a submission had no idempotency identity to be receipted against. `submissionId` is required, and a separate query request reads the receipt a lost response would have carried, because a missing receipt is not proof that no run started. The submitted text shares the snapshot's message bound, so a submission a client may make is one the transcript can report back. One adapter value with seven DTO-facing operations serves both bindings instead of one interface per binding. Their feature differences stay visible as data: the handshake returns the service's complete capability statement, an operation that depends on a capability is refused through `resolveClientOperationRefusal`, and `requiredClient` is copied from that advertisement rather than chosen by the caller. A capability no client can supply reports null; absent, unavailable, or self-contradicting advertisements refuse instead of reading as support, so a missing capability can never be mistaken for a working one. Nothing in the surface or in any request schema can carry an identity: no principal, renderer, approver, clientKind, asDesktop, callback, AbortSignal, handle, or absolute path is representable, and every object is strict so such a field is rejected. Verification: test/main/contracts 114 passed (37 new in agentServiceClientContract.test.ts); full test/main 9137 passed, 5 skipped, 1 file skipped. `pnpm run lint` and `pnpm run i18n` clean; `oxfmt --check .` clean over 3010 files; typecheck:node and typecheck:web both clean on Node 24.18.0. The repo's default typecheck gate does not include `test/**`, so the file's type-level assertions were checked with a temporary tsconfig extending tsconfig.node.json (test/main/contracts plus the agent-service contracts, removed afterwards): clean, and it fails when the surface is mutated to take an options bag with an AbortSignal. Ablation: the shared surface is one interface plus one resolver, and the resolver is the only place that decides a refusal, so an adapter cannot invent a required client at a call site. Rejected as unproven: a generic RPC envelope, an adapter registry or service locator, a capability index object cached per adapter, a per-binding interface pair, a handshake echo of the requested version under exact negotiation, a capabilities accessor beside the handshake, a caller-supplied refusal builder, an AsyncIterable subscription (live delivery is the transport's, Stage 4), and session lifecycle operations. The two fakes in the suite are fixtures, not shipped adapters, and a dead capability flag in one of them was made live or removed instead of left as decoration.
Four corrections and two boundary records on top of the Stage 1C client adapter commit. No Stage 1A/1B DTO change, and no runtime, transport, Electron, ACP, CLI, event, or database code. **A capability advertisement must agree with itself.** `common.ts` validates `reason` and `requiredClient` independently, so an advertisement could state `requires_desktop_client` without naming a client, or claim `requiredClient: 'desktop'` under `not_supported`. The handshake result now refuses a self-contradicting set instead of repairing it, and `resolveCapabilityRefusal` copies `requiredClient` only from a consistent entry: everything else refuses with `null`, so no caller is pointed at a Desktop lease the advertisement's own reason denies. **The contract test's type assertions are compiled.** `expectTypeOf` is erased at runtime and `tsconfig.node.json` covers `src/**` only, so the statements that this surface takes exactly these DTOs were enforced by nothing. `typecheck:contracts` compiles the client contract test together with the agent-service contract sources, is wired into `typecheck`, and fails when `submit` takes an options bag with an `AbortSignal` or a public request type gains a host type. **A binding that keeps no receipt no longer answers `not_found`.** A receipt query now answers an outcome: `receipt`, or `receipt_not_retained` when the binding cannot answer at all. `not_found` for a submission the binding accepted and executed reads as "no run started", which is the reading that licenses the resubmit this path exists to prevent. No new error code was added, and this is a value rather than a capability refusal because `session.persistence` gates the query while the same binding must keep advertising it to express a snapshot; a receipt-retention capability id would be a Stage 1A vocabulary change. **An interaction answer is addressed.** The 1B response DTO names its session, interaction, message, and tool call but no service instance, and one client can hold adapters for more than one instance. `respond` now takes a strict client-side envelope stating the instance plus the run and request the published interaction belongs to. No principal, approver, renderer, or client-kind field exists in it, and the 1B file is unchanged. **Hand-off, where the next slice can cite it.** `client.ts` records that live delivery is the local transport slice's concern and that no `AsyncIterable`, callback, or long-connection handle is introduced here, and that steering, the pending-input queue, and session lifecycle are not client operations on this surface. A contract test pins both boundaries to the frozen seven-operation vocabulary. `plan.md` is deliberately untouched. Verification: `test/main/contracts` 121 passed (44 in the client contract test); full `test/main` 9144 passed, 5 skipped, 1 file skipped; `typecheck:contracts`, `typecheck:node`, `typecheck:web`, and the composed `typecheck` clean; `oxfmt --check .` clean over 3010 files; `lint` and `i18n` clean. Ablation: reverting the consistency guard fails the two capability tests; reverting the receipt-retention outcome fails the two query tests; dropping the instance check in `respond` fails the addressing test; and the new gate exits non-zero for a `submit` that takes an `AbortSignal` options bag and for a submission request that gains `z.instanceof(AbortSignal)`.
The gate's diagnostic filter keeps only diagnostics that carry a scoped
file, so TS6053 ("File ... not found.") for an absent root was dropped,
the program simply got smaller, and the gate exited 0 — losing the
`expectTypeOf` assertions it exists to enforce. Scoped roots are now
checked against the filesystem before compiling, and the failure names
the missing relative path on stderr.
The root check used `ts.sys.fileExists`, which is true for a `chmod 000` root, so an unreadable root still compiled a smaller program and the gate exited 0 — the false green the missing-file check was meant to close. Roots are now read: an undefined `ts.sys.readFile` marks the root absent or unreadable, the report keeps the two apart, and stderr names the relative path. The gate regression test covers the unreadable root with a real `chmod 000` (restored in `finally`, mode and content asserted), skips with a note where mode bits are not enforced, drops the fragile empty-stderr assertion, and normalizes separators in path expectations.
The gate compiled a smaller program and reported a pass whenever a scoped root could not be read: the scope filter dropped file-less diagnostics, which is where TypeScript puts the config, option, global, and missing-root TS6053 errors, so only in-scope file diagnostics survived. File-less diagnostics are now kept alongside `parsed.errors`, and the root readability check stays for the missing/unreadable distinction. A config using a deprecated compiler option now fails on TS5101 instead of passing. The regression test no longer deletes or chmods the tracked contract test. Each case runs a copy of the real script in a `mkdtemp` fixture holding a minimal tsconfig, a scoped source, a scoped contract test, and a junction to the resolved `node_modules`: pass, missing root, unreadable root, in-scope type error, unparseable config. Path expectations normalize `\` so they match `path.relative` output on Windows, and success asserts the pass line rather than an empty stderr. Verification (Node 24.18.0): `typecheck:contracts` and composed `typecheck` exit 0; `test/main/contracts` + `test/main/scripts` 38 files / 382 tests pass, with the tracked client contract test byte-, mode-, and inode-identical before and after the run and no fixture left in the temp dir; `lint`, `i18n`, and `oxfmt --check .` (3010 files) clean. Ablation, in fixtures: a `fileExists`-only root check exits 0 for an unreadable root, and removing the root check while re-filtering file-less diagnostics exits 0 for a missing root — both fail the suite with "expected 0 to be 1". Dropping in-scope file diagnostics fails the type error case.
Stage 2B slice 2B-3a: extract the portable built-in kernel into the private workspace package @deepchat/agent-kernel. - Move the loop/runtime/memory/resources/instance/contracts owner set, the tape domain/ports closure, and the neutral collaborators (lib, hook, memory injection, provider ports, skill, tool, session helpers) into packages/agent-kernel; every historical src/main path keeps a one-line re-export shim so host imports are unchanged. - Copy the @shared value/type modules the kernel closure needs into the package (NodeNext emit forbids path aliases); vitest keeps a single shared-module instance through a bridge back to src/shared. - Extract createDeepChatRuntimeServices and the pending-input wakeup binding into the package; the Desktop facade injects the vision, image-preview, and programmatic-tool-parent collaborators and assembles the ACP compatibility factory over kernel-exposed owners. The kernel services type no longer carries ACP or host types. - Build with plain per-file tsc emit (JS and .d.ts) plus a dist hygiene scan: no @/, @shared/, src/main, electron, better-sqlite3, or node-pty specifiers in emitted output. - Carry-overs from the 2B-2 acceptance note: restore the restart-held queue retry exemption for ACP sessions through the shared pending-input admission read, and backend-route session deletion destroy so ACP sessions never touch the built-in lifecycle.
Regenerate the architecture baselines after the 2B-3a kernel extraction: owner evidence and runtime boundaries now point into packages/agent-kernel, the agent source inventory spans the workspace package, and the loop dependency metrics scan the package loop tree.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
What
Standalone agent service groundwork per
docs/architecture/standalone-agent-harness/{spec,plan}.md: frozen wire contracts and client adapters, ACP ownership seams, and the built-in agent kernel extracted into a workspace-private package — proven to run end-to-end outside Electron by a clean-Node gate.No UI changes. No behavior change on the Desktop path. ACP status publication fixes a pre-existing cold-session first-
generatingdrop and adds close-time terminal finalization.Stage summary (each slice independently accepted; evidence in plan.md acceptance notes)
853cc953a): cut five Electron/application value-import leaks out of the kernel tree (shared logger, ACP prompt builder, programmatic launch error, usage label helpers, provider db source url).2eac46f47…4ca72e501): ACP state seam — the direct ACP backend uses a neutralAcpSessionStateAdapter(9-methodSessionStatePort) over the host settings store instead of hydrating built-in scope.97d9fc68c): kernel dependency narrowing — 26 contracts modules, 11 named structural ports (TranscriptStorePort,SessionSettingsStorePort,PendingInputStorePort,TapeStorePort, …); allPick<HostClass>removed from kernel modules; newtypecheck:kernel-portsgate gives the structuralexpectTypeOfassertions real enforcement.200e51e94): ACP compatibility factory neutralization — ACP-own status publication through existing typed host channels, ACP-ownedDeepChatAgentInstances with injectable ownership fences (built-in semantics byte-equivalent), backend-routed transcript/skills/destroy so ACP sessions never hydrate built-in scope.f17afd0de,a0299f80a): physical extraction intopackages/agent-kernel(@deepchat/agent-kernel, workspace-private): loop/runtime/memory/resources/instance/contracts/collab/composition + 69 physical@sharedcopies; 175 one-line re-export shims keep all host imports unchanged; plain-tsc build (492 emitted files) with a forbidden-specifier dist scan; Desktop embeds the same kernel via the workspace link.859450a3a,483ae8efd): the clean-Node gate below.How to verify the gate
Three tests (~1.4 s): builds the package, checks the 69
@sharedcopies stay faithful tosrc/shared(specifier-normalized compare, drift fails the build), then amkdtempconsumer running a real Node child process (syncmodule.registerHooksinterception) imports only the package entry and completes a two-round tool-continuation turn: provider request → tool admission/execution → tool result present in the next provider request → final settlement, durable transcript, observable event order — with no client callback between rounds (settles viaonSessionCompleted). It also proves swallowed forbidden imports (electron,better-sqlite3,node-pty) still exit non-zero, and that the emitted.d.tsclosure is alias-free with every external specifier inside the kernel's declared dependencies, consumable by an externaltsc --noEmit.Validation
test/main: 650 files / 9178 tests (+5 pre-existing skips)typecheck(node / web / contracts / kernel-ports),format:check,lint,i18n,git diff --check: all greenpnpm install --frozen-lockfilepasses (kernel importer added to the lockfile)Notes for reviewers
docs/architecture/standalone-agent-harness/plan.md.ollamais a type-only runtime dependency of the kernel because the publicProviderRuntimePortsurface inherited from@sharedexposesShowResponse.plan.md; safeStorage is the documented blocker.Draft while review proceeds.