[SirLouen]

Closes #126

What

• Host.StartEach(ctx) migrates and then starts every plugin on its own, in registration order, going on past a failure or a panic. It answers a StartOutcome with the plugins that started and how long each Start took, and each Failure with its plugin id, its Step, whether it panicked and its error.
• A plugin whose migration failed does not start. A failed Record is reported as StepRecord and that plugin still starts, because its schema already moved.
• Once the context ends, every plugin still waiting is reported with an error matching the context's, and none of them is called.
• StartEach stops no plugin. Host.Stop still reaches every one at shutdown. A failed read of the MigrationStore is StartEach's own error, and no plugin is called.
• The Plugin doc promises that Stop may run more than once and that Start may run again after it. The docs of Start and Migrate ask them to return by the time their context ends, and those of Routes and PublicPaths ask them to work whether or not Start worked.
• A MigratorOf lookup that answers (nil, true) now gets an error naming the plugin. It crashed the host before, in code no tag has shipped.

Why

A downstream application plans to compile up to 200 plugins from many authors into one binary, and one failed plugin should not stop the other 199. Host.Start returns at the first failure and stops the plugins it already started, so it cannot give that application a running site.

Apart from that lookup answer, Host.Start, Host.Migrate and Host.Seed keep their behaviour, signatures and error texts. Migrate and StartEach now share one step, so migrateEvery and the generic callEach it needed are gone.

Panicked reads a private error type with a plain type assertion, not errors.As. A plugin whose Start returns an error wrapping a panic recovered by a host inside it did not panic itself, and a test pins that.

A plugin that ignores its context still holds up the plugins behind it, because the host calls them one after another and holds no time limit. The docs say so.

Testing

• 23 new tests, 43 cases in all, cover both passes with and without a store: registration order across steps, a failure and a panic at each step with its exact text, the context ending during a migration, during a start and before the call, a record after the context ended, the time each Start took under testing/synctest, the caller's context at every step, and Host.Stop reaching every plugin afterwards.
• Every existing test passes unchanged.
• An old and new comparison of Migrate, Seed, Start and Stop over 185,760 scenarios differs only where the (nil, true) lookup used to crash.
• gremlins killed all 48 mutants in the root package.
• go test -race with 100.0% coverage in the root package, lint clean, no function above 10 on cyclop.

Summary by CodeRabbit

• New Features
  • Added per-plugin startup that migrates plugins before starting them, continues after individual failures, and reports which plugins started and where failures occurred.
  • Migration can now run for plugins even when no migration store is configured.
• Documentation
  • Clarified that plugin start and stop methods may be called multiple times, routes and public paths are available regardless of startup outcome, and lifecycle methods should respect their contexts.
• Bug Fixes
  • Improved error reporting when a plugin’s migrator lookup succeeds but provides no migrator.

[coderabbitai]

Walkthrough

The pull request adds Host.StartEach, which migrates and starts plugins individually and returns started-plugin and failure details. It updates migration handling, panic reporting, lifecycle documentation, and tests for startup outcomes and context propagation.

Changes: Plugin lifecycle

Layer / File(s)	Summary
Migration and host operation handling pluginkit/host.go, pluginkit/store_test.go	Host.Migrate uses migrateUnlessCurrent with or without a migration store. A nil migrator returned by a successful lookup now produces an error. Recovered panics use a private panicError type.
StartEach outcomes and execution pluginkit/start_each.go, pluginkit/start_each_test.go	Host.StartEach migrates all plugins before starting them in registration order. It records successful starts and per-plugin failures. Tests cover ordering, failure handling, migration-store behavior, and elapsed start times.
Context handling and lifecycle contracts pluginkit/start_each_context_test.go, pluginkit/seed_test.go, pluginkit/host.go, pluginkit/pluginkit.go, pluginkit/CHANGELOG.md	Tests cover context cancellation and propagation during startup and seeding. Host.Seed iterates seeder plugins. Documentation describes lifecycle behavior, context expectations, and provider availability independent of Start.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Host.StartEach
  participant MigrationStore
  participant MigratorOf
  participant Migrator
  participant Plugin
  Host.StartEach->>MigrationStore: Read plugin versions
  loop Each registered plugin
    Host.StartEach->>MigratorOf: Resolve plugin migrator
    MigratorOf-->>Host.StartEach: Return migrator
    Host.StartEach->>Migrator: Migrate plugin
    Migrator-->>Host.StartEach: Return migration result
    Host.StartEach->>MigrationStore: Record successful migration
  end
  loop Each registered plugin
    Host.StartEach->>Plugin: Start plugin
    Plugin-->>Host.StartEach: Return start result
  end

Loading

Possibly related PRs

• gopherium/framework#123: Adds migration-store contracts used by Host.StartEach and updates existing migration behavior.

Merge Risk: Minimal · up to eebe0

No concrete issue identified in the current change warrants delaying the merge.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 21.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: starting each plugin independently and reporting failures.
Linked Issues check		Issue #126 is the only directly linked issue, and it is open. Its coding requirements are met at the reviewed head, as shown by pluginkit/start_each.go and the pluginkit/host.go diff. Migration befor…
Out of Scope Changes check		The whole-PR diff from merge base 8e39b37 to head eebe072 was checked against the issue. Each change connects to #126 or supports a change that does. - StartEach, Step, Failure, StartedPlugin, StartO…

Full details: Linked Issues check

Explanation

Issue #126 is the only directly linked issue, and it is open. Its coding requirements are met at the reviewed head, as shown by pluginkit/start_each.go and the pluginkit/host.go diff.

Migration before start: StartEach calls migrateUnlessEnded for every plugin in one loop, then calls startAfter for every plugin in a second loop. No plugin starts before all migrations finish.

Migration failure blocks start: startAfter returns before the start call when migration.Err is non-nil and migration.Step is not StepRecord. A StepRecord failure is listed in Failed and the plugin still starts.

Continuation and panics: each plugin runs through safeCall. failureOf sets Panicked only when the error is a panicError value. guarded returns a panic as panicError and wraps ordinary errors with fmt.Errorf and %w, so a Start error that wraps a panic is not marked as a panic.

Context end: migrateUnlessEnded and the start step check context.Context.Err before each call. A waiting plugin is reported with the context error (wrapped with %w) and is not called.

Store read: readVersions runs before any plugin call. A read error is returned as the StartEach error and no plugin is called.

Stop: StartEach has no stop call. Host.Stop is unchanged and calls every plugin.

Start durations: StartedPlugin.Took is measured from before the start call to after it.

Host.Start: its body is unchanged in the diff. Its migration step keeps the same safeCall operation names and error texts.

Limits of this review: I did not read the test files or the pluginkit.go documentation directly. The documentation requirement (Stop may repeat, Start may run again) is documentary, and the StartEach code does not depend on it. I rely on the summaries for the test presence and the doc text. These gaps do not change the result for the coding behavior.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff from merge base 8e39b37 to head eebe072 was checked against the issue. Each change connects to #126 or supports a change that does.

• StartEach, Step, Failure, StartedPlugin, StartOutcome (start_each.go): the new feature.
• readVersions, migrateUnlessCurrent returning a Step, and Migrate sharing that step (host.go): needed so StartEach and Migrate use one migration path. Migrate keeps its signature and error texts.
• migrateEvery and callEach removed; Seed inlined (host.go): refactoring that removes the generic helper after Migrate and Seed stop needing it. Seed still calls Seeder.Seed with the operation name seed.
• panicError type in guarded (host.go): needed so Failure.Panicked can use a plain type assertion.
• errNilMigrator in migratorFor (host.go): a fix for a (nil, true) lookup that crashed the shared migration path that StartEach now uses. The PR discloses it as a behavior change. It is connected to the shared step.
• Plugin, Migrator, RouteProvider and PublicPathProvider documentation (pluginkit.go): documents the lifecycle promises that #126 asks for.
• CHANGELOG.md: records the same behavior.
• Tests (start_each_test.go, start_each_context_test.go, seed_test.go, store_test.go): cover the new behavior, the refactored paths and the nil lookup fix.

I found no product feature or unrelated code change. The test files and documentation were judged from the summaries, not from direct reads, so this result is limited to the code paths I read.

Full details: Docstring Coverage

Explanation

Docstring coverage is 21.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 19 functions across 7 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Adds Host.StartEach so plugins can start despite another plugin failing.

• The host starts eligible plugins and reports each result separately.
• A lookup that claims a migrator but returns none gets a named error.
• Plugin docs spell out repeat calls and behavior when startup fails.
• Seeding keeps its order and caller context with a direct loop.

_{Reviews (2) · Last reviewed commit: "docs(pluginkit): list the Migrate, Route..." · Reviewed by Greptile}

[coderabbitai]

Additional comments: 1

pluginkit/CHANGELOG.md-11-12: 1

11-12: Trivial · Maintainability & Code Quality · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Add a changelog entry for the (nil, true) lookup change.

migratorFor in pluginkit/host.go now returns errNilMigrator when a MigratorOf lookup reports ok with a nil Migrator. This changes the error that Migrate, Start and StartEach report for that lookup. The Unreleased section does not mention it. It also does not mention the new Routes and PublicPaths documentation. Add one line so consumers can find the changed error.

📝 Proposed changelog line

 - The `Plugin` doc promises that `Stop` may run more than once and that `Start` may run again after it, and asks `Start` to return by the time its context ends.
+- A `MigratorOf` lookup that answers a nil `Migrator` with `true` now fails the migration with an error naming the plugin.

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 60ad75cb-4c48-4f4a-b338-5203ff43cdb3

Commits

Reviewing files that changed from the base of the PR and between 8e39b37 and 22a9543.

Files selected for processing: 8

• pluginkit/CHANGELOG.md
• pluginkit/host.go
• pluginkit/pluginkit.go
• pluginkit/seed_test.go
• pluginkit/start_each.go
• pluginkit/start_each_context_test.go
• pluginkit/start_each_test.go
• pluginkit/store_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[codescene-access]

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

[coderabbitai]

Additional comments: 1

pluginkit/CHANGELOG.md-11-13: 1

11-13: Trivial · Maintainability & Code Quality · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Add a changelog entry for the nil-migrator behavior change.

The PR reports that a MigratorOf result of (nil, true) now returns an error naming the plugin. This change affects Host.Migrate, Host.Start and Host.StartEach. The Unreleased section does not mention it. Callers that upgrade will see a new error path and a changed contract for MigratorOf.

Add one line to the Unreleased section.

📝 Proposed changelog line

 - `Host.StartEach` migrates and then starts every plugin on its own, goes on past a failure or a panic, and answers a `StartOutcome` naming each plugin that started and each `Failure`.
+- A `MigratorOf` lookup that answers `(nil, true)` now makes the host return an error naming the plugin, instead of using a nil `Migrator`.

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 084ffcf5-5aea-4e03-a42b-496bc03993f6

Commits

Reviewing files that changed from the base of the PR and between 8e39b37 and eebe072.

Files selected for processing: 8

• pluginkit/CHANGELOG.md
• pluginkit/host.go
• pluginkit/pluginkit.go
• pluginkit/seed_test.go
• pluginkit/start_each.go
• pluginkit/start_each_context_test.go
• pluginkit/start_each_test.go
• pluginkit/store_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.