Skip to content

feat(plugins): plugin contract v2 and one provider registry - #3767

Merged
thymikee merged 2 commits into
mainfrom
feat/plugin-registry-v2
Oct 17, 2026
Merged

thymikee merged 2 commits into
mainfrom
feat/plugin-registry-v2

Conversation

@thymikee

@thymikee thymikee commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

Part 1 of 4 of #3755 (umbrella #3747). Blocked by #3748: do not merge before it.

One plugin contract and one provider registry. Built-in providers, store plugins and (later) embedder plugins register through the same shape, in that order.

  • agent-device/plugins exports the type-only v2 contract: AgentDevicePluginFactory, { apiVersion: 2, provider, credentials, connection?, setup(host) }. { webDriver } stays a data descriptor.
  • A package's ./agent-device export is resolved with import conditions. The root export is used only when that subpath is absent. A throwing subpath never falls back.
  • v1 packages load through an adapter, dispatched by manifest before any factory runs.
  • A plugin that fails to load, validate or set up is recorded and skipped. The first registration of a provider id claims it; later ones fail with duplicate_provider. cloud and proxy stay reserved.
  • The inventory (active/failed, typed error.reason, credentials with byBackend) is published in daemon.json. The client fingerprint reads variable names from it and falls back to v1 manifests.
  • Retires RESERVED_PLUGIN_PROVIDERS and DEFAULT_PROVIDER_RUNTIME_REQUIRED_IDS; their consumers (provider policy, fingerprint, connect capabilities, daemon composition) move in this PR.
  • A v2 plugin declares connect capabilities in code, so the client loads the store (factories only, no setup) before reading them.

Also: a Limrun fingerprint now hashes untrimmed values, like the other providers. 59 files; ~3k gross lines, mostly the replaced loader and its tests.

Stack: this → feat/plugin-registry-v2-status (daemon status) → docs/ADR 0034 → @agent-device/testmu.

Validation

pnpm check:affected --run --head a304bb7f40 (the stack top, which contains this layer): passed (exactCommit). Packed-types consumer type-check added to the installed-package test. CI owns integration, coverage and device lanes.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.34 MB 5.37 MB +27.5 kB
Package (unpacked) 5.34 MB 5.37 MB +27.5 kB
Package (download) 1.61 MB 1.62 MB +8.8 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.5 ms 26.7 ms +0.2 ms
CLI --help 76.9 ms 76.4 ms -0.5 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 59 files

You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread test/integration/provider-scenarios/harness.ts
Comment thread src/__tests__/provider-device-runtimes.test.ts Outdated
Comment thread packages/plugins/src/manifest.ts Outdated
Comment thread packages/plugins/src/store.ts
Comment thread packages/plugins/src/validate.ts Outdated
Comment thread packages/plugins/src/load.test.ts Outdated
Comment thread packages/plugins/src/legacy.ts Outdated
Comment thread src/__tests__/test-utils/builtin-plugin-inventory.ts
Comment thread src/provider-credential-fingerprint.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 16 files (changes from recent commits).

You’re at about 93% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/plugins/src/registry.ts Outdated
Comment thread packages/plugins/src/load.ts
@thymikee
thymikee force-pushed the feat/plugin-registry-v2 branch from 2f64746 to deae993 Compare October 16, 2026 22:35
Comment thread packages/plugins/src/load.test.ts Fixed

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 5 files (changes from recent commits).

You’re at about 93% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/plugins/src/store.ts Outdated
@thymikee
thymikee force-pushed the feat/plugin-registry-v2 branch from deae993 to 447eb9e Compare October 16, 2026 23:02
Comment thread packages/plugins/src/load.test.ts Fixed

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 13 reviewed files. 1 file intentionally excluded from review.

You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread src/provider-plugin-policy.ts Outdated
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-17 12:46 UTC

@thymikee

Copy link
Copy Markdown
Member Author

I reviewed 9096044 and found one defect that must be fixed before merge. CI is green: 23 checks, none failing at this commit. I know of no conflicts.

withDisposal in registry.ts wraps any runtime whose setup returns dispose in a Proxy. If the plugin returns a frozen runtime, the Proxy get trap throws a TypeError on every access to ownsDevice, getInteractor, deviceInventoryProvider and shutdown. The contract types the runtime as Readonly, and Object.freeze is this repo's idiom. I reproduced this in a standalone node snippet: "'get' on proxy: property 'ownsDevice' is a read-only and non-configurable data property". assertProviderRuntime checks the unwrapped object, so the plugin shows as active in daemon.json. Then its first lease or the daemon shutdown throws a raw TypeError, and its dispose never runs. The dispose test at load.test.ts:255 uses a plain mutable runtime, so it cannot show this. The rule should be that the registry never changes the identity or property semantics of the runtime a plugin returns, and each plugin's dispose runs exactly once, after its runtime shuts down. Could you keep dispose on ProviderRegistration and run it from the one shutdown owner? registry.close() already exists, but only tests call it, so the daemon could stop through it. Then withDisposal can go. Please add a test with an Object.freeze'd runtime that fails on the current Proxy.

Not blocking, and you can take or leave these: the built-in providers are described twice, in builtin-provider-plugins.ts and in builtin-provider-ids.ts (the client reads that one, and the Limrun candidate does not declare its capability table), and the cloud candidates' connection.resolve/verify never run in production; the integration harness at provider-platform-registrations.ts:9 now holds the retired classifier, and its header comment claims daemon parity the daemon no longer has, so it could build registrations through startProviderRegistry(resolveProviderPlugins(builtinProviderCandidates(env)), env, PROVIDER_PLUGIN_POLICY); limrun.md:32 still says provider_runtime_skipped while the daemon emits provider_plugin_failed, plugins.md covers only v1 authoring, and there is no changeset or help/CHANGELOG entry for the Limrun fingerprint change, so either add them here or say the stacked docs PR owns them; PluginProviderRuntime in contract.ts:56 names only provider and shutdown while assertProviderRuntime also requires leaseLifecycle, ownsDevice, getInteractor and deviceInventoryProvider, and ProviderRegistry.close, the embedder option and the ./inventory export have no production caller.

This PR rewires how the daemon builds every cloud provider runtime, and the only validation shown is check:affected. Could you post one live run at this commit? Start the daemon with LIMRUN_API_KEY and show plugins in daemon.json with limrun active. Run connect limrun, then open and snapshot, so a lease is allocated through the registry-registered runtime. Rotate LIMRUN_API_KEY in the shell and show provider-credentials-changed on the next allocation. Then run disconnect, stop the daemon, and show the runtime shuts down cleanly. I ran no tests myself, and I did not trace how #3748 handles a v2 store plugin whose import fails before its provider id is known.

Could this land smaller? The diff is 1230 net production lines in one commit. One built-in descriptor list (id, credentials, connection capabilities, lazy setup) could feed both builtinProviderCandidates and the client, which would delete builtin-provider-ids.ts, the dead candidate connection callbacks and BUILTIN_CONNECT_PROVIDER_IDS. If the registry returned registrations with dispose and the daemon shut down through registry.close(), that would delete withDisposal. Resolving capabilities once in an async connect-provider resolver would replace priming a global before synchronous reads. That points to a split: layer A is contract, registry, daemon composition and inventory in daemon.json, and layer B is the client side. The PR body does not say why a narrower split than the four-layer stack was rejected, so please answer that. Would landing ADR 0034 first settle who owns built-in descriptors and plugin shutdown before the code does?

The open cubic-dev-ai thread on the test-only classifier in the harness (#3767 (comment)) is marked resolved but still applies. Please fix the frozen-runtime shutdown path, then post the live Limrun run, before merge.

@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the review. Fixed in 105f415 (stack top 7f449c5 passed pnpm check:affected --run).

Blocking: frozen runtime. withDisposal and the Proxy are gone. The registry no longer touches the runtime a plugin returns. dispose sits on the registration, and registry.close() is the one shutdown owner: for each registration it runs runtime.shutdown(), then dispose, even if shutdown throws, and the whole close runs once. The daemon now shuts providers down only through registry.close() (the deps field shutdownProviderRuntimes replaces the runtime list, so the server never reaches into a runtime). New test: an Object.freezed runtime registers, serves ownsDevice, and is shut down and disposed exactly once across two closes. It fails on the old Proxy.

Live run (this build, fake LIMRUN_API_KEY=lim-live-key-1, empty plugin home, HTTP daemon):

  • Before start: daemon status --json -> state: stopped.
  • After a command started the daemon: daemon status --json -> state: running, plugins = browserstack, aws-device-farm, limrun, all builtin / active; limrun lists byBackend ios-instance and android-instance.
  • agent_device.lease.allocate for limrun with the daemon's key was accepted; with the key rotated to lim-live-key-2 it was refused: INVALID_ARGS, reason: provider-credentials-changed.
  • daemon stop --json: stopped: true, mode: graceful; daemon status afterwards: stopped. The shutdown report listed the one lease I allocated as a pending limrun release, which is expected with a fake key.
  • Not run: connect limrun, open, snapshot and a real lease/release. They need a real Limrun account, so there is no live device evidence in this PR.

Harness classifier: the harness takes pre-built runtimes from each test, so it cannot go through the registry; I rewrote the header so it no longer claims daemon parity. PluginProviderRuntime: the doc comment now says it names only provider and shutdown and lists the other facets the host requires. Docs/changelog: limrun.md, the plugins page and the Limrun fingerprint change are in the stacked docs PR (#3769).

Smaller split? I kept four layers. Contract + registry cannot land without the client-side priming: a v2 plugin declares its connect capabilities in code, so the client has to load the store before the synchronous reads in connect/connection status. Moving the built-in descriptors into one list and resolving capabilities in an async connect resolver are both fair follow-ups; I left them out of this round to keep the contract change reviewable. I have no objection to landing ADR 0034 first if you'd rather settle ownership before the code.

#3748 / #3722: still blocked on #3773. I'll rebase and add the failed inventory entry once it merges; #3722 would conflict in the same plugin files, and I'd keep this branch's versions and take its R85 rule.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 10 files (changes from recent commits).

You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/daemon/src/server/http/daemon-runtime-composition.test.ts">

<violation number="1" location="packages/daemon/src/server/http/daemon-runtime-composition.test.ts:71">
P2: This test no longer checks that every provider shuts down when one throws. The per-provider isolation moved into `registry.close`, and no test covers it, so a regression to a sequential loop would pass unnoticed. Add a registry test where one registration's `runtime.shutdown()` throws and assert the others still run, or rename this test to match what it now asserts.</violation>
</file>

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/daemon/src/server/http/daemon-runtime-composition.test.ts
@thymikee
thymikee force-pushed the feat/plugin-registry-v2 branch from 105f415 to bd57cd7 Compare October 17, 2026 05:23
@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the update. The registry shutdown fix and the per-plugin isolation test are in at bd57cd7, and all 23 checks pass. One piece of evidence is still missing, so I can't call this ready yet.

The posted run used a fake LIMRUN_API_KEY. It covered the daemon.json inventory, the credential-fingerprint refusal and a graceful stop. It never served a lease from the Limrun runtime that the registry registers, and it never released that runtime through registry.close. src/provider-device-runtimes.ts:43 now rebuilds every cloud provider runtime through the v2 registry, and daemon shutdown goes through registry.close. A regression in how the registry adapts or stops a real cloud runtime would ship without one exercised lease. The "needs a real account" reason does not hold, because the key is in the main checkout .env. Please load only LIMRUN_API_KEY from that file into the worktree dist, with a fresh AGENT_DEVICE_STATE_DIR. Then run connect limrun --platform ios, open com.apple.Preferences, snapshot (any non-empty tree), close, disconnect and daemon stop --json. Please post three outputs: the daemon.json plugins entry showing limrun active, the snapshot JSON, and a stop report with no pending limrun release. Then confirm no instance leaked via iosInstances.list.

Not blocking, and you can take or leave these: the composition tests in provider-device-runtimes.test.ts still shut down through composition.runtimes, and the daemon tests mock shutdownProviderRuntimes, so nothing exercises composition.shutdown, the daemon's only provider-shutdown route (replacing daemon-composition.ts:129 with async () => {} would still pass); making that test call await composition.shutdown() and assert that a store plugin's dispose ran would close it. Two smaller items: Promise.allSettled([deps.shutdownProviderRuntimes()]) at daemon-runtime.ts:736, 835 and 906 is a one-element allSettled where .catch(() => {}) says the same, and the lock-loss test is still named "shuts down every constructed provider" but now asserts one callback.

The cubic-dev-ai P2 thread on per-plugin shutdown isolation is fixed at this head, because load.test.ts:576-612 now covers it, so you can resolve it: #3767 (comment)

I ran no tests and judged the regression tests by reading them. I did not check the output of the fake-key run beyond the comment text, did not read the stacked docs PR #3769, and did not trace the #3748 failed-import inventory entry that you say waits on #3773. No conflicts are known. Before merge, please post the real-key Limrun run at bd57cd7, so the stop goes through registry.close.

@thymikee
thymikee force-pushed the feat/plugin-registry-v2 branch from bd57cd7 to aa98411 Compare October 17, 2026 07:32
@thymikee

Copy link
Copy Markdown
Member Author

Real-key run, as asked. Build: the stack top 2dd219b18b (contains this PR's code; rebased onto main at bb05c63025). Only LIMRUN_API_KEY was read from the main checkout's .env into the shell (nothing copied into the worktree); fresh AGENT_DEVICE_STATE_DIR and AGENT_DEVICE_HOME; tokens redacted.

$ agent-device connect limrun --platform ios --json
  connected: true, leaseProvider: limrun, leaseBackend: ios-instance
  verification: { status: verified, service: Limrun, message: "Credentials and iOS instance access verified." }
  liveSession: not-created, device: deferred (Provider-selected iOS simulator)

$ agent-device open com.apple.Preferences --json
  success, appBundleId: com.apple.Preferences, device: "Limrun ios ios_euna",
  selection: { reason: single-provider-device, source: provider }, startup.durationMs: 567

daemon.json plugins (limrun entry)
  [{"provider":"limrun","source":"builtin","state":"active","backends":["ios-instance","android-instance"]}]

$ agent-device snapshot --json     # 17 nodes
  e1 Application "Settings"
  e2 Heading "Settings"
  e3 Button "Apple Account, Sign in to access your iCloud data, ..."
  e4 Button "General"
  e5 Button "Accessibility"  ...

iosInstances.list while the session is open: { ready: 1, terminated: 49 }

$ agent-device close --json       -> success, provider.limrunInstanceId: ios_euna_01m54de29wfzqbepm583dky220
$ agent-device disconnect --json  -> connected: false
$ agent-device daemon stop --json
  stopped: true, mode: graceful, cleanupConfidence: known,
  providerReleases: { status: completed, released: [], pending: [] }, warnings: []
$ agent-device daemon status --json -> state: stopped

iosInstances.list after stop: { terminated: 50 }   # the ready instance is gone; nothing leaked

So a lease was served by the Limrun runtime the registry registers, snapshot returned a non-empty tree, and the stop went through registry.close() with no pending limrun release.

@thymikee
thymikee force-pushed the feat/plugin-registry-v2 branch from aa98411 to 9467595 Compare October 17, 2026 08:01
@thymikee

Copy link
Copy Markdown
Member Author

Taken in 9467595 (stack top 73a0bbc passed pnpm check:affected --run): the composition tests now shut down through composition.shutdown(), and a new test registers a store plugin with dispose and asserts composition.shutdown() ran its runtime shutdown and then its dispose (it fails if the composition's shutdown becomes a no-op). The three daemon sites now use deps.shutdownProviderRuntimes().catch(() => {}), and the lock-loss test is renamed. The real-key Limrun run is above.

@thymikee
thymikee added this pull request to stack #3779 October 17, 2026 08:16
@thymikee

Copy link
Copy Markdown
Member Author

I found no code problems at 9467595. The earlier findings from bd57cd7 are fixed, and nothing from them remains.

The PR now has merge conflicts with main. After #3722 merged, 18 files conflict, including packages/plugins/src/load.ts, manifest.ts and store.ts, src/provider-device-runtimes.ts and provider-policy.ts. Please rebase and re-run the gate.

Smoke Tests is still running and shows no failure yet. Every daemon start goes through createDaemonProviderRuntimeComposition and composition.shutdown, which smoke exercises, so a failure there would be relevant. The other 22 checks pass.

I ran no tests and judged the new shutdown test by reading it. The real-key Limrun run was on 2dd219b, not on 9467595. I accepted it because the changes since then are test-only plus the .catch rewrite. I did not resolve the conflicts, so the rebased shape of the conflicting files still needs a look.

After the rebase and a green gate, this is ready for human review once #3748 lands.

@thymikee
thymikee force-pushed the feat/plugin-registry-v2 branch 2 times, most recently from 1fb2e78 to 5ccff7f Compare October 17, 2026 09:32
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto main (0563666b09) and reworked after the holistic review. Validation: pnpm check:affected --run --head c09dbc6c4f passes on the stack top.

  1. One cleanup owner. registry.close() is idempotent and returns a structured PluginCleanupOutcome per plugin and step (runtime shutdown, then dispose). Runtimes are no longer wrapped (the Proxy is gone), so frozen runtimes work, with a test. The type is exported from @agent-device/contracts/plugin-inventory. The daemon logs a failed step as provider_plugin_cleanup_failed.
  2. Transactional composition. createDaemonRuntimeDeps closes the registry when any later step throws and hands nothing over; on success the daemon owns shutdownProviderRuntimes. Tests cover both.
  3. Exports resolution. Entry discovery follows Node's package exports rules (patterns, null conditions, condition order, target containment), with the two named tests plus a conformance test against real Node resolution.
  4. Candidates in, selection out. The registry takes candidates; composition picks built-in, store and embedder sources. startDaemonProviderRegistry is removed.

Still blocked on #3748 (#3773): not mergeable before it. After it merges I'll rebase and add the failed inventory entry to provider-not-registered.

@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the rework. In the earlier review (#3767 (comment)) the conflict was the open item, and no conflicts are known now. At 5ccff7f one defect remains, the cleanup logging, and two things stay open before merge.

The failed shutdown or dispose outcome from closeProviders never reaches daemon.log. closeProviders reports it with bare emitDiagnostic. It runs at the lock-lost exit (770), the startup-failure catch (869) and shutdown() (940). None of these is inside withDaemonDiagnosticsScope, and emitDiagnostic returns at once without a scope (host-kit/src/internal/diagnostics.ts:141-142). The same file says so at line 898. So when a plugin's runtime.shutdown or dispose throws on SIGTERM, SIGINT, idle reap or startup failure, daemon.log gets no record, and the provider_plugin_cleanup_failed claim in the PR does not hold. The rule: every daemon event emitted outside a request goes through emitDaemonDiagnostic(logPath, ...), which makes the scope, as provider_plugin_failed already does at line 839. Please use it in closeProviders, and decide on purpose what the lock-lost path does, since that daemon does not own the log. Then add a test in daemon-runtime-metadata-ownership.test.ts. Let shutdownProviderRuntimes resolve to one outcome with shutdown {status:'failed'}, call runtime.shutdown(), and assert that logEvents(stateDir) holds the phase with provider, source and the error code. That test fails at head.

Not blocking: the new ./discovery export in packages/plugins/package.json has no importer of @agent-device/plugins/discovery, so you can drop it until a cross-package consumer exists, or keep it.

Smoke Tests is still running, and no failure is captured yet. The job covers daemon start and stop through createDaemonRuntimeDeps and closeProviders, so a failure there would likely relate to this PR. Before merge, fix the logging above, land #3748 through #3773 (it is not in this PR, and an unclaimed provider such as testmu still passes assertProviderRuntimeAvailable in lease.ts:340-346), rebase, and get Smoke Tests green.

I ran no tests, so regression validity is judged by reading. The real-key Limrun run was on 2dd219b, before this rework. Shutdown now calls the real runtime directly, and no live run covers that at 5ccff7f.

On the open threads, one P2 still applies: test harness owner inference is wrong, so use this link instead: #3767 (comment). It is test-only and not blocking. No lower-priority threads still apply. These do not apply and you can resolve them: #3767 (comment) (a test now covers one plugin failing to shut down), #3767 (comment) (a test covers the primed store reuse), #3767 (comment) (withPluginConnection returns the primed connection first), #3767 (comment) (the CLI renders the unavailable message when name is undefined), #3767 (comment) (plain errors get the typed reason plugin_load_failed), #3767 (comment) (the untrimmed Limrun fingerprint is intentional and stated in the PR body), #3767 (comment) (a test-only helper whose doc block states its scope), #3767 (comment) (production validates owners through assertProviderRuntime).

thymikee and others added 2 commits October 17, 2026 12:52
Built-in providers, the plugin store and an embedder now register through
one registry in one shape. A plugin that fails to load, validate or set up
is recorded in a daemon inventory and no longer stops the daemon.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…plugins on provider-not-registered

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the feat/plugin-registry-v2 branch from 5ccff7f to b55c985 Compare October 17, 2026 11:50
@thymikee

Copy link
Copy Markdown
Member Author

Pushed b55c985297, rebased onto main (a61556b8fd, with #3773 in). pnpm check:affected --run --head a304bb7f40 passes on the stack top.

Applied from the 10:39Z review

  • Cleanup reaches daemon.log. closeProviders now logs provider_plugin_cleanup_failed through emitDaemonDiagnostic(logPath, …), at the startup-failure catch and at shutdown(). The lock-lost exit deliberately writes to stderr instead, because that daemon does not own daemon.log. New test in daemon-runtime-metadata-ownership.test.ts: shutdownProviderRuntimes resolves one failed outcome and logEvents(stateDir) holds the phase with provider, source and the error code.
  • provider-not-registered names failed plugins (planned now that fix(daemon): refuse leases for providers without a registered runtime #3773 is in). details.failedPlugins lists failed plugins for the requested provider, plus any that failed before their provider id was known (provider absent). Covered in lease.test.ts; CLI help, the plugins docs and the changelog say so. The old providerRuntimeRequiredIds / claimedProviders plumbing is gone, since fix(daemon): refuse leases for providers without a registered runtime #3773 removed its consumer.
  • Test-harness owner inference. I took the "update the comment" option: provider-platform-registrations.ts now says it is test-only owner inference for fixture runtimes not built through the registry, instead of claiming it matches daemon composition.
  • ./discovery export dropped from packages/plugins/package.json.
  • Through composition.shutdown. src/__tests__/provider-device-runtimes.test.ts already registers a store plugin with dispose and asserts composition.shutdown() runs its runtime shutdown and then its dispose; the other composition tests also close through composition.shutdown().
  • The threads the review listed as no longer applying were already resolved (0 unresolved).

Live Limrun run on this head (LIMRUN_API_KEY only, from the main checkout's .env; token redacted): connect limrun --platform ios verified credentials; open com.apple.Preferences served by the Limrun runtime the registry registers; snapshot returned 18 nodes; close reported the instance; disconnect and daemon stop --json finished with providerReleases: { status: "completed", released: [], pending: [] }. Afterwards the first page of iosInstances.list (50 instances) is all terminated.

@thymikee

Copy link
Copy Markdown
Member Author

The PR is ready at b55c985. The fix for the earlier findings (#3767 (comment)) is in, and I found no new problems that need a change.

Not blocking, and you can take or leave these: claimedProviders in https://github.com/callstack/agent-device/blob/b55c985/packages/plugins/src/registry.ts#L87 has no production reader left, so you could drop it from both types and assert the claim rule through the inventory duplicate_provider entries; the lock-lost path in https://github.com/callstack/agent-device/blob/b55c985/packages/daemon/src/server/http/daemon-runtime.ts#L470 writes raw plugin error messages to stderr without redactDiagnosticData, and the daemon.log line it produces should go through the same redaction; and failedPlugins at https://github.com/callstack/agent-device/blob/b55c985/packages/daemon/src/server/http/daemon-runtime.ts#L526 is only covered by the handler-level lease test, so deleting that line keeps every test green, and typing the field as the failed variant of PluginInventoryEntry with one assertion through createRequestHandler or startDaemonRuntime would close the gap.

The cubic-dev-ai P2 thread on provider-platform-registrations.ts no longer applies, because the comment now calls it test-only owner inference and drops the claim about daemon composition, so you can resolve it: #3767 (comment)

I read the code and ran no tests. The Limrun run on b55c985 is your report only, and no run exercises the failed-cleanup or lock-lost stderr paths. I reviewed only the b55c985 commit as the delta, not the upstream rebase content.

Smoke Tests now passes at b55c985, and there are no conflicts.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 17, 2026
@thymikee
thymikee merged commit 78849de into main Oct 17, 2026
23 checks passed
@thymikee
thymikee deleted the feat/plugin-registry-v2 branch October 17, 2026 12:46
thymikee added a commit that referenced this pull request Oct 17, 2026
…ment

* origin/main:
  feat(provider-testmu): publish @agent-device/testmu on plugin contract v2 (#3770)
  docs(plugins): document plugin contract v2, the inventory and ADR 0034 (#3769)
  feat(daemon): daemon status lists the registered provider plugins (#3768)
  feat(plugins): plugin contract v2 and one provider registry (#3767)

# Conflicts:
#	packages/daemon/src/sessions/handlers/lease.ts
#	src/__tests__/cloud-connect-testmu.test.ts
#	src/cli/commands/connection.ts
#	src/cli/commands/daemon.ts
#	src/daemon-client/daemon-client-metadata.ts
#	src/daemon-client/daemon-client.ts
thymikee added a commit that referenced this pull request Oct 17, 2026
…-no-process-globals

* refactor/host-environment:
  feat(provider-testmu): publish @agent-device/testmu on plugin contract v2 (#3770)
  docs(plugins): document plugin contract v2, the inventory and ADR 0034 (#3769)
  feat(daemon): daemon status lists the registered provider plugins (#3768)
  feat(plugins): plugin contract v2 and one provider registry (#3767)

# Conflicts:
#	packages/daemon/src/server/http/daemon-runtime-composition.test.ts
#	packages/daemon/src/server/http/daemon-runtime-metadata-ownership.test.ts
#	packages/daemon/src/server/http/daemon-runtime.ts
#	src/daemon-composition.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants