feat(notifier): in-server notifier delivery, IM fanout, Hub handlers, and notifier create fix#57
Conversation
6df574d to
5d4bc45
Compare
Rebased onto upstream/main. Adds internal/runtime/notifier (webhook HTTP, relay/subscription, pull worker, IM fanout via notifierbridge), ProfileExtensionsPolicy and API/bootstrap notifier runtime kind, agent create/skip default sandbox template for notifier, and Web UI notifier fields with Bearer-only webhook tests.
| type NotifyCard struct { | ||
| Type string `json:"type"` | ||
| SchemaVersion int `json:"schema_version"` | ||
| Provider string `json:"provider"` |
There was a problem hiding this comment.
csgclaw.notify_card looks like a generic notification schema, but Provider is currently documented and encoded around gitlab / github / generic. That makes the contract narrower than the message type itself, and future providers would either need a schema change or be collapsed into generic. Consider defining provider as an open source identifier string, with GitHub/GitLab only as current built-in examples, and keeping the stable contract on the generic display fields (title, subtitle, badge, summary, link, meta, raw).
| ProfileComplete bool `json:"profile_complete"` | ||
| DetectionResults []ProfileDetectionResult `json:"detection_results,omitempty"` | ||
| // LegacyDiskNotifierDetails is read from older state.json (top-level notifier_details); load migrates into agent.runtime_extensions. | ||
| LegacyDiskNotifierDetails map[string]any `json:"notifier_details,omitempty"` |
There was a problem hiding this comment.
LegacyDiskNotifierDetails looks unnecessary now that notifier config has been moved to runtime_extensions. Consider removing this compatibility field and keeping a single persisted representation.
| ReadHeaderTimeout: 5 * time.Second, | ||
| } | ||
|
|
||
| if opts.Service != nil && handler != nil { |
There was a problem hiding this comment.
Since notifier agents are expected to stay few in number, it may be cleaner to let each notifier own its own pull loop instead of running a global scanner from server startup. That keeps polling state and lifecycle local to the notifier runtime, and avoids leaking notifier-specific background orchestration into the server layer.
| } else { | ||
| var err error | ||
| created, err = s.agents.CreateWorker(ctx, agent.CreateAgentSpec{ | ||
| var createErr error |
There was a problem hiding this comment.
Using createErr here makes the block a bit noisier without really clarifying anything. Reusing the existing err would keep this path simpler.
| } | ||
|
|
||
| func deriveAgentHandle(a agent.Agent) string { | ||
| if notifier.IsDeliveryWorker(a.Role, a.RuntimeKind) { |
There was a problem hiding this comment.
This makes the bot layer aware of notifier-specific runtime behavior just to derive a handle. That coupling does not look necessary for the notifier feature itself; it seems cleaner to keep deriveAgentHandle() generic here.
| } | ||
| } | ||
|
|
||
| func TestNewStoreRejectsLegacyNotifierBotRole(t *testing.T) { |
There was a problem hiding this comment.
This test feels redundant with TestNewStoreRejectsInvalidState: role=notifier is just another invalid role now. If notifier-role compatibility is intentionally gone, adding a dedicated test for the legacy value seems unnecessary.
Align API, persistence, and web UI on runtime_options; remove legacy disk fields for runtime_extensions and top-level notifier_details. Run notifier pull delivery via a per-agent Supervisor started from serve through server.NotifierBackground. Classify notify cards with standard webhook headers when present; document open provider ids. Decouple bot deriveAgentHandle from notifier; merge redundant bot store tests and reuse err in createWorker.
Move routing to notifier.ServeNotifyHTTP, return 503 when fanout is nil after auth, and drop the create-user role guard for "notifier".
| OnReady func() | ||
| // NotifierBackground starts notifier-specific background work after the API handler is wired. | ||
| // The implementation should block until ctx is cancelled; Run invokes it in its own goroutine. | ||
| NotifierBackground func(ctx context.Context, svc *agent.Service, deliver runtimenotifier.Fanouter) |
There was a problem hiding this comment.
I don't think NotifierBackground should live in internal/server.
internal/server is a generic HTTP/server lifecycle layer, but this adds a notifier-specific startup hook to its public API. Since the pull supervisor is a long-running background loop, it also does not fit OnReady, which reads more like a one-shot readiness callback.
A cleaner split would be to keep internal/server focused on serving HTTP and start the notifier supervisor explicitly from the composition layer (cli/serve or runtime wiring), so the lifecycle ownership stays at the top level and notifier-specific wiring does not leak into the server package.
| } | ||
| } | ||
|
|
||
| func cloneRuntimeOptionsMapForBot(m map[string]any) map[string]any { |
There was a problem hiding this comment.
This helper looks redundant. Since we already have agentruntime.CloneAnyMap, it would be cleaner to reuse that here instead of keeping a bot-local cloneRuntimeOptionsMapForBot wrapper.
…ring Merge notifier/runtime helper files, colocate flat keys and API view logic in notifier, drop NotifierBackground from server in favor of explicit pull supervisor startup in cli/serve, and reuse runtime.CloneAnyMap in bot.
|
|
||
| // ProfileExtensionsPolicy defines how runtime_options (and related request_options) behave | ||
| // for a concrete runtime_kind. Implementations register via RegisterProfileExtensionsPolicy. | ||
| type ProfileExtensionsPolicy interface { |
There was a problem hiding this comment.
ProfileExtensionsPolicy -> RuntimeOptionsPolicy
| ProfileLLMFields(runtimeKind, baseURL, modelID string) (string, string) | ||
| // LooksLikeFlatStorageAtRoot detects notifier-shaped flat keys at the map root (used for legacy load heuristics). | ||
| LooksLikeFlatStorageAtRoot(m map[string]any) bool | ||
| ProfileComplete(isGatewayRuntime, llmComplete bool, agentExt map[string]any, runtimeKind string, mergedFlat map[string]any) bool |
| @@ -0,0 +1,28 @@ | |||
| package runtime | |||
There was a problem hiding this comment.
internal/runtime/maps.go -> internal/utils/maps.go
| // DeliverNotifierFanout posts notifier chat content (JSON notify card) to every IM room that includes this agent as a member and publishes SSE events. | ||
| func (h *Handler) DeliverNotifierFanout(agentID string, content string) error { | ||
| return notifier.DeliverNotifierFanout(agentID, content, h.notifierFanoutBridge()) | ||
| } |
There was a problem hiding this comment.
call API: codex SendMessage()
| llmComplete := profileIsComplete(out) | ||
| rk := normalizeRuntimeKind(runtimeKind) | ||
| pol := agentruntime.ProfileExtensionsPolicyForKind(agentruntime.NormalizeRuntimeKind(rk)) | ||
| out.ProfileComplete = pol.ProfileComplete(isGatewayRuntimeKind(rk), llmComplete, agentRuntimeExt, rk, notifierFlat) |
There was a problem hiding this comment.
- Simplify ProfileComplete(), only needs runtimOptions.
- out.ProfileComplete = llmComplete && pol.ProfileComplete()
| if isGatewayRuntime { | ||
| return llmComplete | ||
| } | ||
| return llmComplete |
| if strings.TrimSpace(profile.ModelID) == "" { | ||
| profile.ModelID = modelID | ||
| } | ||
| func (s *Service) profileForCreateRequest(ctx context.Context, spec *CreateAgentSpec) (AgentProfile, error) { |
| } | ||
| handler := server.NewHandler(serverOpts) | ||
| serverOpts.Handler = handler | ||
| go runtimewiring.RunNotifierPullSupervisor(ctx, svc, handler) |
There was a problem hiding this comment.
put PullSupervisor related goroutine in OnReady().
…ns policy Route notifier webhook fanout through POST /api/v1/messages instead of handler-internal IM/publish paths. Rename ProfileExtensionsPolicy to RuntimeOptionsPolicy and move shared map helpers to internal/utils.
Rename notifier helpers for runtime_options vs extensions, drop unused policy hooks on persist/create, pass through profile request_options in API views, and hide agent lifecycle controls in the web UI behind a flag.
Remove RuntimeOptionsPolicy.WithPullSubscriptionDefaults; notifier policy applies EnsurePullRemoteSubscriptionInNotifierDetails after MergeFlatForAgentPatch.
Stop storing notifier WebhookHTTPDeps on the API handler; extend server OnReady with the ServeMux so runtimewiring can mount POST /api/v1/notify/ alongside existing routes.
Move create-agent runtime_options copying out of the API handler into utils with a regression test for nested map isolation.
Summary
Adds and hardens Notifier support on top of the latest upstream/main: webhook and remote-pull delivery via request_options.notifier, fanout to IM rooms, alignment with worker + notifier runtime semantics, Hub template workspace API handlers used by the bootstrap/publish flow, and a fix so creating a notifier no longer resolves the default sandbox worker Hub template (avoids runtime_kind mismatch with templates such as builtin/picoclaw-worker).
Changes
Notifier: relay/subscription wiring, webhook vs remote-pull modes, structured notify payloads into IM (with existing SSE/event paths as implemented in-tree).
Agent / IM / UI: notifier treated consistently as a worker-style agent with notifier runtime; profile/DM and related Web UI adjustments from this branch.
API: Hub template workspace detail and file handlers completed where callers expected them.
Agent service: skip default Hub worker template resolution when runtime_kind is notifier; regression test added.
Misc: minor web merge/style cleanup from the branch history.