mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Use short hex IDs for subagents instead of UUIDs (#128)
This PR updates subagent ID generation to use short 8-character hex
strings instead of full UUID v4 strings. Previously, subagent IDs were
36-character UUIDs (e.g. `550e8400-e29b-41d4-a716-446655440000`), which
were verbose in CLI output and unwieldy when the LLM needed to reference
them in tools like `send_input`, `wait`, and `close_agent`. The new
format generates IDs like `a3f1b20c` — compact, human-readable, and with
~4 billion possible values, effectively collision-free within a session.
The change is made at the source in `subagent.rs`, where UUID generation
is replaced with `format!("{:08x}",
uuid::Uuid::new_v4().as_fields().0)`. Because IDs are now inherently 8
characters, the display-layer truncations in `cli.rs` (5 occurrences)
and `run_progress.rs` (2 occurrences) are redundant and have been
removed — `agent_id` is used directly in format strings instead of a
`short_id` slice.
### Plan Summary
- **Replace UUID generation** in `subagent.rs`: use the first field of a
UUID v4 formatted as 8-char lowercase hex, yielding IDs like `a3f1b20c`
instead of full 36-char UUIDs
- **Remove `short_id` truncation** in `cli.rs` (5 places) and
`run_progress.rs` (2 places): since IDs are now already 8 chars, the
`let short_id = &agent_id[..8.min(agent_id.len())]` pattern is
eliminated and `{agent_id}` is used directly in all format strings
- No test changes required — existing tests use hardcoded IDs like
`"sa-1"` and don't assert on ID length or format
<details>
<summary>Full plan</summary>
````md
The plan has been written to `/home/daytona/workspace/plan.md`.
It covers:
- **4 files to modify**: `fabro-agent/Cargo.toml` (add `rand` dep), `subagent.rs` (replace UUID with 8-char hex), `cli.rs` (remove 5 `short_id` truncations), `run_progress.rs` (remove 2 `short_id` truncations)
- **Step-by-step implementation** with exact line references and before/after code
- **Verification commands** to confirm correctness
- **Test case analysis** explaining why no test changes are needed
````
</details>
### Fabro Details
<details>
<summary>Ran 3 stages in 18m 46s for $0.57</summary>
| Stage | Duration | Cost | Retries |
|---|---|---|---|
| start | 0s | – | 0 |
| plan | 1m 28s | $0.57 | 0 |
| implement | 17m 6s | – | 0 |
| **Total** | **18m 46s** | **$0.57** | **0** |
</details>
<details>
<summary>Ran <code>GhImplement.fabro</code> (4 nodes and 3
edges)</summary>
```dot
digraph GhImplement {
graph [
goal="Implement a GitHub issue",
model_stylesheet="
* { model: claude-opus-4-6; }
"
]
rankdir=LR
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
plan [label="Plan", prompt="Fetch the GitHub issue from the goal using: gh issue view $goal --json title,body,labels,comments\n\nRead the issue title, description, and any comments carefully. Analyze what code changes are needed to resolve the issue.\n\nWrite a detailed implementation plan to plan.md that includes:\n- Summary of the issue\n- Files to create or modify\n- Step-by-step implementation approach\n- Test cases to add or update\n\nThe plan should be specific enough for another agent to implement without seeing the original issue.\n\nRespond with the location of the plan file (plan.md)."]
implement [label="Implement", shape=house, stack.child_workflow="fabro/workflows/implement/workflow.fabro", manager.max_cycles=100]
start -> plan
plan -> implement [fidelity="summary:high"]
implement -> exit
}
```
</details>
⚒️ Generated with [Fabro](https://fabro.sh)
---------
Co-authored-by: Fabro <noreply@fabro.sh>
This commit is contained in:
parent
77064677d5
commit
c119bf3c79
4 changed files with 79 additions and 15 deletions
|
|
@ -539,7 +539,6 @@ pub async fn run_with_args_and_client(
|
|||
task,
|
||||
..
|
||||
} => {
|
||||
let short_id = &agent_id[..8.min(agent_id.len())];
|
||||
let task_preview = if task.len() > 60 {
|
||||
&task[..crate::truncation::floor_char_boundary(task, 60)]
|
||||
} else {
|
||||
|
|
@ -548,7 +547,7 @@ pub async fn run_with_args_and_client(
|
|||
eprintln!(
|
||||
" {}",
|
||||
s.dim.apply_to(format!(
|
||||
"\u{25b6} subagent {short_id} spawned (depth={depth}) task={task_preview:?}"
|
||||
"\u{25b6} subagent {agent_id} spawned (depth={depth}) task={task_preview:?}"
|
||||
)),
|
||||
);
|
||||
}
|
||||
|
|
@ -558,11 +557,10 @@ pub async fn run_with_args_and_client(
|
|||
success,
|
||||
turns_used,
|
||||
} => {
|
||||
let short_id = &agent_id[..8.min(agent_id.len())];
|
||||
eprintln!(
|
||||
" {}",
|
||||
s.dim.apply_to(format!(
|
||||
"\u{25a0} subagent {short_id} completed (depth={depth}, success={success}, turns={turns_used})"
|
||||
"\u{25a0} subagent {agent_id} completed (depth={depth}, success={success}, turns={turns_used})"
|
||||
)),
|
||||
);
|
||||
}
|
||||
|
|
@ -571,20 +569,18 @@ pub async fn run_with_args_and_client(
|
|||
depth,
|
||||
error,
|
||||
} => {
|
||||
let short_id = &agent_id[..8.min(agent_id.len())];
|
||||
eprintln!(
|
||||
" {}",
|
||||
s.red.apply_to(format!(
|
||||
"\u{2717} subagent {short_id} failed (depth={depth}): {error}"
|
||||
"\u{2717} subagent {agent_id} failed (depth={depth}): {error}"
|
||||
)),
|
||||
);
|
||||
}
|
||||
AgentEvent::SubAgentClosed { agent_id, depth } => {
|
||||
let short_id = &agent_id[..8.min(agent_id.len())];
|
||||
eprintln!(
|
||||
" {}",
|
||||
s.dim.apply_to(format!(
|
||||
"\u{25a0} subagent {short_id} closed (depth={depth})"
|
||||
"\u{25a0} subagent {agent_id} closed (depth={depth})"
|
||||
)),
|
||||
);
|
||||
}
|
||||
|
|
@ -593,11 +589,10 @@ pub async fn run_with_args_and_client(
|
|||
event: child_event,
|
||||
..
|
||||
} if verbose => {
|
||||
let short_id = &agent_id[..8.min(agent_id.len())];
|
||||
eprintln!(
|
||||
" {}",
|
||||
s.dim
|
||||
.apply_to(format!("[subagent {short_id}] {child_event:?}")),
|
||||
.apply_to(format!("[subagent {agent_id}] {child_event:?}")),
|
||||
);
|
||||
}
|
||||
_ => {}
|
||||
|
|
|
|||
|
|
@ -64,7 +64,7 @@ impl SubAgentManager {
|
|||
)));
|
||||
}
|
||||
|
||||
let agent_id = uuid::Uuid::new_v4().to_string();
|
||||
let agent_id = format!("{:08x}", uuid::Uuid::new_v4().as_fields().0);
|
||||
let followup_queue = session.followup_queue_handle();
|
||||
let cancel_token = session.cancel_token();
|
||||
|
||||
|
|
|
|||
|
|
@ -1419,11 +1419,10 @@ impl ProgressUI {
|
|||
}
|
||||
AgentEvent::SubAgentSpawned { agent_id, task, .. } if self.verbose => {
|
||||
let dim = Style::new().dim();
|
||||
let short_id = &agent_id[..agent_id.len().min(8)];
|
||||
self.insert_subagent_line_for_stage(
|
||||
stage_node_id,
|
||||
&dim.apply_to(format!(
|
||||
"\u{25b8} subagent[{short_id}] \"{}\"",
|
||||
"\u{25b8} subagent[{agent_id}] \"{}\"",
|
||||
truncate(task, 50)
|
||||
))
|
||||
.to_string(),
|
||||
|
|
@ -1435,11 +1434,10 @@ impl ProgressUI {
|
|||
success,
|
||||
..
|
||||
} if self.verbose => {
|
||||
let short_id = &agent_id[..agent_id.len().min(8)];
|
||||
let glyph = if *success { green_check() } else { red_cross() };
|
||||
self.insert_subagent_line_for_stage(
|
||||
stage_node_id,
|
||||
&format!("{glyph} subagent[{short_id}] ({turns_used} turns)"),
|
||||
&format!("{glyph} subagent[{agent_id}] ({turns_used} turns)"),
|
||||
);
|
||||
}
|
||||
_ => {}
|
||||
|
|
|
|||
71
plan.md
Normal file
71
plan.md
Normal file
|
|
@ -0,0 +1,71 @@
|
|||
# Plan: Use short hex IDs for subagents instead of UUIDs
|
||||
|
||||
## Summary
|
||||
|
||||
GitHub issue #126: Subagent IDs are currently full UUID v4 strings (36 chars, e.g. `550e8400-e29b-41d4-a716-446655440000`). These are verbose in CLI output and error-prone when referenced by the LLM in tools like `send_input`, `wait`, and `close_agent`. The codebase already truncates agent IDs to 8 chars for display in multiple places. This change replaces UUID generation with a random 8-char hex string (e.g. `a3f1b20c`) so the full ID matches what's already displayed.
|
||||
|
||||
## Files to modify
|
||||
|
||||
### 1. `lib/crates/fabro-agent/Cargo.toml`
|
||||
- Add `rand.workspace = true` to `[dependencies]`. (`rand = "0.8"` is already defined in workspace root `Cargo.toml`; other crates like `fabro-cli`, `fabro-llm`, `fabro-workflows` already use it.)
|
||||
- Do **not** remove `uuid.workspace = true` — it's still used in `session.rs:58` for session IDs.
|
||||
|
||||
### 2. `lib/crates/fabro-agent/src/subagent.rs`
|
||||
- **Line 67**: Replace `uuid::Uuid::new_v4().to_string()` with `format!("{:08x}", rand::random::<u32>())`.
|
||||
- This generates 8-char lowercase hex strings with ~4 billion possible values — collision-free within a session.
|
||||
|
||||
### 3. `lib/crates/fabro-agent/src/cli.rs`
|
||||
Remove the `short_id` truncation pattern in 5 places. Since IDs are now 8 chars, `short_id == agent_id`, so use `agent_id` directly:
|
||||
|
||||
- **Line 542** (`SubAgentSpawned` handler): Remove `let short_id = &agent_id[..8.min(agent_id.len())];` and replace `{short_id}` with `{agent_id}` in the format string.
|
||||
- **Line 561** (`SubAgentCompleted` handler): Same removal and replacement.
|
||||
- **Line 574** (`SubAgentFailed` handler): Same removal and replacement.
|
||||
- **Line 583** (`SubAgentClosed` handler): Same removal and replacement.
|
||||
- **Line 596** (`SubAgentEvent` handler, verbose mode): Same removal and replacement.
|
||||
|
||||
### 4. `lib/crates/fabro-cli/src/commands/run_progress.rs`
|
||||
Remove the `short_id` truncation pattern in 2 places:
|
||||
|
||||
- **Line 1416** (`SubAgentSpawned` handler): Remove `let short_id = &agent_id[..agent_id.len().min(8)];` and replace `{short_id}` with `{agent_id}` in the format string.
|
||||
- **Line 1432** (`SubAgentCompleted` handler): Same removal and replacement.
|
||||
|
||||
## Step-by-step implementation
|
||||
|
||||
1. **Add `rand` dependency to `fabro-agent`**:
|
||||
In `lib/crates/fabro-agent/Cargo.toml`, add `rand.workspace = true` to the `[dependencies]` section (e.g. after the `uuid.workspace = true` line).
|
||||
|
||||
2. **Replace UUID generation in `subagent.rs`**:
|
||||
In `lib/crates/fabro-agent/src/subagent.rs` line 67, change:
|
||||
```rust
|
||||
let agent_id = uuid::Uuid::new_v4().to_string();
|
||||
```
|
||||
to:
|
||||
```rust
|
||||
let agent_id = format!("{:08x}", rand::random::<u32>());
|
||||
```
|
||||
|
||||
3. **Remove `short_id` truncation in `cli.rs`**:
|
||||
In `lib/crates/fabro-agent/src/cli.rs`, for each of the 5 occurrences of `let short_id = &agent_id[..8.min(agent_id.len())];` (lines 542, 561, 574, 583, 596):
|
||||
- Delete the `let short_id = ...` line.
|
||||
- Replace `{short_id}` with `{agent_id}` in the corresponding format string on the same match arm.
|
||||
|
||||
4. **Remove `short_id` truncation in `run_progress.rs`**:
|
||||
In `lib/crates/fabro-cli/src/commands/run_progress.rs`, for each of the 2 occurrences of `let short_id = &agent_id[..agent_id.len().min(8)];` (lines 1416, 1432):
|
||||
- Delete the `let short_id = ...` line.
|
||||
- Replace `{short_id}` with `{agent_id}` in the corresponding format string.
|
||||
|
||||
## Verification
|
||||
|
||||
- `cargo build --workspace` — clean build with no errors.
|
||||
- `cargo test -p fabro-agent` — all existing subagent tests pass. Tests use hardcoded IDs like `"sa-1"`, not UUIDs, so no test changes needed.
|
||||
- `cargo clippy --workspace -- -D warnings` — no new warnings.
|
||||
- `cargo fmt --check --all` — formatting is clean.
|
||||
|
||||
## Test cases
|
||||
|
||||
No new test cases are needed. The existing tests in `subagent.rs` (e.g. `spawn_creates_agent_and_returns_id`) already verify that:
|
||||
- `agent_id` is non-empty
|
||||
- The agent can be looked up by its ID
|
||||
- Spawn/wait/close/send_input work with the generated IDs
|
||||
|
||||
The generated IDs will now be 8 chars instead of 36, but the tests don't assert on length or format, so they pass unchanged.
|
||||
Loading…
Add table
Reference in a new issue