feat: IOListener with accept(), plus anonymous kernel objects and a run-queue cache fix - #1007
Merged
Conversation
`runQueueLengthCache` uses a negative value to mean "unknown, re-read from the DB", but enqueueRun/dequeueRun adjusted it arithmetically without materializing it first. An enqueue while the cache was -1 (its value at daemon startup) produced 0 for a queue that actually held an item, and since 0 isn't negative it was never re-read: the run loop then saw an empty queue, went to sleep, and stranded the queued messages forever, with no error and no log. Also wake the run loop on any non-empty queue rather than only on the empty->1 transition, so a drifted count cannot silently lose the wakeup. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds registerAnonymousKernelObject/releaseAnonymousKernelObject: a kref is allocated and entered in the by-kref routing table, but deliberately not in the service-name index, so the object has no name in the global service namespace and cannot be requested via a cluster config's `services` list. Authority comes from holding the reference. Needed for IOListener.accept(), where each accepted connection is a per-session object that should be reachable only by reference. Returned krefs are handed to kslot() so a kernel service method can return one; krefOf has no allocation path of its own. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…channels Splits the point of contact from the connection, BSD-style. An IOListener is what a cluster config's `io` entry now creates; its accept() yields one IOChannel per peer, each wrapped in its own exo and hosted as an anonymous kernel object, so the vat receives a Presence per connection. Sessions are isolated because they are distinct objects: holding one connection conveys no way to reach another, and `direction` is enforced per connection. IOManager tracks accepted connections per subcluster and releases them when the subcluster (or the listener) goes away. accept() resolves null once the listener is closed, so an accept loop can terminate rather than hang. **BREAKING:** Kernel's `ioChannelFactory` option becomes `ioListenerFactory`, and `IOChannelFactory` is replaced by `IOListener`/`IOListenerFactory`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces makeSocketIOChannel with makeSocketIOListener. The server hands each connection to accept() as its own IOChannel whose buffer, decoder, line queue, and reader queue are all local to that connection, so any number of peers can be served at once. Connections that arrive before accept() is called are queued rather than dropped. Gone with the single-client design: currentSocket, pendingSessionEnd, the merged lineQueue, and the socket.destroy() that rejected every second connection. Session boundaries need no latch now — one channel serves one peer, so the end of the socket simply is the end of the channel. **BREAKING:** makeIOChannelFactory becomes makeIOListenerFactory; makeSocketIOChannel becomes makeSocketIOListener. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eers The io-vat's `repl` endowment is now an IOListener, so it accepts connections and addresses them by index, letting a test drive several peers independently. The integration test drops its hand-rolled duplicate channel in favour of the real makeIOListenerFactory, and adds a case covering two concurrent peers end to end through a real kernel — neither reading the other's data nor receiving the other's writes. That case was unrepresentable before: the second connection was destroyed on arrival. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nSchema
Adds an `InterfaceJsonSchema` variant — `{ type: 'interface', description?,
methods }` — describing an object whose methods can be invoked, so a method
that hands back an object reference can declare the returned object's API
inline and a client need not make a second round-trip to discover it. The
`methods` field is recursive, so a returned interface can itself return
interfaces.
The schema describes an *interface*. Whether the reference to that object is
unforgeable is a property of the reference plumbing, not of the description,
so the same schema serves either case.
service-discovery-types converts the new variant to a `RemotableSpec` via
`interfaceJsonSchemaToRemotableSpec`, which means `remotable` is no longer
among the kinds `JsonSchema` cannot express.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
FUDCo
force-pushed
the
chip/kernel-io-listener
branch
from
August 4, 2026 23:30
705e573 to
0b27067
Compare
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The interface case validates that the value is a non-null object and nothing more — the declared `methods` describe the object for the caller rather than a shape to enforce here, since whether the object honours them is only discoverable by invoking it. Covers both halves: any object passes regardless of its methods, and every non-object is rejected. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Aug 4, 2026
FUDCo
marked this pull request as ready for review
August 5, 2026 01:21
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
sirtimid
reviewed
Aug 5, 2026
| if (!this.#kernelServicesByObject.delete(kref)) { | ||
| return; | ||
| } | ||
| this.#kernelStore.unpinObject(kref); |
Member
There was a problem hiding this comment.
Suggested change
| this.#kernelStore.unpinObject(kref); | |
| this.#kernelStore.unpinObject(kref); | |
| const { reachable, recognizable } = this.#kernelStore.getObjectRefCount(kref); | |
| if (reachable === 0 && recognizable === 0) { | |
| this.#kernelStore.deleteKernelObject(kref); | |
| } |
we need to do this here because collectGarbage skips kernel-owned objects. But I do know that now this will never fire because our base is (1,1), though I m working on #1006 which means this will be fixed
Contributor
Author
There was a problem hiding this comment.
Applied, thanks — and thanks for the pointer about collectGarbage skipping kernel-owned objects, which I'd have missed. Documented at the call site that the branch is a no-op at the current (1, 1) baseline and becomes live with #1006, so nobody later reads it as dead code and removes it.
sirtimid
reviewed
Aug 5, 2026
sirtimid
left a comment
sirtimid
left a comment
Member
There was a problem hiding this comment.
FYI this bug doesn't happen only at startup, rollbackCrank also sets the cache back to -1 (store/methods/crank.ts:57), because a rollback may have put dequeued items back. And a rollback is normally followed right away by enqueuing something (an error or termination message), which is exactly what triggers the bug: the cache becomes 0 while the queue is not empty, and the run loop goes to sleep and never wakes. But the fix here handles that as well.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Aug 5, 2026
…1008) Small, self-contained quality-of-life change to the daemon's log transport, extracted from `chip/orchestration-demo`. Independent of the kernel work in #1007 — this branches off `main` directly rather than stacking. ## Problem `daemon.log` recorded every level. In practice `debug` output — refcount churn especially — dominated the file badly enough to make it hard to read while debugging anything else. On a busy daemon the signal you actually want is buried. ## Change The file transport drops entries below a minimum severity, defaulting to `info`. Set `$OCAP_DAEMON_LOG_LEVEL=debug` to record everything again. Two details worth a reviewer's eye: - `LOG_LEVELS` mirrors `@metamask/logger`'s level ordering locally because `logLevels` isn't part of that package's public surface. If it's ever exported, this should switch to importing it rather than keeping a copy in sync. - It's declared *above* the file-scope logger construction deliberately. The transport factory is invoked during module init, so a later declaration would put `LOG_LEVELS` in its temporal dead zone at exactly the moment it's read. ## Not included The fatal-path handler work that lives in the same file is already on `main` (#966), so this PR touches only the level-filtering lines. ## Validation `@metamask/kernel-cli` builds, lints, and its tests pass. Changelog entry follows in a second commit once this PR has a number to link to. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Observability-only change to log file filtering; no auth, RPC, or persistence behavior is affected. > > **Overview** > **`daemon.log` now skips entries below a minimum severity** (default **`info`**) in the daemon file transport, so noisy **`debug`** lines no longer bury useful output. > > The threshold comes from **`OCAP_DAEMON_LOG_LEVEL`**; set it to **`debug`** to record all levels again. **`makeFileTransport`** compares each entry against a local **`LOG_LEVELS`** map (mirroring `@metamask/logger` ordering, since levels aren’t exported). Changelog documents the behavior change. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 66dc26d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…fetimes Three findings from review: - Closing a listener dropped its sockets but left every accepted connection's kref pinned, since release only ran from a connection's own `close()`. The listener service now tracks what it handed out and releases the outstanding ones when it closes. - A connection's `close()` signalled EOF and only then flushed the receive buffer, so a trailing partial line could still be handed to a later `read()` after EOF had been reported. Closing now discards buffered data first; a peer-initiated end still flushes, since that data arrived before the peer went away. - `releaseAnonymousKernelObject` now deletes the kernel object once nothing references it, rather than leaving it to `collectGarbage`, which skips kernel-owned objects (per review; a no-op at the current refcount baseline, correct once #1006 changes that). Peer disconnect still does not release on its own, and that is deliberate: the holder's c-list still names the kref, so releasing there would make a later call on the dropped reference reach `invokeKernelService`, find nothing registered, and throw — taking down the run loop. That is worse than a leak bounded by the listener's lifetime. Documented at the call site, pending #1006. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ener Follow-up to review on the previous commit: setting `ended` inside `close()` made `handleEnd` return early and skip `onClosed`, so a channel closed by its holder stayed registered with the listener — a long-lived listener would accumulate every session it ever served. The flush-or-discard decision now lives in `handleEnd` and is keyed on `closed`, so both paths reach `onClosed` exactly once while a trailing partial line is still flushed for a peer-initiated end and discarded for a holder close. `makeConnectionChannel` is exported so this is testable directly; the package's public surface is unchanged, since `io/index.ts` does not re-export it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Per review: `rollbackCrank` also invalidates the length cache, and a rollback is normally followed straight away by enqueueing an error or termination message — which is precisely the sequence that trips the bug. That path is more likely in practice than the startup one the entry originally described. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
@sirtimid — that's a better characterization of the bug than mine, and I've updated the changelog to match. I'd framed it as a startup problem because that's the instance I hit: a vat resuming its accept loop during So the rollback path is the likelier of the two in practice, and it means this could bite any vat error path, not just startup. The entry now leads with the silent-failure behaviour and names both paths, flagging rollback as the more probable. Also applied your Both Bugbot findings are addressed too, plus a regression I introduced fixing one of them and it caught on the next pass. One finding I've argued against rather than fixed — releasing connection krefs on peer disconnect would wedge the run loop, since the holder's c-list still names the kref; details in that thread. |
…el ends Node can still emit 'data' after `socket.destroy()`, and `handleData` checked neither flag. A late chunk therefore refilled the queue that `close()` had just cleared, and since `read()` drains the queue before consulting the flags, it would hand that line out after EOF had been reported. Data that arrived before the end is unaffected — it is already queued and stays readable, which is what a peer-initiated end owes its reader. Both halves are now covered by tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
# Conflicts: # packages/kernel-node-runtime/src/kernel/make-kernel.ts # packages/ocap-kernel/CHANGELOG.md
…ous incarnation Per review. `registerAnonymousKernelObject` recorded its object only in the in-memory routing table, but `initKernelObject` and `pinObject` both write to the store — so an anonymous object survived a restart while its routing entry did not. Unlike a named service there is no name to re-register it under, leaving it unreachable but still pinned, accumulating with every restart. Worse, it stayed owned by `'kernel'`, so a delivery to a stale connection kref would reach `invokeKernelService`, find nothing registered, throw, and kill the run loop — the same failure this PR's other fix exists to prevent. Anonymous objects are now recorded in the store and swept at init, before the run queue starts so nothing can be delivered to a stale kref in the meantime. These host things that cannot outlive the process — an accepted socket connection, say — so a survivor is unambiguously garbage. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f18ca41. Configure here.
…vice A throw escaped the crank and killed the run loop. The init sweep cannot prevent this: a (1,1) refcount baseline keeps the object alive.
FUDCo
marked this pull request as draft
August 7, 2026 01:56
FUDCo
marked this pull request as ready for review
August 7, 2026 01:56
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
2 participants
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.
Extracts the kernel-side work developed on
chip/orchestration-demointomain. No demo code is included — every change here is general-purpose kernel machinery. Each commit is independently meaningful and reviewable in order.Why
A vat needed to serve a line-delimited JSON-RPC socket to more than one local client at a time. It couldn't:
IOChannelmodels exactly one bidirectional stream, so the socket server destroyed every connection after the first. Chasing that surfaced two further bugs, one of them a latent kernel defect with a genuinely nasty failure mode.Commits
fix(ocap-kernel): honor the run-queue length cache's invalid sentinelrunQueueLengthCacheuses a negative value to mean "unknown, re-read from the database", butenqueueRun/dequeueRunadjusted it arithmetically without materializing it first. An enqueue while the cache held its startup value of-1produced0for a queue that actually held an item — and because0isn't negative, it was never re-read. The run loop then saw an empty queue, went to sleep, and stranded everything queued behind it. No error, no log, no crash: the kernel just silently stops delivering. The run loop is also now woken by any non-empty queue rather than only the empty→1 transition, so a drifted count can't lose the wakeup either.Latent for a long time — reachable only when something enqueues before the run loop's first length read. Worth reviewing on its own merits regardless of the rest.
feat(ocap-kernel): anonymous kernel-hosted objectsregisterAnonymousKernelObject()/releaseAnonymousKernelObject(): allocate a kref and enter the object in the by-kref routing table, but deliberately not in the service-name index. The object therefore has no name in the global service namespace and cannot be requested via a cluster config'sserviceslist — authority comes from holding the reference.Needed by
accept(), and the naming half is the part we specifically didn't want: a per-session connection should be reachable by reference only.feat(ocap-kernel): IOListener with accept(), replacing single-client channelsThe BSD listen/accept split. A cluster config's
ioentry now creates a listener;accept()yields oneIOChannelper peer, each wrapped in its own exo and hosted as an anonymous kernel object, so the vat receives a Presence per connection.Isolation is structural rather than by discipline: sessions are separate objects, so holding one connection conveys no way to reach another. That matters because the names a vat hands across a non-ocap boundary are plain forgeable strings; scoping them per connection is what stops one client naming another's references.
directionmoves to the connection, where the data actually flows.accept()resolvesnullonce the listener closes, so an accept loop terminates instead of hanging.feat(kernel-node-runtime): socket listener with per-connection channelsmakeSocketIOChannel→makeSocketIOListener. Each connection's buffer, decoder, line queue, and reader queue are local to it, which is precisely why many peers can now be served at once. Connections arriving beforeaccept()are queued rather than dropped.Deleted with the single-client design:
currentSocket,pendingSessionEnd, the merged line queue, and thesocket.destroy()that rejected second connections. The session-boundary latch didn't need replacing — one channel serves one peer, so the end of the socket simply is the end of the channel.test(kernel-test): io-vat accepts connections; cover two concurrent peersThe integration test drops its hand-rolled duplicate channel in favour of the real
makeIOListenerFactory, and adds a case driving two concurrent peers end to end through a real kernel, asserting neither reads the other's data nor receives the other's writes. That case was unrepresentable before — the second connection was destroyed on arrival.feat(kernel-utils,service-discovery-types): interface variant for JsonSchema{ type: 'interface', description?, methods }describes an object whose methods can be invoked, so a method returning an object reference can declare that object's API inline instead of forcing a second round-trip.methodsis recursive. The variant describes an interface; whether the reference is unforgeable is a property of the reference plumbing, not the description, so one schema serves both cases.service-discovery-typesconverts it to aRemotableSpec, which meansremotableis no longer among the kindsJsonSchemacan't express.Renamed API surface
Nothing in this repository is left broken — every in-tree consumer is updated in this PR, and the full suite passes. These renames are flagged
**BREAKING:**in the changelogs because the packages are published and the exported surface changed, so release tooling and any external consumer need the signal:Kernel.make({ ioChannelFactory })Kernel.make({ ioListenerFactory })IOChannelFactoryIOListener/IOListenerFactorymakeIOChannelFactory()makeIOListenerFactory()makeSocketIOChannel()makeSocketIOListener()The behavioural change behind the renames: a vat that previously read and wrote an
ioendowment directly now callsaccept()to obtain a connection first.IOChannelitself is unchanged and still represents exactly one connection.Validation
Full monorepo on this exact tree: 30/30 builds, 52/52 test tasks, lint clean. Files that
mainalso changed since the branch point were three-way merged and individually diffed againstmainto confirm nothing ofmain's was reverted — in particularmain's #958 optional-parameter handling inmethodSchemaToMethodSpecis preserved.The whole stack has also been exercised live: two independent clients holding concurrent connections to one vat, each with its own isolated name table, driving a multi-service workflow end to end.
🤖 Generated with Claude Code
Note
High Risk
Touches core kernel IO, service routing, and run-queue delivery with breaking public API renames; incorrect behavior could strand the run loop or mishandle concurrent RPC sessions.
Overview
Replaces single-client Unix-socket IO with a listen/accept model so vats can serve many concurrent line-delimited peers. Cluster
ioentries now createIOListenerservices; vats callaccept()to get a per-peerIOChannel(Presence), withdirectionenforced on each connection.Breaking renames:
ioChannelFactory→ioListenerFactory,makeIOChannelFactory/makeSocketIOChannel→makeIOListenerFactory/makeSocketIOListener. Node runtime gives each connection its own buffer/decoder/queues; early connects are queued instead of dropped.Kernel adds anonymous kernel objects (
registerAnonymousKernelObject/ release + init sweep) so accepted connections are routable by kref but not by global service name.invokeKernelServicerejects missing services withENDPOINT_UNREACHABLEinstead of throwing (avoids killing the run loop on stale IO refs after restart).Fixes a run-queue length cache bug: enqueue/dequeue now materialize the
-1sentinel before arithmetic, and the run loop wakes on any non-empty queue.JsonSchemagains aninterfacevariant (recursive methods) for inline return-type APIs; service-discovery converts it toRemotableSpec.Reviewed by Cursor Bugbot for commit f7570df. Bugbot is set up for automated code reviews on this repo. Configure here.