Skip to content

Add the capability plugin SPI and move server-time onto it - #104

Merged
codegeek merged 3 commits into
mainfrom
014-capability-plugin-spi
Oct 23, 2026
Merged

codegeek merged 3 commits into
mainfrom
014-capability-plugin-spi

Conversation

@codegeek

@codegeek codegeek commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Capability modules are mostly markers: their behaviour lives in jircd-core, keyed by CapabilityName constants, because the capability SPI only offers contributeTags and includeSenderInFanOut. Feature 014 (#98) gives modules a real plugin SPI. This is the first of four PRs: it adds the foundation and moves server-time onto it, so the SPI is proven end to end before the other modules migrate in #101.

Changes

Migration (unreleased 0.1.0-SNAPSHOT)

Testing

  • ./gradlew build: green, 1125 tests. The existing jircd-integration-tests sources are unchanged and pass.
  • Coverage: 94.95% line / 87.30% branch (JaCoCo), against main's 94.7% / 87.5% on SonarCloud.
  • Local SonarCloud branch analysis: quality gate passed, 0 open issues.
  • ./gradlew loadTest on this branch and on main (5912b68), with repeated interleaved runs: all 8 load tests pass, and the differences are within run-to-run noise. Details in specs/014-capability-plugin-spi/validation/pr-100/.
  • Independent review by Codex over three rounds; every finding is fixed (validation/pr-100/review.md).
  • irctest not run for this PR; the comparison run comes with [Feature] Move message-tags, echo-message, away-notify, multi-prefix and batch onto the capability SPI #101.

Fixes #100

🤖 Generated with Claude Code

Summary by Sourcery

Establish the capability plugin SPI and migrate server-time to validated outbound-tag hooks while preserving legacy providers for subsequent migrations.

New Features:

  • Introduce a typed capability-provider SPI covering events, tags, recipients, reply decorations, output groups, and provider commands.
  • Add immutable runtime snapshots, lifecycle-managed hook publication, dependency validation, command ownership, and defensive hook dispatch with safe fallbacks.
  • Add bounded, all-or-nothing prepared frame delivery and generic reply/tag budget validation.
  • Migrate server-time to the outbound-tag registration hook while preserving its wire output.

Bug Fixes:

  • Contain provider exceptions, invalid results, oversized contributions, and cleanup failures without disrupting core delivery or unrelated providers.
  • Prevent command replacement and ensure provider-installed handlers are removed safely during disable and rollback.
  • Ensure legacy and typed tag contributions are validated and budgeted at enqueue time so invalid contributions do not drop the underlying message.

Enhancements:

  • Extend server context with capability services and strengthen extension lifecycle cleanup, retry, and reentrancy behavior.
  • Add generic reply layouts and contribution catalogs for consistent startup, reload, and enable-time size validation.

Documentation:

  • Document capability-provider registration, hook behavior, lifecycle guarantees, bounds, migration status, and temporary legacy compatibility in the README and API documentation.
  • Add the Feature 014 capability-plugin SPI specification, contracts, implementation plan, tasks, and validation records.

Tests:

  • Add extensive coverage for registration validation, hook dispatch containment, lifecycle publication and cleanup, reply budgets, prepared delivery, command ownership, tag composition, and the server-time migration.

codegeek and others added 2 commits October 22, 2026 16:22
Capability modules are mostly markers today: their behaviour lives in
core, keyed by CapabilityName constants, and disabling one at runtime
leaves it active for sessions that negotiated it. This spec designs a
registration SPI so each module owns its behaviour, and one lifecycle
for enable, disable and plugin failure (prune negotiated sets, CAP
DEL/NEW, CAP REQ :-cap, cap-notify advertised in CAP LS).

Delivery is split into four PRs: #100 (SPI foundation, server-time),
#101 (remaining migrations), #97 (runtime lifecycle) and #102 (fault
isolation and load measurement).

Refs #98

Co-Authored-By: GPT-6.1-Sol (Codex) <noreply@openai.com>
Capability modules were mostly markers: core held their behaviour,
keyed by CapabilityName constants, and the only SPI hooks were
contributeTags and includeSenderInFanOut. This adds the foundation from
spec 014 so modules can own their behaviour, and proves it end to end
with server-time.

- CapabilityRegistrar with typed hooks (events, inbound and outbound
  tags, recipients, member status, NAMES identity, reply fields, output
  groups, commands) and CapabilityDescriptor dependencies.
- Hooks are published in an immutable RuntimeSnapshot and run through
  HookDispatcher, which contains a throwing or invalid hook, reports it
  by provider, hook and category, and falls back to the default.
- Enable publishes hooks and commands transactionally and rolls back on
  any failure; disable removes exactly the handlers the provider
  installed and runs every cleanup step even if one fails. Providers
  can't replace core commands, and duplicate provider ids are rejected.
- FrameSequence/PreparedDelivery admit validated frames all-or-nothing.
  Frames are checked for IRC syntax and the per-origin tag budgets, and
  tag contributions that would overflow are dropped instead of the
  message.
- NAMES/WHO reply budgets go through ReplyLayout/ReplyBudgetCatalog,
  evaluated against the configured limits.
- server-time uses the outbound-tag hook; its wire output is unchanged.
  The other modules move in #101.

API changes (unreleased 0.1.0-SNAPSHOT): ServerContext gains capability
services, hook types moved to the Hooks interface, and
CommandRegistrar gains abstract isRegistered/unregister.

Fixes #100

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

This PR establishes a typed, validated capability-provider SPI with immutable runtime publication, defensive hook dispatch, transactional extension and command lifecycle handling, generic reply/tag/frame budgeting, and an end-to-end migration of server-time to the new outbound-tag hook while leaving the remaining capability migrations and runtime fault withdrawal for follow-up PRs.

Sequence diagram for transactional capability enablement

sequenceDiagram
    participant Registry as ExtensionRegistry
    participant Provider as CapabilityExtension
    participant Registrar as CapabilityRegistrar
    participant Runtime as CapabilityRuntime
    participant Commands as CommandRegistrar

    Registry->>Provider: start(ServerContext)
    Registry->>Provider: register(CapabilityRegistrar)
    Provider->>Registrar: onOutboundTags(...)
    Registrar-->>Registry: build()
    Registry->>Runtime: publish(CapabilityDescriptor)
    Registry->>Commands: register(command, handler)
    alt publication or command installation fails
        Registry->>Commands: unregister(command, installed)
        Registry->>Runtime: withdraw(providerId)
        Registry->>Provider: stop()
    else enable succeeds
        Registry-->>Registry: state = ENABLED
    end
Loading

Sequence diagram for server-time outbound tag delivery

sequenceDiagram
    participant Message as OutboundMessage
    participant Renderer as CapabilityTagRenderer
    participant Snapshot as RuntimeSnapshot
    participant Dispatcher as HookDispatcher
    participant ServerTime as server-time
    participant Writer as SessionWriter

    Renderer->>Snapshot: hooksFor(OUTBOUND_TAGS, negotiated)
    Renderer->>Message: contribution(hook, dispatcher)
    Message->>Dispatcher: outboundTags(registration, message)
    Dispatcher->>ServerTime: timeTag(message)
    ServerTime-->>Dispatcher: time tag map
    Dispatcher-->>Message: validated tags
    Message-->>Renderer: memoized contribution
    Renderer->>Writer: enqueue outbound message
Loading

Flow diagram for defensive capability hook dispatch

flowchart TD
    Invoke[Invoke registered hook] --> Result{Result valid?}
    Result -->|Yes| Apply[Apply hook result]
    Result -->|No| Report["FaultReporter.report(providerId, hook, category)"]
    Report --> Default[Use documented default]
    Exception[Hook throws] --> Report
    Invoke --> Exception
Loading

Flow diagram for reply budget validation

flowchart LR
    Providers[Discovered providers] --> Catalog[ReplyBudgetCatalog]
    Config[Startup or reload configuration] --> Limits[ReplyLimits]
    Catalog --> Layout[ReplyLayout]
    Limits --> Layout
    Layout --> Check[HostmaskReplyBudget.requireFits]
    Check -->|Fits| Accept[Accept configuration or enable]
    Check -->|Exceeds 512 bytes| Reject[Reject configuration or enable]
Loading

File-Level Changes

Change Details Files
Introduces the typed capability plugin SPI and its immutable, defensive runtime model.
  • Adds capability descriptors, staged registrations, hook types, dependencies, ownership checks, negotiation prerequisites, and reply/tag bounds.
  • Indexes published hooks in immutable runtime snapshots and dispatches every result with validation, fallback, fault categorization, and containment.
  • Adds provider-owned command installation with collision prevention, transactional rollback, identity-aware removal, and lifecycle cleanup semantics.
  • Exposes capability services through ServerContext and documents the transitional API and migration path.
jircd-core/src/main/java/net/jircd/core/extension/capability/Audience.java
jircd-core/src/main/java/net/jircd/core/extension/capability/CapabilityDescriptor.java
jircd-core/src/main/java/net/jircd/core/extension/capability/CapabilityRegistrar.java
jircd-core/src/main/java/net/jircd/core/extension/capability/CapabilityRuntime.java
jircd-core/src/main/java/net/jircd/core/extension/capability/CapabilityServices.java
jircd-core/src/main/java/net/jircd/core/extension/capability/CommittedEvent.java
jircd-core/src/main/java/net/jircd/core/extension/capability/DeliveryIntent.java
jircd-core/src/main/java/net/jircd/core/extension/capability/DeliveryRole.java
jircd-core/src/main/java/net/jircd/core/extension/capability/EventIntent.java
jircd-core/src/main/java/net/jircd/core/extension/capability/EventKind.java
jircd-core/src/main/java/net/jircd/core/extension/capability/FaultCategory.java
jircd-core/src/main/java/net/jircd/core/extension/capability/FaultReporter.java
jircd-core/src/main/java/net/jircd/core/extension/capability/HookDispatcher.java
jircd-core/src/main/java/net/jircd/core/extension/capability/HookKind.java
jircd-core/src/main/java/net/jircd/core/extension/capability/HookRegistration.java
jircd-core/src/main/java/net/jircd/core/extension/capability/Hooks.java
jircd-core/src/main/java/net/jircd/core/extension/capability/InboundTagResult.java
jircd-core/src/main/java/net/jircd/core/extension/capability/InboundTags.java
jircd-core/src/main/java/net/jircd/core/extension/capability/MemberSnapshot.java
jircd-core/src/main/java/net/jircd/core/extension/capability/OutputGroup.java
jircd-core/src/main/java/net/jircd/core/extension/capability/Registration.java
jircd-core/src/main/java/net/jircd/core/extension/capability/ReplyBudgetCatalog.java
jircd-core/src/main/java/net/jircd/core/extension/capability/ReplyLayout.java
jircd-core/src/main/java/net/jircd/core/extension/capability/ReplyLimits.java
jircd-core/src/main/java/net/jircd/core/extension/capability/ReplySlot.java
jircd-core/src/main/java/net/jircd/core/extension/capability/RequestContext.java
jircd-core/src/main/java/net/jircd/core/extension/capability/RuntimeSnapshot.java
jircd-core/src/main/java/net/jircd/core/extension/capability/Tokens.java
jircd-core/src/main/java/net/jircd/core/extension/capability/package-info.java
jircd-core/src/main/java/net/jircd/core/extension/CapabilityExtension.java
jircd-core/src/main/java/net/jircd/core/extension/AbstractCapabilityExtension.java
jircd-core/src/main/java/net/jircd/core/extension/CommandRegistrar.java
jircd-core/src/main/java/net/jircd/core/extension/Extension.java
jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java
jircd-core/src/main/java/net/jircd/core/extension/ServerContext.java
jircd-core/src/main/java/net/jircd/core/session/ConnectionHandler.java
jircd-server/src/main/java/net/jircd/server/JircdServerApplication.java
README.md
Adds bounded, immutable output preparation and exact wire validation for plugin-produced frames and tags.
  • Materializes frame sequences before queue admission and enqueues them atomically as one SendQ entry.
  • Validates command/prefix/parameter syntax, NUL handling, tag keys, escaping, per-origin tag budgets, and exact serialized byte counts.
  • Adds prepared-delivery lifecycle data and writer support that prevents partial group admission and drain-time provider callbacks.
  • Updates warning throttling and outbound contribution memoization for safe, shared envelope-level results.
jircd-core/src/main/java/net/jircd/core/session/FrameSequence.java
jircd-core/src/main/java/net/jircd/core/session/FrameSyntax.java
jircd-core/src/main/java/net/jircd/core/session/PreparedDelivery.java
jircd-core/src/main/java/net/jircd/core/session/SessionWriter.java
jircd-core/src/main/java/net/jircd/core/session/OutboundMessage.java
jircd-core/src/main/java/net/jircd/core/session/CapabilityTagRenderer.java
jircd-core/src/main/java/net/jircd/core/session/WarningThrottle.java
Generalizes NAMES/WHO reply budget calculation to account for installed provider decorations.
  • Introduces generic layouts, reply slots, contribution catalogs, and validated configured reply limits.
  • Applies the same discovered-provider budget validation during startup, reload, and runtime enable.
  • Preserves the legacy two-status envelope while allowing future status and identity providers to compose safely.
jircd-core/src/main/java/net/jircd/core/config/ConfigurationLoader.java
jircd-core/src/main/java/net/jircd/core/config/ConfigurationReloader.java
jircd-core/src/main/java/net/jircd/core/config/HostmaskReplyBudget.java
jircd-core/src/main/java/net/jircd/core/extension/capability/ReplyBudgetCatalog.java
jircd-core/src/main/java/net/jircd/core/extension/capability/ReplyLayout.java
jircd-core/src/main/java/net/jircd/core/extension/capability/ReplyContribution.java
Migrates server-time to the outbound-tag SPI while retaining wire-compatible behavior.
  • Registers ownership of the time tag with a declared 29-byte bound.
  • Generates the timestamp solely from the immutable envelope send time.
  • Replaces the provider-specific purity test with coverage for registration, immutability, negotiation, fault containment, and single-path behavior.
jircd-capabilities/server-time/src/main/java/net/jircd/capabilities/servertime/ServerTimeExtension.java
jircd-capabilities/server-time/src/test/java/net/jircd/capabilities/servertime/ServerTimeExtensionTest.java
jircd-capabilities/server-time/src/test/java/net/jircd/capabilities/servertime/ServerTimeExtensionPurityTest.java
Adds migration documentation, feature contracts, and extensive validation for the staged SPI foundation.
  • Documents provider authoring, transitional legacy hooks, ownership, lifecycle, bounds, and future migration work.
  • Adds specification, contracts, implementation plan, task breakdown, research, and validation records.
  • Adds focused tests for registration, dispatch, extension lifecycle, budgets, frame admission, command ownership, tag migration, and server-time.
specs/014-capability-plugin-spi/spec.md
specs/014-capability-plugin-spi/plan.md
specs/014-capability-plugin-spi/tasks.md
specs/014-capability-plugin-spi/research.md
specs/014-capability-plugin-spi/data-model.md
specs/014-capability-plugin-spi/quickstart.md
specs/014-capability-plugin-spi/contracts/capability-spi.md
specs/014-capability-plugin-spi/contracts/runtime-lifecycle.md
specs/014-capability-plugin-spi/contracts/reply-budgets.md
specs/014-capability-plugin-spi/checklists/requirements.md
specs/014-capability-plugin-spi/validation/pr-100/baseline.md
specs/014-capability-plugin-spi/validation/pr-100/integration-fixtures.md
specs/014-capability-plugin-spi/validation/pr-100/irctest-environment.md
specs/014-capability-plugin-spi/validation/pr-100/results.md
specs/014-capability-plugin-spi/validation/pr-100/review.md
jircd-core/src/test/java/net/jircd/core/extension/capability/CapabilityRegistrarTest.java
jircd-core/src/test/java/net/jircd/core/extension/capability/CapabilityDispatchTest.java
jircd-core/src/test/java/net/jircd/core/extension/ExtensionRegistryCapabilityTest.java
jircd-core/src/test/java/net/jircd/core/config/CapabilityReplyBudgetTest.java
jircd-core/src/test/java/net/jircd/core/session/PreparedDeliveryTest.java
jircd-core/src/test/java/net/jircd/core/session/CapabilityTagRendererMigrationTest.java
jircd-core/src/test/java/net/jircd/core/session/ConnectionHandlerCommandTableTest.java
jircd-core/src/test/java/net/jircd/core/session/PreparedDeliveryTest.java
jircd-capabilities/server-time/src/test/java/net/jircd/capabilities/servertime/ServerTimeExtensionTest.java

Assessment against linked issues

Issue Objective Addressed Explanation
#100 Provide the capability SPI with typed registrations and dependencies, generic dispatch and failure containment for each hook type, and reply-budget accounting for size-changing hooks. ✅
#100 Move server-time tag behavior from the legacy contribution hook to the new outbound-tag registration, keeping the wire output unchanged and the behavior owned by its module. ✅
#100 Deliver the feature 014 specification documentation and tests for registration types and hook failure isolation, while preserving existing integration-test behavior. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai 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.

Hey - I've found 15 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java" line_range="384" />
<code_context>
+    capabilityRuntime.withdraw(id);
+    Throwable failure = null;
+    try {
+      removeCommands(installedCommands.getOrDefault(id, List.of()));
+    } catch (RuntimeException | LinkageError e) {
+      failure = e;
</code_context>
<issue_to_address>
**Failed commands remain callable**

When the command registrar throws while unregistering a provider handler, `disable` and rollback discard the installed-command records even when `removeCommands` fails, while the guarded handler has no withdrawn-state check. The handler remains callable after the provider stops, and the registry cannot retry removing it; clients can still invoke the provider and re-enable can fail.

Retain failed-removal records for retry and make guarded handlers reject calls after withdrawal.

Also at `jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:280`, `jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:282`, `jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:388-390`, `jircd-core/src/main/java/net/jircd/core/extension/capability/HookDispatcher.java:376-377`.
</issue_to_address>

### Comment 2
<location path="jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java" line_range="267" />
<code_context>
+    validateReplyBudget(next);
+    List<HookRegistration> commands = commandsOf(descriptor);
+    requireUnownedCommands(commands);
+    capabilityRuntime.publish(descriptor);
+    boolean installed = false;
+    List<InstalledCommand> done = new ArrayList<>();
</code_context>
<issue_to_address>
**Failed enable still affects clients**

When command installation fails after hooks are published, while another thread captures or uses the published hooks, `capabilityRuntime.publish` exposes hooks before `installCommands` completes. Captured snapshots remain usable after rollback withdraws the descriptor, so hooks from an enable that fails still affect output or run provider code.

Make hook publication visible only after command installation succeeds, so a failed enable cannot expose hooks.

Also at `jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:311`.
</issue_to_address>

### Comment 3
<location path="jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java" line_range="244" />
<code_context>
     }
     try {
       entry.extension.start(serverContext);
+      if (entry.extension instanceof CapabilityExtension capability) publish(capability);
       entry.state = Extension.State.ENABLED;
       if (point != null) {
</code_context>
<issue_to_address>
**Provider callbacks block lifecycle**

When a provider callback blocks or waits for another thread that needs the registry monitor, `enable` and `disable` hold the registry monitor while running lifecycle callbacks, including `start`, `register`, and `stop`. The callback blocks other synchronized lifecycle operations, and waiting on one deadlocks lifecycle changes.

Run provider callbacks without holding the registry monitor while preserving lifecycle-operation coordination.

Also at `jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:390`.
</issue_to_address>

### Comment 4
<location path="jircd-core/src/main/java/net/jircd/core/session/CapabilityTagRenderer.java" line_range="91" />
<code_context>
+    List<CapabilityExtension> selected = new ArrayList<>();
+    for (var extension : extensionRegistry.enabled()) {
+      if (extension instanceof CapabilityExtension capability
+          && !snapshot.hasHooks(capability.id())
+          && negotiated.contains(capability.providedCapability().name())) {
+        selected.add(capability);
</code_context>
<issue_to_address>
**Legacy tags use stale lifecycle state**

When a provider is enabled or disabled between snapshot capture and the live enabled-provider scan, `CapabilityTagRenderer.prepare` captures a `RuntimeSnapshot`, but `legacySelection` separately reads `ExtensionRegistry.enabled()`. The two views can disagree, so a withdrawn provider can contribute tags after withdrawal or legacy tags can be included or omitted inconsistently with the captured view.

Select legacy providers from lifecycle state consistent with the captured runtime snapshot.

Also at `jircd-core/src/main/java/net/jircd/core/session/CapabilityTagRenderer.java:52-89`.
</issue_to_address>

### Comment 5
<location path="jircd-core/src/main/java/net/jircd/core/extension/capability/HookDispatcher.java" line_range="163-164" />
<code_context>
+            registration,
+            () -> registration.hook(Hooks.OutboundTagHook.class).apply(message),
+            tags -> validTags(tags, registration))
+        .map(Map::copyOf)
+        .orElse(Map.of());
+  }
+
</code_context>
<issue_to_address>
**Mutable hook results bypass validation**

When a hook returns a mutable result container that another thread changes after validation, `HookDispatcher` validates provider-owned containers and then copies or consumes the originals. A concurrent mutation can introduce invalid tags, recipients, or roles after validation, or make copying throw outside hook containment, so invalid results reach callers or escape preparation.

Copy each provider result before validating it, then consume only the validated copy.

Also at `jircd-core/src/main/java/net/jircd/core/extension/capability/HookDispatcher.java:72`, `jircd-core/src/main/java/net/jircd/core/extension/capability/HookDispatcher.java:149`.
</issue_to_address>

### Comment 6
<location path="jircd-core/src/main/java/net/jircd/core/config/ConfigurationLoader.java" line_range="82" />
<code_context>
+        serverName,
+        nicknameMaxLength,
+        channelNameMaxLength,
+        ReplyBudgetCatalog.legacyCompatible().plus(discoveredReplyCatalog.get()));
   }

</code_context>
<issue_to_address>
**Invalid bounds escape configuration validation**

When a provider’s reply bound is valid at reference limits but fails or exceeds 512 at the configured limits, `checkReplyBudgets` evaluates each installed provider’s bound against the actual configuration. If a bound is valid at staging’s reference limits but throws or exceeds 512 for these limits, `ReplyContribution.evaluate` throws `IllegalArgumentException`, which escapes `ConfigurationLoader.load` and reload instead of the declared `ConfigurationException` validation path.

Translate bound-evaluation failures into an actionable `ConfigurationException` during startup/reload validation, while retaining provider fault reporting for invalid declarations.
</issue_to_address>

### Comment 7
<location path="jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java" line_range="249" />
<code_context>
         extensionPointOwners.put(point, id);
       }
-    } catch (RuntimeException e) {
+    } catch (RuntimeException | LinkageError e) {
+      // A start that threw may have acquired resources first; stop must tolerate that.
+      stopQuietly(entry.extension);
</code_context>
<issue_to_address>
**REHASH aborts on linkage failures**

When a provider throws a `LinkageError` while being enabled during REHASH, `ExtensionRegistry.enable` marks the provider failed and rethrows a `LinkageError`, but `ConfigurationReloader.reconcileSection` catches only `RuntimeException`. The error aborts the reload before `current` is updated and can leave earlier reconciliation changes applied, so a valid REHASH partially takes effect.

Handle `LinkageError` as an isolated provider failure during reconciliation so the reload can finish applying unrelated valid changes and publish the configuration.
</issue_to_address>

### Comment 8
<location path="jircd-core/src/main/java/net/jircd/core/session/OutboundMessage.java" line_range="131" />
<code_context>
-        capability, key -> WireBytes.tags(key.contributeTags(this)));
+        capability,
+        key -> {
+          Map<String, String> tags = Map.copyOf(key.contributeTags(this));
+          return new Contribution(tags, WireBytes.tags(tags));
+        });
</code_context>
<issue_to_address>
**Legacy tag failure aborts delivery**

When an unmigrated provider's `contributeTags` throws or returns null, `legacyContribution` calls the unmigrated provider directly and `Map.copyOf` propagates its exception or null-result failure; `CapabilityTagRenderer.prepare` does not contain that failure. A faulty legacy provider therefore aborts message preparation instead of dropping its tags and allowing the message through.

Apply equivalent exception/null containment to the temporary legacy path, report the provider fault, and continue with an empty contribution.
</issue_to_address>

### Comment 9
<location path="jircd-core/src/main/java/net/jircd/core/extension/capability/HookDispatcher.java" line_range="176" />
<code_context>
+        return false;
+      }
+    }
+    return MessageSerializer.tagDataBytes(tags) <= registration.maxBytes();
+  }
+
</code_context>
<issue_to_address>
**Oversized tags allocate before rejection**

When a faulty provider returns an exceptionally large tag value or group tag map, `MessageSerializer.tagDataBytes` constructs the escaped tag section and UTF-8 byte array before comparing the result with the declared or per-origin limit. A faulty hook or group containing a very large tag value therefore causes memory use proportional to the invalid result, and concurrent validations can exhaust server memory before the bound rejects it.

Count escaped UTF-8 tag bytes incrementally with an early limit check instead of materializing the serialized section to validate it.

Also at `jircd-core/src/main/java/net/jircd/core/session/FrameSyntax.java:63`.
</issue_to_address>

### Comment 10
<location path="jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java" line_range="249" />
<code_context>
         extensionPointOwners.put(point, id);
       }
-    } catch (RuntimeException e) {
+    } catch (RuntimeException | LinkageError e) {
+      // A start that threw may have acquired resources first; stop must tolerate that.
+      stopQuietly(entry.extension);
</code_context>
<issue_to_address>
**Provider errors skip cleanup**

When `start` or staging throws a non-linkage, non-VM-fatal `Error` after acquiring resources, `ExtensionRegistry.enable` catches `RuntimeException` and `LinkageError`, so another `Error` bypasses `stopQuietly` and the FAILED-state update. The provider remains recorded as DISABLED with its resources unreleased.

Handle non-VM-fatal `Error` values in the enable cleanup path, then rethrow after stopping the provider and recording FAILED.
</issue_to_address>

### Comment 11
<location path="jircd-core/src/main/java/net/jircd/core/session/CapabilityTagRenderer.java" line_range="56-57" />
<code_context>
+    List<CapabilityExtension> legacy = legacySelection(snapshot, negotiated);
+    Map<String, String> composed = new LinkedHashMap<>();
+    for (CapabilityExtension capability : legacy) {
+      composed.putAll(message.legacyTags(capability));
+      bytes = bytes.plus(message.contributionBytes(capability));
+    }
+    List<Map<String, String>> typed = new ArrayList<>();
</code_context>
<issue_to_address>
**Oversized legacy tags drop messages**

When a still-supported legacy provider returns server tags whose aggregate exceeds the origin budget, a legacy provider's tag map is counted for queue reservation but is not checked against the per-origin limit until `renderSelected`. That check throws `MessageTooLongException`, and `SessionWriter.writeFanOutMessage` returns without writing the message, so an oversized legacy contribution drops the core message instead of only the contribution.

Validate and omit an oversized legacy contribution during preparation, reporting it without preventing the message from being written.
</issue_to_address>

### Comment 12
<location path="jircd-core/src/main/java/net/jircd/core/extension/capability/CapabilityRegistrar.java" line_range="79-91" />
<code_context>
+  /** Declares hard availability dependencies on other providers; self, duplicates rejected. */
+  public synchronized void requiresProviders(String... providerIds) {
+    requireOpen();
+    for (String id : providerIds) {
+      Tokens.require(id, "required provider id");
+      if (id.equals(providerId)) {
+        throw new IllegalArgumentException("provider '" + id + "' cannot require itself");
+      }
+      if (requiredProviders.contains(id)) {
+        throw new IllegalArgumentException("duplicate required provider '" + id + "'");
+      }
+      if (requiredProviders.size() >= MAX_REQUIRED_PROVIDERS) {
+        throw new IllegalStateException(
+            "more than " + MAX_REQUIRED_PROVIDERS + " required providers");
+      }
+      requiredProviders.add(id);
+    }
+  }
</code_context>
<issue_to_address>
**Rejected dependencies remain staged**

When a multi-id `requiresProviders` call encounters an invalid, duplicate, self, or over-limit id and the provider catches the exception, `CapabilityRegistrar.requiresProviders` adds ids to `requiredProviders` as it validates them. The failed call leaves its valid prefix in the descriptor, so the provider can be enabled or later withdrawn with dependencies the caller thought were rejected.

Validate the entire id list before adding any ids to `requiredProviders`.

Also at `jircd-core/src/main/java/net/jircd/core/extension/capability/CapabilityRegistrar.java:92`.
</issue_to_address>

### Comment 13
<location path="jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java" line_range="395" />
<code_context>
+      // Already withdrawn: it stays withdrawn, recorded as failed rather than half-enabled.
+      failure = accumulate(failure, e);
+    }
+    entry.state = failure == null ? Extension.State.DISABLED : Extension.State.FAILED;
     String point = extensionPoint(entry.extension);
     if (point != null) {
</code_context>
<issue_to_address>
**Failed stop frees exclusive extension point**

When `stop()` fails before releasing a resource associated with the extension point, `disable` marks the provider FAILED but still removes its extension-point claim. Another provider can then enable with the same claim while the failed provider's `stop()` left its resource or behavior active, so both providers can own the supposedly exclusive point.

Keep the claim reserved until cleanup succeeds, or otherwise prevent another provider from acquiring it while the previous provider's cleanup is unresolved.
</issue_to_address>

### Comment 14
<location path="jircd-core/src/main/java/net/jircd/core/session/ConnectionHandler.java" line_range="309" />
<code_context>
+
+      @Override
+      public void unregister(Command command, CommandHandler installed) {
+        handlers.remove(command, installed);
+      }
+    };
</code_context>
<issue_to_address>
**Replacement command handler removed**

When a later command handler for the same command is a distinct instance whose `equals` returns true for the provider’s installed handler, `handlers.remove(command, installed)` compares values using `equals`, not object identity, so disabling the provider removes a distinct replacement handler when its implementation considers itself equal to the provider’s handler; the command then becomes unhandled.

Remove the mapping atomically only when its current value is the exact installed instance, using an identity check in a conditional map update.
</issue_to_address>

### Comment 15
<location path="jircd-core/src/main/java/net/jircd/core/session/FrameSyntax.java" line_range="55" />
<code_context>
+    Map<String, String> client = new HashMap<>();
+    for (var entry : tags.entrySet()) {
+      String key = entry.getKey();
+      if (key.isEmpty() || hasAny(key, " ;=\r\n\0"))
+        throw new IllegalArgumentException("Invalid tag key");
+      // CR and LF are escaped by the serializer, but NUL would be emitted unchanged.
</code_context>
<issue_to_address>
**Malformed tag keys reach clients**

When a provider returns a framed message with a tag key containing a tab or another disallowed control character, `FrameSyntax.validateTags` rejects only spaces, separators, and a few line-breaking characters, so a provider-supplied key containing a tab or another control character passes validation and `MessageSerializer` writes it verbatim into the tag section. Clients receive a malformed IRC line and may discard the message.

Validate tag keys against the protocol’s allowed key grammar before serializing them.
</issue_to_address>

Sourcery assessment

Approval pending. 15 findings to address first.

Blocking findings: jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:384, jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:267, jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java:244, jircd-core/src/main/java/net/jircd/core/session/CapabilityTagRenderer.java:91, jircd-core/src/main/java/net/jircd/core/extension/capability/HookDispatcher.java:164, and 10 more


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java Outdated
Comment thread jircd-core/src/main/java/net/jircd/core/extension/ExtensionRegistry.java Outdated
Comment thread jircd-core/src/main/java/net/jircd/core/session/CapabilityTagRenderer.java Outdated
Comment thread jircd-core/src/main/java/net/jircd/core/session/CapabilityTagRenderer.java Outdated
Comment thread jircd-core/src/main/java/net/jircd/core/session/ConnectionHandler.java Outdated
Comment thread jircd-core/src/main/java/net/jircd/core/session/FrameSyntax.java Outdated
Sourcery's review of #104 found gaps in the lifecycle and failure
paths that the earlier rounds missed; a follow-up Codex check of the
fixes found four more.

- Provider start/register/stop run outside the registry monitor, under
  a private lifecycle lock that no delivery path takes. Callbacks that
  re-enter enable, disable or reload fail fast instead of deadlocking.
- Commands are installed before hooks are published, and a provider
  command runs only while the current snapshot contains its provider,
  so hooks and commands switch together.
- Failed command removal and failed stop keep their records and the
  extension-point claim; disable (including from REHASH, and before a
  re-enable) retries the cleanup. Removal compares handlers by identity.
- Enable cleans up after any Error; REHASH handles LinkageError.
- Hook results are copied before validation; requiresProviders stages
  nothing unless every id is valid; tag keys must match the IRCv3 key
  grammar; oversized tags are rejected from their raw length first.
- Legacy contributeTags providers come from the captured snapshot, are
  contained like hooks, and are budgeted at enqueue, so an overflow
  drops their tags instead of the message.
- A reply bound that fails at the configured limits is a configuration
  error, not an IllegalArgumentException.

Refs #100

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codegeek
codegeek merged commit fc3aeab into main Oct 23, 2026
6 checks passed
@codegeek
codegeek deleted the 014-capability-plugin-spi branch October 23, 2026 20:16
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.

[Feature] Capability plugin SPI foundation, with server-time as the first module

1 participant