diff --git a/AGENTS.md b/AGENTS.md index ba1b10ab..ca8ba153 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,12 @@ # Notes for coding agents +## Subsystem design and review + +Before changing a documented subsystem, read its +[design reference](docs/subsystems/README.md) and verify the linked source. +When preparing or reviewing a PR, follow the same page's documentation update, +semantic change description, and review rules. + ## Built-in subscriptions that a plugin serves Some built-in subscriptions are deprecated. Reaching them can break their diff --git a/docs/subsystems/README.md b/docs/subsystems/README.md new file mode 100644 index 00000000..636ada5a --- /dev/null +++ b/docs/subsystems/README.md @@ -0,0 +1,34 @@ +# Subsystem design references + +These pages describe current responsibilities, runtime paths, state, and contracts. Start with the subsystem relevant to your task, then follow its source links to verify the implementation you change. + +| Subsystem | Reference | +| --- | --- | +| Built-in subscriptions moving to plugins | [Provider and plugin ownership](provider-plugins.md) | + +A subsystem can span several packages or repositories. Its reference describes the behavior those parts provide together; it does not need to list every function. + +## Maintain a reference + +Update the owning reference in the same PR when a change alters a documented responsibility, interface, state transition, runtime path, compatibility rule, or failure behavior. An internal refactor that preserves these contracts does not require a prose change. + +Keep each fact in one reference and link to it from agent instructions or other pages. Link to source files and named symbols rather than copying implementation details that change frequently. Explain field meanings and constraints that signatures alone do not express. + +For a new reference, include the responsibilities and boundaries, source of truth, main runtime paths, state transitions, important constraints, and existing verification commands. Include failure and compatibility behavior where relevant. Document current behavior separately from proposed changes; the proposal belongs in its issue or PR. + +## Describe a PR's semantic changes + +For each affected subsystem, explain the observable change with its trigger and before/after behavior. Include changes to interfaces, state, compatibility, and failure behavior when relevant. Link the affected reference, implementation, and verification evidence. State when a refactor preserves the documented behavior. + +For example, a change to plugin adoption should explain which migration state and account condition trigger it, which implementation handles requests before and after adoption, and which test exercises that transition. + +This is an author-provided explanation that a reviewer verifies against the diff and surrounding code. It is not an automatically proven compatibility verdict. + +## Review a change + +1. Identify the affected subsystem and the actual runtime path, including plugin or built-in ownership. +2. Compare the claimed before/after behavior with the base and head implementation. Trace relevant callers and state transitions beyond the changed lines. +3. Check that changed contracts appear in the owning reference and that source links still resolve. +4. Check that the cited tests or runtime evidence exercise the claimed behavior. Report checks that were not run. + +A documentation update alone does not prove the implementation. Existing tests and runtime evidence remain necessary for behavior changes. diff --git a/docs/subsystems/provider-plugins.md b/docs/subsystems/provider-plugins.md new file mode 100644 index 00000000..0fd68748 --- /dev/null +++ b/docs/subsystems/provider-plugins.md @@ -0,0 +1,84 @@ +# Provider and plugin ownership + +Some built-in subscriptions are deprecated and have community plugins that provide the same service. A subscription can still use its built-in implementation or move to its plugin. That ownership determines where sign-in, refresh, model discovery, requests, usage, and errors are handled. + +## Responsibilities and sources of truth + +| Part | Responsibility | Source | +| --- | --- | --- | +| Provider migration | Match subscriptions to plugins, record ownership, transfer accounts, and maintain required plugin versions | [`migrate.go`](../../internal/provider/migrate.go) and `migrate_*.go` in the same directory | +| Provider integration | Expose plugin-backed provider cards, usage, and sign-in | [`plugins.go`](../../internal/provider/plugins.go), [`plugin_usage.go`](../../internal/provider/plugin_usage.go), [`pluginsignin.go`](../../internal/provider/pluginsignin.go) | +| Plugin host | Load plugins and execute their code | [`internal/plugin`](../../internal/plugin), [`host.js`](../../internal/plugin/host.js) | +| Gateway | Dispatch requests through the implementation that owns the subscription | [`internal/gateway`](../../internal/gateway) | +| GUI | Present available subscriptions, migration state, and plugin sign-in | [`app.js`](../../internal/gui/assets/app.js) | +| Community plugin | Implement the moved subscription's upstream behavior | [magpie-community/plugins](https://github.com/magpie-community/plugins), `packages/` | + +The `movers` map assembled in `internal/provider/migrate*.go` is the authoritative subscription-to-plugin mapping. Each mover's `min` is the minimum plugin version required by magpie. The list of moved subscriptions in [AGENTS.md](../../AGENTS.md#built-in-subscriptions-that-a-plugin-serves) is a contributor lookup; confirm the mapping in source when changing it. + +## Ownership and runtime paths + +`provider.Moved(id)` identifies a subscription whose migration state in `migrations.json` is `plugin`. In `/api/providers`, its row exposes `move.state == "plugin"`; the GUI also receives its id in `onPlugins`. + +| Operation | Built-in owns the subscription | Plugin owns the subscription | +| --- | --- | --- | +| Sign-in, refresh, and models | Built-in provider implementation | Community plugin through magpie's plugin integration | +| Upstream requests and translation | Built-in gateway adapter | Plugin fetch through the plugin host | +| Usage and upstream errors | Built-in implementation | Plugin, exposed through magpie's provider integration | +| Migration state and required version | Magpie | Magpie | + +For a moved subscription, its built-in upstream implementation does not run. Plugin accounts use `Account.Agent == "plugin"`; requests reach `plugin.Fetch` through `Provider.Do` and bypass the built-in subscription adapter. Magpie still handles routing, protocol conversion, quota handling, and compatibility adjustments, as well as the plugin host, provider integration, migration code, and GUI plugin paths. + +## How ownership changes + +Migration records use four states: + +| State | Meaning | +| --- | --- | +| `moving` | Migration is in progress or was interrupted. An explicit retry first rolls back the interrupted move. | +| `plugin` | The plugin owns the built-in provider id. This is the only state recognized by `Moved`. | +| `back` | Ownership returned to the built-in. Automatic handover leaves it there. | +| `failed` | Migration failed and the built-in continues serving. Automatic retry waits six hours. | + +The editor's "Move to plugin" action calls `provider.Adopt`. With accounts, adoption delegates to `Move`, which imports and validates accounts and model coverage before committing plugin ownership. With no accounts, adoption verifies the package and the provider it serves, then records plugin ownership without account checks. + +Installing the plugin independently can also hand over ownership through `provider.HandOver`. The package must be enabled, meet the mover's minimum version, and serve the expected provider. After add, update, or upgrade, the GUI calls `HandOver(ctx, false)`, which can adopt a built-in with no accounts. Existing built-in accounts wait for `HandOver(ctx, true)` in gateway maintenance. + +`KeepRetiringMoved` first runs after 20 seconds, then hourly. Each cycle maintains moved plugin versions, calls `MoveRetiring`, then performs automatic handover. `MoveRetiring` moves subscriptions listed in `provider.Retiring` that have built-in accounts, preserving move-back choices and waiting six hours after a failed attempt. That list is currently empty. Handover preserves a plugin already signed in under its separate `-plugin` id and skips existing migration records except a `failed` record old enough to retry. + +`MoveBack` restores built-in ownership and records `back`. Compatible accounts added through the plugin can return too; incompatible accounts can remain with the plugin under its separate id. Removing or disabling a plugin first moves affected built-ins back and stops if that fails. A package installed by the user remains after moving back; a migration-installed package is removed only when no remaining signed-in or moved provider needs it. + +New users do not see an unused deprecated built-in in the Add sheet. `unusedSub` leaves it out when it has no account. Until an installed replacement takes ownership, `replacedSub` makes the Add sheet show the plugin tile. The Plugins page is the installation entry point. + +`keepMovedCurrent` maintains the required plugin version for moved subscriptions. Raising the owning mover's `min` after publishing a plugin fix triggers this update path. Update failures are logged; disabled packages and local-folder installations are not automatically replaced. + +## Constraints for fixes + +Determine ownership before choosing the fix location. A bug affecting a moved subscription usually needs a change in the community plugin. A built-in-only fix does not reach moved users. + +For a plugin fix, publish the corrected community package and raise the owning mover's `min` to that version. Change the built-in too when the fix must reach users who have not moved. + +Keep the migration, host, and upstream behavior separate when investigating failures. A plugin bug can require a community change; a host, provider-card, sign-in orchestration, or migration bug can require a magpie change even for moved users. + +The GUI's plugin integration includes `subOf`, `pluginSubs`, and `startPluginSignIn`. A change to sign-in or provider presentation must follow these paths as well as the built-in paths it affects. + +## Verification + +[`migrate_notice_test.go`](../../internal/provider/migrate_notice_test.go) includes `TestMovedBuiltinsSayTheirPlugin`, which checks that moved built-ins tell contributors which plugin serves them. Migration tests live beside [`migrate.go`](../../internal/provider/migrate.go). + +Gateway parity tests in [`plugin_parity_test.go`](../../internal/gateway/plugin_parity_test.go) and other `plugin_*_test.go` files compare or exercise plugin paths. A built-in test alone does not establish moved-user behavior. Inspect each test's fixture to confirm that it covers the provider and operation being changed. + +Run the relevant package tests with an isolated home directory. Resolve GOPATH and Go's caches before changing HOME, so repeated runs reuse dependencies and downloaded toolchains. The subshell makes temporary files writable and cleans up its home on exit, including when tests fail, and returns the test command's exit status. + +```sh +( + verification_home=$(mktemp -d) || exit + trap 'chmod -R u+w "$verification_home"; rm -rf "$verification_home"' EXIT + verification_gopath=$(go env GOPATH) || exit + verification_modcache=$(go env GOMODCACHE) || exit + verification_buildcache=$(go env GOCACHE) || exit + HOME="$verification_home" GOPATH="$verification_gopath" GOMODCACHE="$verification_modcache" GOCACHE="$verification_buildcache" \ + go test -tags nogui ./internal/provider ./internal/gateway ./internal/plugin +) +``` + +Some migration and gateway plugin tests require Bun on PATH and use local plugin fixtures; they skip when Bun is unavailable. Report any skipped tests and verify the community package separately when its implementation changes.