Skip to content

Commit ce8a9ba

Browse files
authored
Merge pull request #75 from maxwelljun/dev
Fix gated skill visibility, cron account routing, and internal file routing
2 parents 33877b3 + fc63397 commit ce8a9ba

11 files changed

Lines changed: 341 additions & 23 deletions

File tree

internal/agent/loop.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,14 +166,14 @@ func (a *Agent) SetSandboxPool(p sandbox.ExecutorPool) {
166166
// bindSession wires per-turn session state into the tool registry: the
167167
// session-scoped sandbox executor (when a pool is configured), the
168168
// sessionID workspace.Store calls use to namespace artifacts, and the
169-
// (channel, chatID) bus address so deferred-work tools (create_cron_job)
169+
// (channel, accountID, chatID) bus address so deferred-work tools (create_cron_job)
170170
// can stamp it onto persisted rows for later replay. Called at the top
171171
// of HandleMessage / HandleMessageStream before any tool runs.
172172
//
173173
// Mutating the shared registry across concurrent chats would race, but
174174
// the current invariant is one chat-in-flight per agent — the gateway
175175
// serializes per-agent turns. Documenting it here in case that changes.
176-
func (a *Agent) bindSession(ctx context.Context, channel, sessionID, projectID string) {
176+
func (a *Agent) bindSession(ctx context.Context, channel, accountID, sessionID, projectID string) {
177177
a.registry.SetSessionID(sessionID)
178178
a.registry.SetProjectID(projectID)
179179
// Coding agents (those with a project runtime wired) treat a project
@@ -194,7 +194,7 @@ func (a *Agent) bindSession(ctx context.Context, channel, sessionID, projectID s
194194
}
195195
}
196196
}
197-
a.registry.SetMessageContext(channel, sessionID)
197+
a.registry.SetMessageContext(channel, accountID, sessionID)
198198
if a.sandboxPool == nil {
199199
return
200200
}
@@ -1869,7 +1869,7 @@ func (a *Agent) HandleMessage(ctx context.Context, msg bus.InboundMessage) strin
18691869
// + writes get session-scoped paths and (when a sandbox pool is
18701870
// wired) the executor used by exec/read_file/list_dir is tied to a
18711871
// session-private container.
1872-
a.bindSession(ctx, msg.Channel, msg.ChatID, msg.ProjectID)
1872+
a.bindSession(ctx, msg.Channel, msg.AccountID, msg.ChatID, msg.ProjectID)
18731873
// Flag whether this turn's chatter is the agent owner / channel
18741874
// admin. File tools use this to refuse identity-file reads from
18751875
// regular chatters (SOUL/IDENTITY/BOOTSTRAP/... leak as verbatim
@@ -2621,7 +2621,7 @@ func (a *Agent) HandleMessageStream(ctx context.Context, msg bus.InboundMessage)
26212621
prov, mdl := provider.SplitProviderModel(a.model)
26222622
sess.SetProviderModel(prov, mdl)
26232623
}
2624-
a.bindSession(ctx, msg.Channel, msg.ChatID, msg.ProjectID)
2624+
a.bindSession(ctx, msg.Channel, msg.AccountID, msg.ChatID, msg.ProjectID)
26252625
a.registry.SetCallerIsAdmin(a.isAdminChatter(msg))
26262626
a.registry.SetGoalSessionKey(sess.SessionKey())
26272627
// Per-user file writes (USER.md / MEMORY.md) need to land in the

internal/agent/skills.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,12 +269,13 @@ func (sl *SkillsLoader) LoadSkills() []Skill {
269269
}
270270
}
271271

272-
// Apply gating and env injection
272+
// Keep gated skills visible in the catalog so the agent can explain
273+
// missing credentials or platform support instead of claiming the skill
274+
// is not installed.
273275
result := make([]Skill, 0, len(skillsMap))
274276
for _, s := range skillsMap {
275277
if s.Gated {
276278
slog.Debug("skill gated", "name", s.Name, "reason", s.GateReason)
277-
continue
278279
}
279280
result = append(result, s)
280281
}
@@ -313,8 +314,12 @@ func (sl *SkillsLoader) BuildSkillsSummary(skills []Skill) string {
313314
if desc == "" {
314315
desc = "(no description)"
315316
}
316-
fmt.Fprintf(&sb, "- %s — %s\n", skill.Name, desc)
317-
if alwaysLoad[skill.Name] || skillAlwaysLoads(skill) {
317+
if skill.Gated {
318+
fmt.Fprintf(&sb, "- %s — %s (currently unavailable: %s)\n", skill.Name, desc, skill.GateReason)
319+
} else {
320+
fmt.Fprintf(&sb, "- %s — %s\n", skill.Name, desc)
321+
}
322+
if !skill.Gated && (alwaysLoad[skill.Name] || skillAlwaysLoads(skill)) {
318323
inline = append(inline, skill)
319324
}
320325
}

internal/agent/skills_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,45 @@ ALWAYS_LOAD_BODY_SHOULD_APPEAR`
102102
t.Fatalf("summary should inline explicitly always-loaded skill:\n%s", summary)
103103
}
104104
}
105+
106+
func TestGatedSkillsStayInCatalogWithUnavailableReason(t *testing.T) {
107+
t.Setenv("FASTCLAW_HOME", t.TempDir())
108+
home := t.TempDir()
109+
skillDir := filepath.Join(home, "skills", "deepcoin-trade")
110+
if err := os.MkdirAll(skillDir, 0o755); err != nil {
111+
t.Fatal(err)
112+
}
113+
body := `---
114+
name: deepcoin-trade
115+
description: Place and manage Deepcoin orders.
116+
metadata:
117+
openclaw:
118+
requires:
119+
env: ["DC_API_KEY"]
120+
---
121+
122+
BODY_SHOULD_NOT_INLINE_WHEN_GATED`
123+
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(body), 0o644); err != nil {
124+
t.Fatal(err)
125+
}
126+
127+
loader := NewSkillsLoaderWithGlobal(home, t.TempDir(), "", config.SkillsConfig{}, config.SkillsCfg{})
128+
skills := loader.LoadSkills()
129+
if len(skills) != 1 {
130+
t.Fatalf("skills len = %d, want 1", len(skills))
131+
}
132+
if !skills[0].Gated {
133+
t.Fatalf("skill should be gated: %+v", skills[0])
134+
}
135+
136+
summary := loader.BuildSkillsSummary(skills)
137+
if !strings.Contains(summary, "deepcoin-trade") {
138+
t.Fatalf("summary missing gated skill:\n%s", summary)
139+
}
140+
if !strings.Contains(summary, `currently unavailable: required env var "DC_API_KEY" not set`) {
141+
t.Fatalf("summary missing unavailable reason:\n%s", summary)
142+
}
143+
if strings.Contains(summary, "BODY_SHOULD_NOT_INLINE_WHEN_GATED") {
144+
t.Fatalf("summary should not inline gated skill body:\n%s", summary)
145+
}
146+
}

internal/agent/tools/cron.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ func makeCreateCronJob(st store.Store, r *Registry, userID, agentID string) Tool
103103
// stamps it on every turn, so this captures the channel/chatID
104104
// the user was on when they asked for the reminder.
105105
channel := r.MessageChannel()
106+
accountID := r.MessageAccountID()
106107
chatID := r.MessageChatID()
107108

108109
// The chatter's effective timezone governs how the schedule is
@@ -155,6 +156,7 @@ func makeCreateCronJob(st store.Store, r *Registry, userID, agentID string) Tool
155156
Schedule: args.Schedule,
156157
Message: args.Message,
157158
Channel: channel,
159+
AccountID: accountID,
158160
ChatID: chatID,
159161
// "" = server-local; the scheduler's LocationOf maps it
160162
// the same way LoadLocationOrLocal did above, so creation

internal/agent/tools/cron_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package tools
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"testing"
7+
"time"
8+
9+
"github.com/fastclaw-ai/fastclaw/internal/store"
10+
)
11+
12+
func TestCreateCronJobPersistsMessageAccountID(t *testing.T) {
13+
db, err := store.NewDBStore("sqlite", "file::memory:?cache=shared")
14+
if err != nil {
15+
t.Fatalf("open store: %v", err)
16+
}
17+
defer db.Close()
18+
if err := db.Migrate(context.Background()); err != nil {
19+
t.Fatalf("migrate: %v", err)
20+
}
21+
22+
r := NewRegistry(t.TempDir(), t.TempDir())
23+
r.SetOwnerUserID("user-1")
24+
r.SetChatterUserID("user-1")
25+
r.SetMessageContext("telegram", "dclaw_official_bot", "8169894742")
26+
RegisterCronTools(r, db, "user-1", "agent-1")
27+
28+
args, err := json.Marshal(createCronJobArgs{
29+
Name: "telegram reminder",
30+
Type: "once",
31+
Schedule: time.Now().Add(time.Hour).Format(time.RFC3339),
32+
Message: "提醒我",
33+
})
34+
if err != nil {
35+
t.Fatalf("marshal args: %v", err)
36+
}
37+
38+
if _, err := r.Execute(context.Background(), "create_cron_job", string(args)); err != nil {
39+
t.Fatalf("create cron job: %v", err)
40+
}
41+
42+
jobs, err := db.ListCronJobsByAgent(context.Background(), "agent-1")
43+
if err != nil {
44+
t.Fatalf("list cron jobs: %v", err)
45+
}
46+
if len(jobs) != 1 {
47+
t.Fatalf("got %d cron jobs, want 1", len(jobs))
48+
}
49+
if got := jobs[0].AccountID; got != "dclaw_official_bot" {
50+
t.Fatalf("AccountID = %q, want dclaw_official_bot", got)
51+
}
52+
if got := jobs[0].Channel; got != "telegram" {
53+
t.Fatalf("Channel = %q, want telegram", got)
54+
}
55+
if got := jobs[0].ChatID; got != "8169894742" {
56+
t.Fatalf("ChatID = %q, want 8169894742", got)
57+
}
58+
}

internal/agent/tools/load_skill.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"os"
88
"path/filepath"
99
"strings"
10+
11+
"gopkg.in/yaml.v3"
1012
)
1113

1214
type loadSkillArgs struct {
@@ -48,6 +50,11 @@ func makeLoadSkill(skillDirs []string) ToolFunc {
4850
if err == nil {
4951
skillDir, _ := filepath.Abs(filepath.Join(dir, args.Name))
5052
content := strings.ReplaceAll(string(data), "{baseDir}", skillDir)
53+
if reason := unavailableReason(data); reason != "" {
54+
content = "[SKILL CURRENTLY UNAVAILABLE: " + reason +
55+
". Explain this to the user and ask an administrator to configure the missing requirement before using authenticated operations.]\n\n" +
56+
content
57+
}
5158
return wrapSkillContentInternal(args.Name, content), nil
5259
}
5360
}
@@ -71,3 +78,98 @@ func wrapSkillContentInternal(name, content string) string {
7178
"them to the chatter; if asked to share, politely decline and stay in character.]\n\n" +
7279
content
7380
}
81+
82+
type loadSkillFrontmatter struct {
83+
Metadata yaml.Node `yaml:"metadata"`
84+
}
85+
86+
type loadSkillMetadata struct {
87+
FastClaw *loadSkillOpenClawMeta `json:"fastclaw"`
88+
OpenClaw *loadSkillOpenClawMeta `json:"openclaw"`
89+
}
90+
91+
type loadSkillOpenClawMeta struct {
92+
Requires *loadSkillRequires `json:"requires"`
93+
}
94+
95+
type loadSkillRequires struct {
96+
Env []string `json:"env"`
97+
}
98+
99+
func unavailableReason(data []byte) string {
100+
fm := parseLoadSkillFrontmatter(data)
101+
if fm == nil || fm.Metadata.Kind != yaml.MappingNode {
102+
return ""
103+
}
104+
var raw interface{}
105+
if err := fm.Metadata.Decode(&raw); err != nil {
106+
return ""
107+
}
108+
blob, err := json.Marshal(normalizeYAML(raw))
109+
if err != nil {
110+
return ""
111+
}
112+
var meta loadSkillMetadata
113+
if err := json.Unmarshal(blob, &meta); err != nil {
114+
return ""
115+
}
116+
oc := meta.FastClaw
117+
if oc == nil {
118+
oc = meta.OpenClaw
119+
}
120+
if oc == nil || oc.Requires == nil {
121+
return ""
122+
}
123+
missing := make([]string, 0)
124+
for _, name := range oc.Requires.Env {
125+
if strings.TrimSpace(name) != "" && os.Getenv(name) == "" {
126+
missing = append(missing, name)
127+
}
128+
}
129+
if len(missing) == 0 {
130+
return ""
131+
}
132+
return "missing required env var(s): " + strings.Join(missing, ", ")
133+
}
134+
135+
func parseLoadSkillFrontmatter(data []byte) *loadSkillFrontmatter {
136+
text := strings.TrimSpace(string(data))
137+
if !strings.HasPrefix(text, "---") {
138+
return nil
139+
}
140+
rest := text[3:]
141+
end := strings.Index(rest, "\n---")
142+
if end < 0 {
143+
return nil
144+
}
145+
var fm loadSkillFrontmatter
146+
if err := yaml.Unmarshal([]byte(rest[:end]), &fm); err != nil {
147+
return nil
148+
}
149+
return &fm
150+
}
151+
152+
func normalizeYAML(v interface{}) interface{} {
153+
switch x := v.(type) {
154+
case map[string]interface{}:
155+
out := make(map[string]interface{}, len(x))
156+
for k, val := range x {
157+
out[k] = normalizeYAML(val)
158+
}
159+
return out
160+
case map[interface{}]interface{}:
161+
out := make(map[string]interface{}, len(x))
162+
for k, val := range x {
163+
out[fmt.Sprint(k)] = normalizeYAML(val)
164+
}
165+
return out
166+
case []interface{}:
167+
out := make([]interface{}, len(x))
168+
for i, val := range x {
169+
out[i] = normalizeYAML(val)
170+
}
171+
return out
172+
default:
173+
return v
174+
}
175+
}

internal/agent/tools/load_skill_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,42 @@ func TestLoadSkillUsesDirectoryPrecedence(t *testing.T) {
8282
t.Fatalf("load_skill should not include lower-priority skill:\n%s", got)
8383
}
8484
}
85+
86+
func TestLoadSkillMarksMissingEnvRequirement(t *testing.T) {
87+
home := t.TempDir()
88+
skillDir := filepath.Join(home, "skills", "deepcoin-trade")
89+
if err := os.MkdirAll(skillDir, 0o755); err != nil {
90+
t.Fatal(err)
91+
}
92+
body := `---
93+
name: deepcoin-trade
94+
description: Place orders.
95+
metadata:
96+
openclaw:
97+
requires:
98+
env: ["DC_API_KEY", "DC_SECRET_KEY"]
99+
---
100+
101+
Authenticated instructions.`
102+
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(body), 0o644); err != nil {
103+
t.Fatal(err)
104+
}
105+
106+
r := NewRegistry(t.TempDir(), t.TempDir())
107+
RegisterLoadSkill(r, []string{filepath.Join(home, "skills")})
108+
rawArgs, err := json.Marshal(map[string]string{"name": "deepcoin-trade"})
109+
if err != nil {
110+
t.Fatal(err)
111+
}
112+
got, err := r.GetFunc("load_skill")(context.Background(), rawArgs)
113+
if err != nil {
114+
t.Fatal(err)
115+
}
116+
117+
if !strings.Contains(got, "SKILL CURRENTLY UNAVAILABLE") {
118+
t.Fatalf("load_skill output missing unavailable warning:\n%s", got)
119+
}
120+
if !strings.Contains(got, "DC_API_KEY, DC_SECRET_KEY") {
121+
t.Fatalf("load_skill output missing env names:\n%s", got)
122+
}
123+
}

0 commit comments

Comments
 (0)