Skip to content

feat(tools): add vault tool group to policy#982

Closed
nguyennguyenit wants to merge 5 commits intomainfrom
feat/vault-tool-group
Closed

feat(tools): add vault tool group to policy#982
nguyennguyenit wants to merge 5 commits intomainfrom
feat/vault-tool-group

Conversation

@nguyennguyenit
Copy link
Copy Markdown
Contributor

Summary

  • Add vault tool group containing vault_search and vault_read
  • Include group:vault in messaging and coding profiles
  • Enables profile-restricted agents (e.g. Telegram bots) to access vault documents

Problem

Agents using messaging profile couldn't access vault_search/vault_read tools, causing them to hallucinate file paths instead of searching vault first.

Solution

Create dedicated vault group and add to relevant profiles. This also enables per-agent Tool Policy configuration via group:vault.

Test plan

  • go build ./... passes
  • go test ./internal/tools/... -run Policy passes

viettranx and others added 5 commits April 20, 2026 08:49
- Add ZAI/GLM context overflow patterns to error_classify.go
- ThinkStage detects overflow, triggers emergency compaction + 1 retry
- Wire ReserveTokensFloor config to pipeline budget calculation
- Send user-friendly error on channel RunFailed events
- Add sessions.compact WebSocket method for manual truncation
- Fix Accept header for JSON response negotiation (Shopee GETs)
- Platform-prefix-aware pageID parsing (spo_ convID format)
- 500-char message limit for Shopee DMs
- Rune-safe truncation with slog.Warn on truncation

Phase 3 (integration smoke) pending: requires real credentials.
Add "vault" group containing vault_search and vault_read tools.
Include group:vault in messaging and coding profiles so agents
using these profiles can search and read vault documents.

Previously, vault tools were only in the goclaw composite group
but not exposed to profile-restricted agents like Telegram bots.
Add MethodSessionsCompact to isWriteMethod to fix drift coverage test.
Method was added in #958 but missing from permissions classification.
Use strings.Contains from stdlib instead of local contains helper
to avoid redeclaration conflict with mcp_grant_revoke_test.go.
@clark-cant
Copy link
Copy Markdown

🔍 Code Review — feat(tools): add vault tool group to policy

🎯 Tổng quan

PR này thực chất là 5 commits gộp nhiều tính năng:

  1. Vault tool group policy (PR title)
  2. Context overflow recovery (Bug: long-lived chat sessions can hit model context limits before GoClaw compacts them #958) — emergency compaction + retry
  3. Shopee platform support (from feat(pancake): add Shopee platform support #975)
  4. User-friendly error messages for RunFailed events
  5. sessions.compact method + RBAC fix + test cleanup

Scope: 24 files, ~800+ additions


✅ Điểm Tốt

  1. Vault tool group — clean, minimal change. Thêm group:vault vào codingmessaging profiles đúng chỗ. Giúp profile-restricted agents (Telegram bots) access vault docs thay vì hallucinate file paths.

  2. Context overflow recovery — design đúng hướng:

    • Detect overflow patterns đa provider (ZAI/GLM, DashScope, Qwen Chinese patterns)
    • Emergency compaction + 1 retry (max) → tránh infinite loop
    • OverflowRetries counter reset on success
    • Graceful fallback khi không có CompactMessages callback
    • ReserveTokensFloor wired từ config → pipeline budget
  3. FormatAgentError — user-friendly messages cho từng error type (overflow, rate limit, API key issues, timeout, overload). Published trên RunFailed event thay vì silent "...".

  4. sessions.compact method — truncate-only (no LLM summarization), permission check đúng, audit log, default keepLast=4.

  5. Shopee support — đã review chi tiết ở feat(pancake): add Shopee platform support #975, code sạch.


⚠️ Issues Cần Lưu Ý

1. [HIGH] — PR squash quá nhiều tính năng độc lập

5 commits = 5 tính năng riêng biệt gộp vô 1 PR. Nếu có bug ở Shopee code, phải revert toàn bộ PR → mất cả vault policy + overflow recovery.

Fix đề xuất: Split thành separate PRs. Ít nhất 3 PRs: vault policy, overflow recovery, Shopee.

2. [MEDIUM] — isContextOverflowErr duplicate pattern matching

internal/pipeline/think_stage.gocmd/gateway_errors.go isContextOverflowError cũng dùng pattern tương tự nhưng không reuse `providers.IsContextOverflowMessage]. Hai chỗ này có thể drift theo thời gian.

Fix đề xuất: Gateway errors nên reuse `providers.IsContextOverflowMessage] thay vì duplicate patterns.

3. [MEDIUM] — sessions.compact không có guard cho concurrent calls

Nếu 2 clients gọi cùng lúc cho cùng session, có thể race condition giữa TruncateHistory] và Save]. Cần mutex hoặc optimistic locking.

4. [LOW] — FormatAgentError messages hard-coded English

Tất cả messages đều tiếng Anh cứng. Nếu user dùng Telegram bot tiếng Việt, sẽ thấy tiếng Anh. Nên dùng i18n lookup thay vì hard-coded strings.


📊 Summary

Severity Count Issues
🔴 HIGH 1 PR squash quá nhiều tính năng độc lập
🟡 MEDIUM 2 Duplicate overflow patterns; no concurrent guard for sessions.compact
🟢 LOW 1 FormatAgentError hard-coded English

💡 Recommendation

🔴 REQUEST CHANGES

Code quality tốt, từng tính năng implement đúng hướng. Nhưng PR quá lớn và squash nhiều features độc lập — nên split ra 3 PRs riêng để dễ review, dễ revert nếu có issue. Sau khi split, từng PR nhỏ sẽ dễ APPROVE.

Great work @nguyennguyenit! 🚀

@nguyennguyenit
Copy link
Copy Markdown
Contributor Author

Superseded by #984 - clean PR with only vault tool group changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants