v0.0.66 - Complete tools refactor, move all tools-named sources/headers into src/tools, and update build wiring
This commit is contained in:
Your Name
285
plans/tools_refactor.md
Normal file
285
plans/tools_refactor.md
Normal file
@@ -0,0 +1,285 @@
|
||||
# Plan: Refactor tools.c into Individual Tool Files
|
||||
|
||||
## Problem
|
||||
|
||||
`src/tools.c` is ~6,123 lines containing 44 `execute_*` functions, a ~650-line schema builder, shared helpers, and a dispatcher. It is the largest file in the project and difficult to navigate, review, and maintain.
|
||||
|
||||
## Goal
|
||||
|
||||
Split `src/tools.c` into focused files organized by tool group, with shared helpers in a common module. The public API (`tools.h`) remains unchanged — no callers need modification.
|
||||
|
||||
## Target File Structure
|
||||
|
||||
```
|
||||
src/
|
||||
├── tools.h # Public API (UNCHANGED)
|
||||
├── tools_internal.h # Shared internal helpers + per-tool-file function declarations
|
||||
├── tools_common.c # Shared helper implementations
|
||||
├── tools_schema.c # tools_build_openai_schema_json()
|
||||
├── tools_dispatch.c # tools_init, tools_cleanup, tools_execute dispatcher
|
||||
└── tools/
|
||||
├── tool_nostr_post.c # nostr_post, nostr_post_readme, nostr_file_md_to_longform_post
|
||||
├── tool_nostr_query.c # nostr_query
|
||||
├── tool_nostr_identity.c # nostr_pubkey, nostr_npub, nostr_encode, nostr_decode
|
||||
├── tool_nostr_social.c # nostr_react, nostr_profile_get, nostr_nip05_lookup
|
||||
├── tool_nostr_dm.c # nostr_dm_send, nostr_dm_send_nip17, nostr_encrypt, nostr_decrypt
|
||||
├── tool_nostr_relay.c # nostr_relay_status, nostr_relay_info
|
||||
├── tool_nostr_list.c # nostr_list_manage, nostr_delete
|
||||
├── tool_skill.c # skill_create, skill_list, skill_adopt, skill_remove, skill_edit, skill_search
|
||||
├── tool_task.c # task_list, task_manage
|
||||
├── tool_local.c # local_shell_exec, local_file_read, local_file_write, local_http_fetch
|
||||
├── tool_admin.c # admin_identity, nostr_admin_profile, nostr_admin_contacts, nostr_admin_relays, nostr_admin_notes
|
||||
├── tool_agent.c # agent_identity, agent_version, message_current
|
||||
├── tool_model.c # model_get, model_set, model_list
|
||||
└── tool_meta.c # tool_list, trigger_list
|
||||
```
|
||||
|
||||
## Shared Helpers → tools_common.c
|
||||
|
||||
These helpers are used by multiple tool groups and must be non-static in `tools_common.c`, declared in `tools_internal.h`:
|
||||
|
||||
| Helper | Current lines | Used by |
|
||||
|---|---|---|
|
||||
| `json_error()` | 27-35 | ALL tools |
|
||||
| `repair_json_control_chars()` | 37-101 | nostr_post, nostr_post_readme |
|
||||
| `find_key_start()` | 103-111 | nostr_post loose parser |
|
||||
| `skip_ws()` | 113-116 | parse_tool_args_json, loose parser |
|
||||
| `parse_loose_kind()` | 118-135 | nostr_post |
|
||||
| `parse_loose_json_string_value()` | 137-186 | nostr_post |
|
||||
| `parse_loose_nostr_post_args()` | 188-212 | nostr_post |
|
||||
| `ensure_tags_array()` | 214-227 | nostr_post |
|
||||
| `has_tag_key()` | 229-243 | longform post helpers |
|
||||
| `add_string_tag()` | 245-255 | nostr_post, delete, react, skill, longform |
|
||||
| `remove_tag_key_all()` | 257-270 | upsert_string_tag |
|
||||
| `upsert_string_tag()` | 272-276 | skill_edit |
|
||||
| `parse_tool_args_json()` | 590-564 | ALL tools |
|
||||
| `is_hex_string_len()` | 581-588 | nostr tools, skill tools, DM tools |
|
||||
| `find_tag_value_string()` | 855-872 | skill tools, list_manage, longform |
|
||||
| `tag_tuple_equal()` | 813-828 | skill tools, list_manage |
|
||||
| `tags_contains_tuple()` | 830-840 | skill tools, list_manage |
|
||||
| `remove_matching_tag_tuples()` | 842-853 | skill tools, list_manage |
|
||||
| `build_tool_path()` | (used by local tools + readme) | local tools, readme |
|
||||
|
||||
## Tool-Local Helpers (stay static in their tool file)
|
||||
|
||||
| Helper | Destination |
|
||||
|---|---|
|
||||
| `validate_skill_d_tag()` | `tool_skill.c` |
|
||||
| `fetch_adoption_list_tags()` | `tool_skill.c` |
|
||||
| `publish_adoption_list()` | `tool_skill.c` |
|
||||
| `adoption_tags_contains_address()` | `tool_skill.c` |
|
||||
| `extract_skill_summary()` | `tool_skill.c` |
|
||||
| `ci_contains()` | `tool_skill.c` |
|
||||
| `d_tagify_string()` | `tool_nostr_post.c` |
|
||||
| `first_markdown_h1()` | `tool_nostr_post.c` |
|
||||
| `first_markdown_paragraph()` | `tool_nostr_post.c` |
|
||||
| `trim_copy()` | `tool_nostr_post.c` |
|
||||
| `auto_enrich_longform_tags()` | `tool_nostr_post.c` |
|
||||
| `basename_lowercase_dup()` | `tool_nostr_post.c` |
|
||||
| `detect_ca_bundle_path_for_tools()` | `tool_local.c` |
|
||||
| `local_http_fetch_write_cb()` | `tool_local.c` |
|
||||
| `persist_llm_config()` | `tool_model.c` |
|
||||
|
||||
## Tool Function Signatures
|
||||
|
||||
Each tool file exposes its `execute_*` functions via `tools_internal.h`. Two signature patterns exist:
|
||||
|
||||
```c
|
||||
// Pattern A: context-free (only needs args)
|
||||
char* execute_nostr_post(const char* args_json);
|
||||
|
||||
// Pattern B: context-dependent
|
||||
char* execute_skill_create(tools_context_t* ctx, const char* args_json);
|
||||
```
|
||||
|
||||
## tools_internal.h Structure
|
||||
|
||||
```c
|
||||
#ifndef DIDACTYL_TOOLS_INTERNAL_H
|
||||
#define DIDACTYL_TOOLS_INTERNAL_H
|
||||
|
||||
#include "tools.h"
|
||||
#include "cjson/cJSON.h"
|
||||
|
||||
// ─── Shared helpers (tools_common.c) ───
|
||||
char* json_error(const char* msg);
|
||||
cJSON* parse_tool_args_json(const char* args_json);
|
||||
int is_hex_string_len(const char* s, size_t expected_len);
|
||||
int add_string_tag(cJSON* tags, const char* key, const char* value);
|
||||
int remove_tag_key_all(cJSON* tags, const char* key);
|
||||
int upsert_string_tag(cJSON* tags, const char* key, const char* value);
|
||||
int has_tag_key(cJSON* tags, const char* key);
|
||||
cJSON* ensure_tags_array(cJSON** tags_inout);
|
||||
cJSON* find_tag_value_string(cJSON* tags, const char* key);
|
||||
int tag_tuple_equal(cJSON* a, cJSON* b);
|
||||
int tags_contains_tuple(cJSON* tags, cJSON* tuple);
|
||||
int remove_matching_tag_tuples(cJSON* tags, cJSON* tuple);
|
||||
char* repair_json_control_chars(const char* in);
|
||||
cJSON* parse_loose_nostr_post_args(const char* in);
|
||||
int build_tool_path(tools_context_t* ctx, const char* rel, char* out, size_t out_size);
|
||||
|
||||
// ─── Per-tool-file execute functions ───
|
||||
|
||||
// tool_nostr_post.c
|
||||
char* execute_nostr_post(const char* args_json);
|
||||
char* execute_nostr_post_readme(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_file_md_to_longform_post(tools_context_t* ctx, const char* args_json);
|
||||
|
||||
// tool_nostr_query.c
|
||||
char* execute_nostr_query(const char* args_json);
|
||||
|
||||
// tool_nostr_identity.c
|
||||
char* execute_nostr_pubkey(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_npub(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_encode(const char* args_json);
|
||||
char* execute_nostr_decode(const char* args_json);
|
||||
|
||||
// tool_nostr_social.c
|
||||
char* execute_nostr_react(const char* args_json);
|
||||
char* execute_nostr_profile_get(const char* args_json);
|
||||
char* execute_nostr_nip05_lookup(const char* args_json);
|
||||
|
||||
// tool_nostr_dm.c
|
||||
char* execute_nostr_dm_send(const char* args_json);
|
||||
char* execute_nostr_dm_send_nip17(const char* args_json);
|
||||
char* execute_nostr_encrypt(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_decrypt(tools_context_t* ctx, const char* args_json);
|
||||
|
||||
// tool_nostr_relay.c
|
||||
char* execute_nostr_relay_status(const char* args_json);
|
||||
char* execute_nostr_relay_info(const char* args_json);
|
||||
|
||||
// tool_nostr_list.c
|
||||
char* execute_nostr_list_manage(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_delete(const char* args_json);
|
||||
|
||||
// tool_skill.c
|
||||
char* execute_skill_create(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_skill_list(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_skill_adopt(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_skill_remove(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_skill_edit(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_skill_search(const char* args_json);
|
||||
|
||||
// tool_task.c
|
||||
char* execute_task_list(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_task_manage(tools_context_t* ctx, const char* args_json);
|
||||
|
||||
// tool_local.c
|
||||
char* execute_local_http_fetch(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_local_shell_exec(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_local_file_read(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_local_file_write(tools_context_t* ctx, const char* args_json);
|
||||
|
||||
// tool_admin.c
|
||||
char* execute_admin_identity(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_admin_profile(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_admin_contacts(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_admin_relays(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_nostr_admin_notes(tools_context_t* ctx, const char* args_json);
|
||||
|
||||
// tool_agent.c
|
||||
char* execute_agent_identity(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_agent_version(const char* args_json);
|
||||
char* execute_message_current(tools_context_t* ctx, const char* args_json);
|
||||
|
||||
// tool_model.c
|
||||
char* execute_model_get(const char* args_json);
|
||||
char* execute_model_set(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_model_list(const char* args_json);
|
||||
|
||||
// tool_meta.c
|
||||
char* execute_tool_list(tools_context_t* ctx, const char* args_json);
|
||||
char* execute_trigger_list(tools_context_t* ctx, const char* args_json);
|
||||
|
||||
#endif
|
||||
```
|
||||
|
||||
## Makefile Changes
|
||||
|
||||
Replace the single `$(SRC_DIR)/tools.c` entry with:
|
||||
|
||||
```makefile
|
||||
SRCS = \
|
||||
$(SRC_DIR)/main.c \
|
||||
$(SRC_DIR)/config.c \
|
||||
$(SRC_DIR)/context.c \
|
||||
$(SRC_DIR)/llm.c \
|
||||
$(SRC_DIR)/nostr_handler.c \
|
||||
$(SRC_DIR)/agent.c \
|
||||
$(SRC_DIR)/tools_common.c \
|
||||
$(SRC_DIR)/tools_schema.c \
|
||||
$(SRC_DIR)/tools_dispatch.c \
|
||||
$(SRC_DIR)/tools/tool_nostr_post.c \
|
||||
$(SRC_DIR)/tools/tool_nostr_query.c \
|
||||
$(SRC_DIR)/tools/tool_nostr_identity.c \
|
||||
$(SRC_DIR)/tools/tool_nostr_social.c \
|
||||
$(SRC_DIR)/tools/tool_nostr_dm.c \
|
||||
$(SRC_DIR)/tools/tool_nostr_relay.c \
|
||||
$(SRC_DIR)/tools/tool_nostr_list.c \
|
||||
$(SRC_DIR)/tools/tool_skill.c \
|
||||
$(SRC_DIR)/tools/tool_task.c \
|
||||
$(SRC_DIR)/tools/tool_local.c \
|
||||
$(SRC_DIR)/tools/tool_admin.c \
|
||||
$(SRC_DIR)/tools/tool_agent.c \
|
||||
$(SRC_DIR)/tools/tool_model.c \
|
||||
$(SRC_DIR)/tools/tool_meta.c \
|
||||
$(SRC_DIR)/trigger_manager.c \
|
||||
$(SRC_DIR)/prompt_template.c \
|
||||
$(SRC_DIR)/http_api.c \
|
||||
$(SRC_DIR)/mongoose.c \
|
||||
$(SRC_DIR)/debug.c
|
||||
```
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
### Phase 1: Create shared infrastructure
|
||||
|
||||
- [ ] Create `src/tools_internal.h` with shared helper declarations and all `execute_*` prototypes
|
||||
- [ ] Create `src/tools_common.c` — move shared helpers from `tools.c`, remove `static`, add `#include "tools_internal.h"`
|
||||
- [ ] Create `src/tools_schema.c` — move `tools_build_openai_schema_json()` from `tools.c`
|
||||
- [ ] Create `src/tools_dispatch.c` — move `tools_init()`, `tools_cleanup()`, `tools_execute()` from `tools.c`
|
||||
- [ ] Update `Makefile` SRCS to include new files (keep `tools.c` temporarily with remaining functions)
|
||||
- [ ] Build and verify — all tests pass, binary runs
|
||||
|
||||
### Phase 2: Extract tool files one at a time
|
||||
|
||||
Each step: move `execute_*` functions + their local helpers into the new file, remove from `tools.c`, build, verify.
|
||||
|
||||
- [ ] Create `src/tools/` directory
|
||||
- [ ] Extract `src/tools/tool_nostr_post.c` — `execute_nostr_post`, `execute_nostr_post_readme`, `execute_nostr_file_md_to_longform_post` + local helpers: `d_tagify_string`, `first_markdown_h1`, `first_markdown_paragraph`, `trim_copy`, `auto_enrich_longform_tags`, `basename_lowercase_dup`
|
||||
- [ ] Extract `src/tools/tool_nostr_query.c` — `execute_nostr_query`
|
||||
- [ ] Extract `src/tools/tool_nostr_identity.c` — `execute_nostr_pubkey`, `execute_nostr_npub`, `execute_nostr_encode`, `execute_nostr_decode`
|
||||
- [ ] Extract `src/tools/tool_nostr_social.c` — `execute_nostr_react`, `execute_nostr_profile_get`, `execute_nostr_nip05_lookup`
|
||||
- [ ] Extract `src/tools/tool_nostr_dm.c` — `execute_nostr_dm_send`, `execute_nostr_dm_send_nip17`, `execute_nostr_encrypt`, `execute_nostr_decrypt`
|
||||
- [ ] Extract `src/tools/tool_nostr_relay.c` — `execute_nostr_relay_status`, `execute_nostr_relay_info`
|
||||
- [ ] Extract `src/tools/tool_nostr_list.c` — `execute_nostr_list_manage`, `execute_nostr_delete`
|
||||
- [ ] Extract `src/tools/tool_skill.c` — `execute_skill_create`, `execute_skill_list`, `execute_skill_adopt`, `execute_skill_remove`, `execute_skill_edit`, `execute_skill_search` + local helpers: `validate_skill_d_tag`, `ci_contains`, `fetch_adoption_list_tags`, `publish_adoption_list`, `adoption_tags_contains_address`, `extract_skill_summary`
|
||||
- [ ] Extract `src/tools/tool_task.c` — `execute_task_list`, `execute_task_manage`
|
||||
- [ ] Extract `src/tools/tool_local.c` — `execute_local_http_fetch`, `execute_local_shell_exec`, `execute_local_file_read`, `execute_local_file_write` + local helpers: `detect_ca_bundle_path_for_tools`, `local_http_fetch_write_cb`, `free_string_array_heap`
|
||||
- [ ] Extract `src/tools/tool_admin.c` — `execute_admin_identity`, `execute_nostr_admin_profile`, `execute_nostr_admin_contacts`, `execute_nostr_admin_relays`, `execute_nostr_admin_notes`
|
||||
- [ ] Extract `src/tools/tool_agent.c` — `execute_agent_identity`, `execute_agent_version`, `execute_message_current`
|
||||
- [ ] Extract `src/tools/tool_model.c` — `execute_model_get`, `execute_model_set`, `execute_model_list` + local helper: `persist_llm_config`
|
||||
- [ ] Extract `src/tools/tool_meta.c` — `execute_tool_list`, `execute_trigger_list`
|
||||
|
||||
### Phase 3: Cleanup
|
||||
|
||||
- [ ] Delete `src/tools.c` (now empty — all code moved)
|
||||
- [ ] Remove `src/tools.c` from Makefile SRCS
|
||||
- [ ] Final build + full test run
|
||||
- [ ] Verify binary size is comparable (no accidental code duplication)
|
||||
- [ ] Increment version and push
|
||||
|
||||
## Validation Criteria
|
||||
|
||||
After each extraction step:
|
||||
1. `make -j4` compiles with zero warnings
|
||||
2. `./didactyl --debug 5` starts and runs normally
|
||||
3. Tool execution via DM or `--test-tool` produces identical results
|
||||
|
||||
## Risk Mitigation
|
||||
|
||||
- **Build after every file extraction** — catch issues immediately
|
||||
- **No logic changes** — purely mechanical code movement
|
||||
- **Keep `tools.h` unchanged** — zero impact on callers (`agent.c`, `http_api.c`, `main.c`)
|
||||
- **Git commit after each phase** — easy rollback points
|
||||
Reference in New Issue
Block a user