fix agent tool handling and deletion

Author: idoubi

internal/agent/loop.go

@@ -719,6 +719,14 @@ func (a *Agent) meterTokens(ctx context.Context, sessionKey string, u provider.U
 // HandleMessage path. Providers that don't actually stream still work
 // — they just deliver one big chunk on Done.
 func (a *Agent) streamChatToResponse(ctx context.Context, messages []provider.Message, tools []provider.Tool) (*provider.Response, error) {
+	return a.streamChatToResponseWithOptions(ctx, messages, tools, true)
+}
+
+func (a *Agent) streamChatToResponseQuiet(ctx context.Context, messages []provider.Message, tools []provider.Tool) (*provider.Response, error) {
+	return a.streamChatToResponseWithOptions(ctx, messages, tools, false)
+}
+
+func (a *Agent) streamChatToResponseWithOptions(ctx context.Context, messages []provider.Message, tools []provider.Tool, emitDeltas bool) (*provider.Response, error) {
 	sr, err := a.provider.ChatStream(ctx, messages, tools, a.model, a.maxTokens, a.temperature)
 	if err != nil {
 		return nil, err
@@ -738,15 +746,17 @@ func (a *Agent) streamChatToResponse(ctx context.Context, messages []provider.Me
 		}
 		if chunk.Content != "" {
 			contentBuilder.WriteString(chunk.Content)
-			// Push the incremental delta. The web chat panel
-			// appends it to the bubble in progress; consumers
-			// that only know about the legacy `content` event
-			// ignore unknown types and rely on the final
-			// emit (caller's responsibility) instead.
-			emitEvent(ctx, ChatEvent{
-				Type: "content_delta",
-				Data: map[string]any{"delta": chunk.Content},
-			})
+			if emitDeltas {
+				// Push the incremental delta. The web chat panel
+				// appends it to the bubble in progress; consumers
+				// that only know about the legacy `content` event
+				// ignore unknown types and rely on the final
+				// emit (caller's responsibility) instead.
+				emitEvent(ctx, ChatEvent{
+					Type: "content_delta",
+					Data: map[string]any{"delta": chunk.Content},
+				})
+			}
 		}
 		if chunk.Done {
 			toolCalls = chunk.ToolCalls
@@ -2325,9 +2335,9 @@ func (a *Agent) HandleMessage(ctx context.Context, msg bus.InboundMessage) strin
 		finalMessages = privacy.ScrubMessages(finalMessages)
 	}
 	finalContent := ""
-	finalResp, finalErr := a.streamChatToResponse(ctx, finalMessages, nil)
+	finalResp, finalErr := a.streamChatToResponseQuiet(ctx, finalMessages, nil)
 	if finalErr == nil {
-		finalContent = finalResp.Content
+		finalContent = scrubLeakedToolCallContent(finalResp.Content)
 		a.meterTokens(ctx, sess.Key(), finalResp.Usage, 0)
 	}
 	if finalContent == "" {

internal/agent/prompt_modules.go

@@ -654,7 +654,19 @@ tool. camoufox-cli is the ONLY browser tool in this sandbox. Do NOT run
 ## Web tools
 
 - If the user gives you a URL, web_fetch it directly — don't web_search first.
+- If the user asks you to search/find/look up something, or asks for nearby
+  places, events, news, reviews, weather, prices, availability, "latest",
+  "recent", or anything else where no exact page URL was provided, call
+  web_search FIRST. Treat Chinese phrasing like "搜一下", "找一下",
+  "附近有什么", "有什么活动", "最近", and "最新" as search intent.
+- Do NOT web_fetch search result pages such as google.com/search,
+  bing.com/search, baidu.com/s, or duckduckgo.com/?q=. Put the query into
+  web_search instead, then web_fetch only a concrete result URL if needed.
 - If web_search snippets already answer the question, reply from those — don't fetch the page.
+- If web_fetch on a concrete page fails with 401/403/429, captcha,
+  anti-bot, "enable JavaScript", or an empty/blocked page, fall back to
+  camoufox-cli in the sandbox: load_skill("camoufox-cli"), open the same
+  URL, wait for the page, and extract/screenshot the visible content.
 
 ## Forbidden actions
 
@@ -842,6 +854,15 @@ Four failure modes that cost rounds:
    the https scheme and fetch the root. Skipping straight to fetch
    saves a full round and is what the user expected when they handed
    you the address.
+   For search intent — "search/find/look up", "nearby", "events",
+   "news", "reviews", "weather", "prices", "availability", "latest",
+   "recent", or Chinese phrasing like "搜一下", "找一下",
+   "附近有什么", "有什么活动", "最近", "最新" — call web_search FIRST
+   unless the user gave you an exact page URL. Do NOT synthesize a
+   search-engine URL and web_fetch it. Search result URLs such as
+   google.com/search, bing.com/search, baidu.com/s, and
+   duckduckgo.com/?q= are not sources; they are failed web_search
+   substitutes.
    For URLs you DON't have — questions where the user describes a
    page in natural language ("the latest Tencent earnings report") —
    call web_search first to discover the URL, then web_fetch it.
@@ -854,6 +875,14 @@ Four failure modes that cost rounds:
    your remaining budget — the runtime refuses retries of the same
    failed URL within this turn, so swap source, not just the path.
 
+   Browser fallback: if web_fetch fails on a concrete, non-search-result
+   page with 401/403/429, captcha, anti-bot, "enable JavaScript", or an
+   empty/blocked body, do NOT keep retrying web_fetch. Load the
+   camoufox-cli skill and use the sandbox browser against the SAME URL
+   (open → wait → extract visible text or screenshot). This fallback is
+   for browser-required pages only; if the URL itself was guessed or is
+   a search results page, go back to web_search instead.
+
 2. **Stop when you have enough.** If web_search snippets already
    contain the specific facts the user asked about (dates, numbers,
    names, yes/no answer), synthesize the answer FROM the snippets and

internal/agent/tool_recovery.go

@@ -56,6 +56,19 @@ func (a *Agent) maybeRecoverToolCalls(resp *provider.Response) {
 	resp.RawAssistant = nil
 }
 
+// scrubLeakedToolCallContent removes visible DSML / tool-call markup from
+// assistant text that is going to be shown directly to the user. It is used
+// on terminal synthesis paths where we do NOT want to execute any newly
+// recovered calls (for example after the iteration cap has already been hit),
+// but we still must not render raw `< | | DSML | | invoke ...>` garbage.
+func scrubLeakedToolCallContent(content string) string {
+	recovered, residual := recoverToolCallsFromContent(content)
+	if len(recovered) > 0 || residual != content {
+		return strings.TrimSpace(residual)
+	}
+	return content
+}
+
 // recoverToolCallsFromContent parses tool-call attempts that some open-
 // source models (DeepSeek, Qwen variants) emit as XML in the assistant
 // `content` field instead of using the OpenAI Chat Completions
@@ -195,9 +208,10 @@ var tagLeakHintRE = regexp.MustCompile(`<invoke|<\s*/?\s*[|｜]`)
 // it), so we let either noise group consume it.
 //
 // Captures:
-//   $1 = optional `/` for closing tags
-//   $2 = real tag name (invoke / parameter / tool_calls / function_calls / DSML)
-//   $3 = attributes that follow the tag name (e.g. ` name="exec"`)
+//
+//	$1 = optional `/` for closing tags
+//	$2 = real tag name (invoke / parameter / tool_calls / function_calls / DSML)
+//	$3 = attributes that follow the tag name (e.g. ` name="exec"`)
 //
 // Replacement `<${1}${2}${3}>` reconstructs `<invoke name="exec">` etc.
 // Clean inputs like `<DSML>` / `<invoke name="x">` round-trip unchanged

internal/agent/tool_recovery_test.go

@@ -184,3 +184,25 @@ func TestRecoverToolCallsFromContent(t *testing.T) {
 		})
 	}
 }
+
+func TestScrubLeakedToolCallContentForDisplay(t *testing.T) {
+	in := `< | | DSML | | tool_calls>
+< | | DSML | | invoke name="exec">
+< | | DSML | | parameter name="command" string="true">curl -sL "https://www.baidu.com/s?wd=idoubi"</ | | DSML | | parameter>
+< | | DSML | | parameter name="timeout" string="false">15</ | | DSML | | parameter>
+</ | | DSML | | invoke>
+</ | | DSML | | tool_calls>`
+	if got := scrubLeakedToolCallContent(in); got != "" {
+		t.Fatalf("scrubbed display content = %q; want empty", got)
+	}
+
+	withPreamble := "I'll check. " + in
+	got := scrubLeakedToolCallContent(withPreamble)
+	if strings.Contains(got, "DSML") || strings.Contains(got, "tool_calls") ||
+		strings.Contains(got, "invoke") || strings.Contains(got, "parameter") {
+		t.Fatalf("scrubbed display content still leaks tool markup: %q", got)
+	}
+	if strings.TrimSpace(got) != "I'll check." {
+		t.Fatalf("scrubbed display content = %q; want preamble only", got)
+	}
+}

internal/agent/tools/web_fetch.go

@@ -109,14 +109,23 @@ func init() {
 }
 
 const webFetchDescription = "Fetch a single known URL and return its plain text. " +
+	"Use this only after you already know the exact target page URL. " +
 	"If the user's message itself contains a URL or bare domain " +
 	"(e.g. 'idoubi.ai', 'https://example.com/cv'), fetch THAT URL " +
 	"directly — prepend https:// for bare domains — instead of " +
-	"running web_search first. DO NOT guess URLs from memory: " +
+	"running web_search first. For search intent like 'search/find/look up', " +
+	"'nearby', 'events', 'news', 'reviews', 'weather', 'latest', or any request " +
+	"where you do not already have a concrete page URL, call web_search first. " +
+	"Never web_fetch search result pages such as google.com/search, bing.com/search, " +
+	"baidu.com/s, or duckduckgo.com/?q=; use web_search for those queries instead. " +
+	"DO NOT guess URLs from memory: " +
 	"your training data has stale paths and you will burn rounds " +
 	"on 404s. When the user described a page in natural language " +
 	"with no URL, run web_search first to discover the URL, then " +
-	"web_fetch that exact URL. If web_search isn't available, " +
+	"web_fetch that exact URL. If web_fetch on a concrete page fails with " +
+	"401/403/429, captcha, anti-bot, or JavaScript-required output, use the " +
+	"camoufox-cli skill in the sandbox against the same URL instead of retrying " +
+	"web_fetch. If web_search isn't available, " +
 	"prefer well-known stable hosts (en.wikipedia.org, github.com), " +
 	"not date-stamped article URLs. A URL that returned 4xx/5xx " +
 	"earlier in this turn will be refused if you retry it."

internal/store/database.go

@@ -182,7 +182,7 @@ func (d *DBStore) Migrate(ctx context.Context) error {
 //
 // Empty default + partial indexes preserve existing query plans for
 // rows written before this column existed. Readers that want the
-// chatter should COALESCE(NULLIF(chatter_user_id,''), user_id) — the
+// chatter should COALESCE(NULLIF(chatter_user_id,”), user_id) — the
 // fallback is exactly right for the web channel (user_id was already
 // the chatter there) and matches the pre-fix behavior on IM (where
 // every chatter was mis-attributed to the channel owner anyway).
@@ -222,7 +222,7 @@ func (d *DBStore) migrateSessionsAddChatterUserID(ctx context.Context) error {
 }
 
 // migrateAgentGoalsAddRouting retrofits channel/account_id/chat_id/
-// project_id onto legacy agent_goals tables. All four default to ''
+// project_id onto legacy agent_goals tables. All four default to ”
 // — pre-existing rows had no continuation infrastructure attached
 // anyway, so the empty value just means "no routing recorded; can't
 // auto-continue this goal" and TryFireContinuation bails safely.
@@ -523,7 +523,7 @@ func (d *DBStore) migrateConfigsAddScopeColumn(ctx context.Context) error {
 // (user_id, agent_id) into a single lookup key: whichever is non-empty
 // wins (they're mutually exclusive for provider/setting rows — the only
 // kinds that remain in configs now that channels have their own table).
-// System rows get scope_id=''.
+// System rows get scope_id=”.
 //
 // Idempotent: skips the ALTER if the column already exists and only
 // backfills rows where scope_id is still empty.
@@ -2002,7 +2002,8 @@ func (d *DBStore) DeleteUser(ctx context.Context, id string) error {
 			return err
 		}
 		if _, err := tx.ExecContext(ctx,
-			fmt.Sprintf("DELETE FROM configs WHERE agent_id = %s", d.ph(1)), aid); err != nil {
+			fmt.Sprintf("DELETE FROM configs WHERE scope_id = %s OR scope_id LIKE %s", d.ph(1), d.ph(2)),
+			aid, "%/"+aid); err != nil {
 			return err
 		}
 	}
@@ -2272,7 +2273,16 @@ func (d *DBStore) DeleteAgent(ctx context.Context, agentID string) error {
 		return err
 	}
 	defer tx.Rollback()
-	for _, t := range []string{"agent_files", "sessions", "session_messages", "session_events", "cron_jobs"} {
+	for _, t := range []string{
+		"agent_files",
+		"sessions",
+		"session_messages",
+		"session_events",
+		"cron_jobs",
+		"projects",
+		"project_runtimes",
+		"agent_goals",
+	} {
 		if _, err := tx.ExecContext(ctx,
 			fmt.Sprintf(`DELETE FROM %s WHERE agent_id = %s`, t, d.ph(1)), agentID); err != nil {
 			return err
@@ -2282,12 +2292,11 @@ func (d *DBStore) DeleteAgent(ctx context.Context, agentID string) error {
 		fmt.Sprintf(`DELETE FROM apikey_agents WHERE agent_id = %s`, d.ph(1)), agentID); err != nil {
 		return err
 	}
-	// Drop every config row pointing at this agent — owner's official
-	// rows (user_id='', agent_id=X), agent owner's per-agent overrides
-	// (user_id=owner, agent_id=X), and any non-owner per-agent
-	// overrides (user_id=other, agent_id=X).
+	// Drop every config row pointing at this agent — official agent rows
+	// (scope_id=X) and per-user agent overrides (scope_id=user/X).
 	if _, err := tx.ExecContext(ctx,
-		fmt.Sprintf(`DELETE FROM configs WHERE agent_id = %s`, d.ph(1)), agentID); err != nil {
+		fmt.Sprintf(`DELETE FROM configs WHERE scope_id = %s OR scope_id LIKE %s`, d.ph(1), d.ph(2)),
+		agentID, "%/"+agentID); err != nil {
 		return err
 	}
 	if _, err := tx.ExecContext(ctx,
@@ -2818,7 +2827,7 @@ func (d *DBStore) ListSessionMessages(ctx context.Context, userID, agentID, sess
 //
 // Filter is strictly on chatter_user_id (no fallback to user_id). Old
 // rows written before the chatter_user_id column existed have it set
-// to '' and are not counted; those predate per-chatter resolution and
+// to ” and are not counted; those predate per-chatter resolution and
 // folding them in would over-count (they're keyed by channel owner,
 // not the actual chatter). New conversations write chatter_user_id
 // correctly so this is only a concern for sessions migrated from
@@ -3385,9 +3394,9 @@ func (d *DBStore) migrateChannelsFromConfigs(ctx context.Context) error {
 		// Each config row may have multiple accounts in its data JSON.
 		// Extract them and create one channel row per account.
 		var cc struct {
-			BotToken string                       `json:"botToken"`
-			BaseURL  string                       `json:"baseUrl"`
-			Accounts map[string]json.RawMessage   `json:"accounts"`
+			BotToken string                     `json:"botToken"`
+			BaseURL  string                     `json:"baseUrl"`
+			Accounts map[string]json.RawMessage `json:"accounts"`
 		}
 		if blob, merr := json.Marshal(cfg.Data); merr == nil {
 			_ = json.Unmarshal(blob, &cc)