Skip to content

Commit bc1eb1c

Browse files
committed
skillinject: write per-tool webhook URL on reconcile
ManifestTool gains a webhookURL field. On each tick, after the existing heartbeat / skill / plugin reconcile passes, the daemon walks tools in manifest order, picks the first one whose rootDir exists AND that declares a non-empty webhookURL, and writes that URL to ~/.pilot/webhook_url. The webhook plugin reads that file on Start. Precedence rule: first installed tool with a URL wins. Today that means openclaw if installed, else hermes if installed, else no daemon-set URL (operator-set URL via pilotctl set-webhook is left alone — we never blank a URL we didn't choose to write). Hot-swap caveat: the URL applies on next daemon restart, since the webhook plugin reads the file only on Start. Restart happens hourly via the auto-updater, or whenever the operator restarts manually. Plumbing a callback so Tick can SetWebhookURL on a running daemon is left as future work — would require widening skillinject.Config or threading deps through internalskillinject.Run. Tests cover: precedence (first installed wins), uninstalled tool skipped, no-tools-want = leave operator-set URL alone, create on cold start, idempotent on second tick, rewrite on drift.
1 parent e961d2f commit bc1eb1c

5 files changed

Lines changed: 284 additions & 0 deletions

File tree

internal/skillinject/manifest.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,15 @@ type ManifestTool struct {
7575
// into a YAML config (today hermes — see ManifestWebhookRoute). One
7676
// tool can declare any number; each reconciles independently.
7777
WebhookRoutes []ManifestWebhookRoute `json:"webhookRoutes,omitempty"`
78+
// WebhookURL, if set, tells the daemon to write this URL into
79+
// ~/.pilot/webhook_url so its webhook plugin POSTs events to the
80+
// tool's receiver. Set per-tool with the canonical default port and
81+
// path (e.g. openclaw → http://127.0.0.1:18789/pilot-webhook).
82+
// When multiple tools declare a URL, the daemon picks the first one
83+
// whose RootDir exists on disk — manifest order is the tiebreaker.
84+
// Operators wanting multi-tool delivery can run pilotctl set-webhook
85+
// explicitly.
86+
WebhookURL string `json:"webhookURL,omitempty"`
7887
}
7988

8089
// ManifestWebhookRoute describes a route the daemon adds to a YAML

internal/skillinject/skillinject.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,12 @@ func Tick(ctx context.Context, cfg Config) (*Report, error) {
247247
}
248248
}
249249

250+
// One pilot-wide webhook URL across all installed tools. Picks the
251+
// first manifest tool whose rootDir exists. Noop if nothing asks
252+
// for a URL or the file is already correct.
253+
report.Outcomes = append(report.Outcomes,
254+
reconcileWebhookURL(home, manifest.Tools))
255+
250256
return report, nil
251257
}
252258

internal/skillinject/state.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const (
4040
KindPluginFile FileKind = "plugin_file"
4141
KindPluginAllowList FileKind = "plugin_allowlist"
4242
KindWebhookRoute FileKind = "webhook_route"
43+
KindWebhookURL FileKind = "webhook_url"
4344
)
4445

4546
func actionFor(s State) Action {
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package skillinject
4+
5+
import (
6+
"os"
7+
"path/filepath"
8+
"strings"
9+
)
10+
11+
// webhookURLFile is the path the webhook plugin reads on Service.Start.
12+
// Sibling to webhook_url.go in plugins/webhook (LoadPersistedURL). We
13+
// don't import the plugin here — that would create a layering edge —
14+
// so the path is duplicated as a known string. If LoadPersistedURL
15+
// ever moves, this constant has to follow.
16+
const webhookURLFile = "webhook_url"
17+
18+
// pilotWebhookURLPath returns the canonical file the daemon's webhook
19+
// plugin reads on startup. Centralized so the test harness can override
20+
// HOME and exercise the file path without rebuilding the plugin.
21+
func pilotWebhookURLPath(home string) string {
22+
return filepath.Join(home, ".pilot", webhookURLFile)
23+
}
24+
25+
// pickWebhookURL walks the manifest's tools in order and returns the
26+
// first non-empty WebhookURL whose corresponding rootDir exists on
27+
// disk (i.e. the tool is actually installed). Empty result = no tool
28+
// asked for a URL OR no asking tool is installed. Caller leaves the
29+
// existing file alone in that case rather than blanking it.
30+
func pickWebhookURL(tools []ManifestTool, home string) (url string, toolName string) {
31+
for _, mt := range tools {
32+
if mt.WebhookURL == "" {
33+
continue
34+
}
35+
root := expandHome(mt.RootDir, home)
36+
if !dirExists(root) {
37+
continue
38+
}
39+
return mt.WebhookURL, mt.Name
40+
}
41+
return "", ""
42+
}
43+
44+
// reconcileWebhookURL writes the picked URL to ~/.pilot/webhook_url so
45+
// the next pilot-daemon restart hands it to the webhook plugin. If the
46+
// file already contains the picked URL, this is a noop. Returns an
47+
// Outcome that surfaces in the tick report.
48+
//
49+
// Today the daemon picks up the change on next restart (hourly via the
50+
// auto-updater, or whenever the operator restarts). Hot-swapping a
51+
// running daemon requires plumbing a callback through skillinject.Run
52+
// → cmd/daemon — left as future work; documented in the manifest
53+
// field comment.
54+
func reconcileWebhookURL(home string, tools []ManifestTool) Outcome {
55+
url, toolName := pickWebhookURL(tools, home)
56+
path := pilotWebhookURLPath(home)
57+
o := Outcome{Tool: toolName, Kind: KindWebhookURL, Path: path}
58+
if url == "" {
59+
// No tool wants a URL right now. Leave any existing file in
60+
// place — operator may have set it manually via pilotctl. We
61+
// only manage the URL when we have one to write.
62+
o.State = StateIdentical
63+
o.Action = ActionNoop
64+
return o
65+
}
66+
67+
current, err := os.ReadFile(path)
68+
if err == nil && strings.TrimSpace(string(current)) == url {
69+
o.State = StateIdentical
70+
o.Action = ActionNoop
71+
return o
72+
}
73+
if err != nil && !os.IsNotExist(err) {
74+
o.State = StateDrifted
75+
o.Action = ActionError
76+
o.Err = err.Error()
77+
return o
78+
}
79+
o.State = StateDrifted
80+
if os.IsNotExist(err) {
81+
o.State = StateAbsent
82+
}
83+
o.Action = actionFor(o.State)
84+
85+
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
86+
o.Action = ActionError
87+
o.Err = "ensure parent dir: " + err.Error()
88+
return o
89+
}
90+
if err := writeFileAtomic(path, []byte(url), 0o600); err != nil {
91+
o.Action = ActionError
92+
o.Err = err.Error()
93+
return o
94+
}
95+
return o
96+
}
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package skillinject
4+
5+
import (
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
)
10+
11+
// TestPickWebhookURL_FirstInstalled walks tools in manifest order and
12+
// returns the first installed tool's URL. Confirms the precedence
13+
// contract: openclaw beats hermes if both are installed and both
14+
// declare a URL, because openclaw comes first in the manifest.
15+
func TestPickWebhookURL_FirstInstalled(t *testing.T) {
16+
t.Parallel()
17+
dir := t.TempDir()
18+
// Make BOTH ~/.openclaw and ~/.hermes exist
19+
if err := os.MkdirAll(filepath.Join(dir, ".openclaw"), 0o700); err != nil {
20+
t.Fatal(err)
21+
}
22+
if err := os.MkdirAll(filepath.Join(dir, ".hermes"), 0o700); err != nil {
23+
t.Fatal(err)
24+
}
25+
tools := []ManifestTool{
26+
{Name: "openclaw", RootDir: "~/.openclaw", WebhookURL: "http://127.0.0.1:18789/pilot-webhook"},
27+
{Name: "hermes", RootDir: "~/.hermes", WebhookURL: "http://127.0.0.1:8644/pilot-events"},
28+
}
29+
url, name := pickWebhookURL(tools, dir)
30+
if name != "openclaw" {
31+
t.Fatalf("expected openclaw to win precedence, got %q", name)
32+
}
33+
if url != "http://127.0.0.1:18789/pilot-webhook" {
34+
t.Fatalf("url=%q", url)
35+
}
36+
}
37+
38+
// TestPickWebhookURL_SkipsUninstalledTools confirms an asking tool
39+
// whose rootDir doesn't exist is skipped (no false positive).
40+
func TestPickWebhookURL_SkipsUninstalledTools(t *testing.T) {
41+
t.Parallel()
42+
dir := t.TempDir()
43+
// Only hermes is installed; openclaw asks but isn't present.
44+
if err := os.MkdirAll(filepath.Join(dir, ".hermes"), 0o700); err != nil {
45+
t.Fatal(err)
46+
}
47+
tools := []ManifestTool{
48+
{Name: "openclaw", RootDir: "~/.openclaw", WebhookURL: "http://127.0.0.1:18789/pilot-webhook"},
49+
{Name: "hermes", RootDir: "~/.hermes", WebhookURL: "http://127.0.0.1:8644/pilot-events"},
50+
}
51+
url, name := pickWebhookURL(tools, dir)
52+
if name != "hermes" {
53+
t.Fatalf("expected hermes (only one installed), got %q", name)
54+
}
55+
if url != "http://127.0.0.1:8644/pilot-events" {
56+
t.Fatalf("url=%q", url)
57+
}
58+
}
59+
60+
// TestPickWebhookURL_NoTools returns empty when no tool is installed
61+
// or no tool declares a URL.
62+
func TestPickWebhookURL_NoTools(t *testing.T) {
63+
t.Parallel()
64+
dir := t.TempDir()
65+
tools := []ManifestTool{
66+
{Name: "openclaw", RootDir: "~/.openclaw", WebhookURL: "http://127.0.0.1:18789/pilot-webhook"},
67+
}
68+
// Nothing installed in dir → pickWebhookURL returns empty.
69+
url, name := pickWebhookURL(tools, dir)
70+
if url != "" || name != "" {
71+
t.Fatalf("expected empty when no tool installed, got name=%q url=%q", name, url)
72+
}
73+
}
74+
75+
// TestReconcileWebhookURL_WritesFileWhenMissing covers the cold-start
76+
// case: ~/.pilot/webhook_url doesn't exist; we should create it with
77+
// the picked URL.
78+
func TestReconcileWebhookURL_WritesFileWhenMissing(t *testing.T) {
79+
t.Parallel()
80+
dir := t.TempDir()
81+
if err := os.MkdirAll(filepath.Join(dir, ".openclaw"), 0o700); err != nil {
82+
t.Fatal(err)
83+
}
84+
tools := []ManifestTool{
85+
{Name: "openclaw", RootDir: "~/.openclaw", WebhookURL: "http://127.0.0.1:18789/pilot-webhook"},
86+
}
87+
o := reconcileWebhookURL(dir, tools)
88+
if o.Action != ActionCreate {
89+
t.Fatalf("expected Create, got %v (err=%q)", o.Action, o.Err)
90+
}
91+
got, err := os.ReadFile(filepath.Join(dir, ".pilot", "webhook_url"))
92+
if err != nil {
93+
t.Fatalf("read-back: %v", err)
94+
}
95+
if string(got) != "http://127.0.0.1:18789/pilot-webhook" {
96+
t.Fatalf("on-disk url=%q", got)
97+
}
98+
}
99+
100+
// TestReconcileWebhookURL_IdempotentWhenCorrect runs reconcile twice;
101+
// the second call should be a Noop.
102+
func TestReconcileWebhookURL_IdempotentWhenCorrect(t *testing.T) {
103+
t.Parallel()
104+
dir := t.TempDir()
105+
if err := os.MkdirAll(filepath.Join(dir, ".openclaw"), 0o700); err != nil {
106+
t.Fatal(err)
107+
}
108+
tools := []ManifestTool{
109+
{Name: "openclaw", RootDir: "~/.openclaw", WebhookURL: "http://127.0.0.1:18789/pilot-webhook"},
110+
}
111+
_ = reconcileWebhookURL(dir, tools)
112+
o := reconcileWebhookURL(dir, tools)
113+
if o.Action != ActionNoop {
114+
t.Fatalf("second tick should be Noop, got %v", o.Action)
115+
}
116+
}
117+
118+
// TestReconcileWebhookURL_LeavesFileWhenNoToolWants ensures we don't
119+
// blank out an operator-set URL when no installed tool asks for one
120+
// — the daemon would lose its webhook config otherwise.
121+
func TestReconcileWebhookURL_LeavesFileWhenNoToolWants(t *testing.T) {
122+
t.Parallel()
123+
dir := t.TempDir()
124+
// Pre-populate with an operator URL.
125+
if err := os.MkdirAll(filepath.Join(dir, ".pilot"), 0o700); err != nil {
126+
t.Fatal(err)
127+
}
128+
manual := filepath.Join(dir, ".pilot", "webhook_url")
129+
if err := os.WriteFile(manual, []byte("https://my-custom-endpoint.example.com/hook"), 0o600); err != nil {
130+
t.Fatal(err)
131+
}
132+
// No installed tool asks for a URL.
133+
tools := []ManifestTool{
134+
{Name: "openclaw", RootDir: "~/.openclaw", WebhookURL: "http://127.0.0.1:18789/pilot-webhook"},
135+
}
136+
o := reconcileWebhookURL(dir, tools)
137+
if o.Action != ActionNoop {
138+
t.Fatalf("expected Noop when no tool asks, got %v", o.Action)
139+
}
140+
got, _ := os.ReadFile(manual)
141+
if string(got) != "https://my-custom-endpoint.example.com/hook" {
142+
t.Fatalf("operator-set URL was blanked: %q", got)
143+
}
144+
}
145+
146+
// TestReconcileWebhookURL_RewritesOnDrift confirms that when the file
147+
// has an old URL and the picked URL differs, we update the file.
148+
func TestReconcileWebhookURL_RewritesOnDrift(t *testing.T) {
149+
t.Parallel()
150+
dir := t.TempDir()
151+
if err := os.MkdirAll(filepath.Join(dir, ".openclaw"), 0o700); err != nil {
152+
t.Fatal(err)
153+
}
154+
if err := os.MkdirAll(filepath.Join(dir, ".pilot"), 0o700); err != nil {
155+
t.Fatal(err)
156+
}
157+
path := filepath.Join(dir, ".pilot", "webhook_url")
158+
if err := os.WriteFile(path, []byte("http://127.0.0.1:18789/old-path"), 0o600); err != nil {
159+
t.Fatal(err)
160+
}
161+
tools := []ManifestTool{
162+
{Name: "openclaw", RootDir: "~/.openclaw", WebhookURL: "http://127.0.0.1:18789/pilot-webhook"},
163+
}
164+
o := reconcileWebhookURL(dir, tools)
165+
if o.Action != ActionRewrite {
166+
t.Fatalf("expected Rewrite on drift, got %v (err=%q)", o.Action, o.Err)
167+
}
168+
got, _ := os.ReadFile(path)
169+
if string(got) != "http://127.0.0.1:18789/pilot-webhook" {
170+
t.Fatalf("url not updated: %q", got)
171+
}
172+
}

0 commit comments

Comments
 (0)