Repository navigation
Conversation
The 014 design assumed an existing administrative mutex; there wasn't one. Record the decision: ExtensionRegistry's lifecycle lock serializes whole transitions and a short CapabilityRuntime mutex covers publish+prune, CAP LS/LIST/REQ and session admission. Update the contracts for snapshot capture, single publication per reload, CAP DEL/NEW, negative REQ, core cap-notify in LS and snapshot-resolved provider commands. Refs #97 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Disabling a capability left it active for clients that had negotiated it, because the negotiated sets were never pruned and CAP validation and apply were separate steps. Publication now swaps an immutable snapshot and prunes every live session under one mutex, then sends CAP DEL/NEW. CAP REQ validates and applies in one step, so a REQ racing a disable cannot restore a pruned name. A REHASH validates the whole candidate graph (cycles, missing prerequisites, conflicts, reply budgets) before changing anything, starts providers outside the mutex, publishes once, and stops withdrawn providers dependents-first. Provider commands are part of the snapshot, so a command moving between providers is never briefly unknown, and a server extension can't shadow a provider's command. NICK changes and self-notification are serialized with publication so CAP DEL never targets a stale nick. Publication with 1,000 sessions takes under 4 ms including DEL/NEW (budget 100 ms). fixes #97 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Sorry @codegeek, this account has used its review budget of 2,500,000 diff characters for the last 7 days. You can request another review in 13 hours and 19 minutes by commenting |
Reviewer's GuideThis PR closes the runtime capability lifecycle gap by atomically publishing immutable capability snapshots, pruning withdrawn capabilities from every live session, and synchronizing CAP negotiation, notifications, reloads, command dispatch, and related session behavior around a two-level locking model, with extensive protocol, concurrency, failure-path, integration, and load-test coverage plus corresponding contract and design documentation. Sequence diagram for atomic capability withdrawalsequenceDiagram
participant Admin as Admin or REHASH
participant Registry as ExtensionRegistry
participant Runtime as CapabilityRuntime
participant Sessions as Live sessions
participant Writers as Session writers
participant Provider as Provider lifecycle
Admin->>Registry: disableLocked(id) or applyDesiredState(...)
Registry->>Registry: acquire lifecycle lock
Registry->>Runtime: withdraw(id) / publish(change)
Runtime->>Runtime: acquire administrative mutex
Runtime->>Runtime: publish immutable RuntimeSnapshot
Runtime->>Sessions: prune withdrawn capabilities
Runtime->>Writers: queue CAP DEL and CAP NEW
Runtime-->>Registry: publication complete
Registry->>Provider: stop() outside runtime mutex
Registry-->>Admin: actual lifecycle outcome
Sequence diagram for race-safe CAP REQ and publicationsequenceDiagram
participant Client as Client
participant CAP as CapCommandHandler
participant Control as CapabilityControl
participant Runtime as CapabilityRuntime
participant Session as ClientSession
Client->>CAP: CAP REQ capabilities
CAP->>Control: runUnderControl(...)
Control->>Runtime: acquire administrative mutex
CAP->>Session: candidate(session, requested)
alt request accepted
CAP->>Session: replaceNegotiatedCapabilities(candidate)
CAP-->>Client: CAP ACK original request
else request refused
CAP-->>Client: CAP NAK original request
end
Runtime-->>Control: release mutex
Note over Runtime,Session: A concurrent publication either prunes first or CAP REQ commits first, and a withdrawn name cannot be restored
Sequence diagram for provider command handoversequenceDiagram
participant Client as Client
participant Handler as ConnectionHandler
participant Registry as ExtensionRegistry
participant Runtime as CapabilityRuntime
participant Snapshot as RuntimeSnapshot
participant Provider as Provider command handler
Client->>Handler: command
Handler->>Handler: resolve core command table
alt no core handler
Handler->>Registry: capabilityCommand(command)
Registry->>Runtime: snapshot()
Runtime-->>Registry: one immutable snapshot read
Registry->>Snapshot: command(command)
Snapshot-->>Registry: published provider handler
Registry->>Provider: guardedCommand(registration)
Provider-->>Client: handle command
else core handler exists
Handler-->>Client: dispatch core handler
end
Note over Runtime,Snapshot: Reload publication swaps provider commands with hooks, so a command moves atomically between providers
Flow diagram for atomic configuration reloadflowchart TD
Reload[ConfigurationReloader.reloadWithOutcome] --> Validate[Validate complete candidate graph and limits]
Validate -->|invalid| Unchanged[Leave snapshot and sessions unchanged]
Validate -->|valid| Start[Start providers dependency-first]
Start --> Publish[Publish snapshot and configuration together]
Publish --> Prune[Prune withdrawn capabilities from live sessions]
Prune --> Notify[Queue CAP DEL and CAP NEW]
Notify --> Stop[Stop withdrawn providers dependents-first]
Start -->|start failure| Failed[Record FAILED provider and dependent outcomes]
Failed --> Publish
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Disabling a capability at runtime (
EXTENSION disable, REHASH or SIGHUP) left it active for clients that had already negotiated it (#97). Negotiated sets were never pruned, and CAP validation and apply were separate steps, so aCAP REQthat raced a disable could bring the name back. This is PR 3 of spec 014 (tasks T042–T054), after #104 and #106.Changes
RuntimeSnapshotand prunes every live session in one step under a short mutex inCapabilityRuntime. CAP DEL/NEW are queued after publication. Work that captured the old snapshot may still finish, which the spec allows.ExtensionRegistry's lifecycle lock serializes whole transitions. The runtime mutex covers publish+prune, CAP LS/LIST/REQ, adding/removing/attaching live sessions and the batch group choice. Lock order is lifecycle → runtime mutex → one session writer lock. Provider start/stop never run under the mutex. Code outside the runtime gets aCapabilityControlhandle that has no publish methods.cap-notifyis always advertised. LS-version tracking is sticky and overflow-safe, LIST distinguishes explicit and implicit, and NEW/DEL/LS/LIST lines are packed within bounds.EXTENSIONandREHASHgo through the same lifecycle and report the actual outcome.plan.md/research.mdrecords the two-level lock. Thespecs/001contracts, the data model and the README are updated. Validation records are inspecs/014-capability-plugin-spi/validation/pr-97/.Testing
./gradlew cleanTest buildis green: 1,340 tests. Coverage is 96.03% lines and 89.11% branches, above [Feature] Move message-tags, echo-message, away-notify, multi-prefix and batch onto the capability SPI #101's 95.72% / 88.70%.validation/pr-97/review.md. The Sonar branch scan reports 0 issues and the gate passes.Open points (recorded in
review.md)fixes #97
🤖 Generated with Claude Code
Summary by Sourcery
Make runtime capability lifecycle changes atomic across snapshots, negotiated sessions, notifications, provider commands, and configuration reloads.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: