Skip to content

fix(ocap-kernel): count remote and terminated importers in getImporters - #1088

Open
sirtimid wants to merge 4 commits into
mainfrom
sirtimid/get-importers-remotes-terminated
Open

sirtimid wants to merge 4 commits into
mainfrom
sirtimid/get-importers-remotes-terminated

Conversation

@sirtimid

@sirtimid sirtimid commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

retireKernelObjects deletes a kernel object once every importer from getImporters has a retireImport queued. getImporters only read vatConfig rows, so it missed remotes and terminated vats whose c-list cleanup had not run yet. Those importers kept c-list entries for deleted krefs, which the refcount audit reports as dangling.

getImporters now returns the union of vat IDs, terminated vats and remote IDs, deduplicated, as EndpointId[].

  • New getRemoteIDs on the kernel store.
  • importsKernelSlot takes an EndpointId.

Merge order: land after #1099. This PR queues retireImports to remotes, and on main a GC action for a remote with no handle, which is every remote until initRemoteComms, still kills the run loop at boot. #1099 holds those back.

Testing

  • gc.test.ts: a remote importer alongside a vat importer, and a terminated vat whose c-list outlived its config.
  • remote.test.ts: getRemoteIDs.

Fixes #1015

🤖 Generated with Claude Code


Note

Medium Risk
Changes kernel GC importer discovery, which affects when objects are deleted and which endpoints receive retireImport; should land with the run-loop fix that tolerates GC to inactive endpoints (#1098).

Overview
Fixes incomplete retireImport notifications when kernel objects are collected by widening getImporters so it no longer scans only active vat configs.

getImporters now deduplicates across live vat IDs, terminated vats whose c-lists still exist, and remote IDs (via new store helper getRemoteIDs), and treats importers as generic **EndpointId**s. retireKernelObjects therefore queues retireImport for remote peers and not-yet-cleaned terminated vats as well, avoiding c-list entries that point at deleted krefs and refcount-audit failures.

Changelog documents the fix and the new getRemoteIDs API; tests cover remote importers, terminated-vat importers, and remote ID listing.

Reviewed by Cursor Bugbot for commit 5b286f5. Bugbot is set up for automated code reviews on this repo. Configure here.

sirtimid and others added 2 commits September 15, 2026 18:56
`retireKernelObjects` deletes a kernel object once every importer
`getImporters` names has a `retireImport` queued. It enumerated `vatConfig`
rows, so it missed remotes entirely and missed a terminated vat whose config
`deleteVat` had already dropped while its c-list waited for
`nextTerminatedVatCleanup`. Either one is left holding an entry that names a
kref the kernel no longer has, which nothing tears down and the refcount audit
reports as dangling.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`getImporters` now returns remotes and vats together and nothing asserted the
sort survived the deduplication. Adds a direct `getRemoteIDs` test, and drops
a cast that was suppressing the check on a `RemoteInfo` literal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sirtimid
sirtimid requested a review from a team as a code owner September 15, 2026 17:21
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 892c00b. Configure here.

Comment thread packages/ocap-kernel/src/store/methods/vat.ts
# Conflicts:
#	packages/ocap-kernel/CHANGELOG.md
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 73.58%
⬆️ +0.22%
10202 / 13864
🔵 Statements 73.34%
⬆️ +0.22%
10340 / 14098
🔵 Functions 73.52%
⬆️ +0.21%
2385 / 3244
🔵 Branches 68.41%
⬆️ +0.35%
4247 / 6208
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/ocap-kernel/src/store/methods/gc.ts 97.56%
⬆️ +6.10%
88.88%
⬆️ +3.70%
100%
🟰 ±0%
97.56%
⬆️ +6.10%
163, 175
packages/ocap-kernel/src/store/methods/remote.ts 98.48%
⬆️ +0.05%
100%
🟰 ±0%
100%
🟰 ±0%
98.48%
⬆️ +0.05%
105-109
packages/ocap-kernel/src/store/methods/vat.ts 98.52%
⬆️ +0.01%
90%
🟰 ±0%
100%
🟰 ±0%
98.51%
⬆️ +0.01%
325-326
Generated in workflow #5207 for commit 5b286f5 by the Vitest Coverage Report Action

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

retireKernelObjects never notifies remote importers, leaving a dangling c-list entry

1 participant