mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-07 08:27:12 +00:00
Fabro
parent
2f1c0e85ed
commit
9fb187cc8f
3 changed files with 117 additions and 12 deletions
|
|
@ -1,30 +1,38 @@
|
|||
{
|
||||
"timestamp": "2026-03-20T23:27:47.483031Z",
|
||||
"current_node": "plan",
|
||||
"timestamp": "2026-03-20T23:44:56.811200Z",
|
||||
"current_node": "implement",
|
||||
"completed_nodes": [
|
||||
"start",
|
||||
"plan"
|
||||
"plan",
|
||||
"implement"
|
||||
],
|
||||
"node_retries": {
|
||||
"start": 1,
|
||||
"plan": 1
|
||||
"plan": 1,
|
||||
"implement": 1
|
||||
},
|
||||
"context_values": {
|
||||
"last_stage": "plan",
|
||||
"current_node": "plan",
|
||||
"last_stage": "simplify_opus",
|
||||
"current_node": "implement",
|
||||
"graph.model_stylesheet": "\n * { model: claude-opus-4-6; }\n ",
|
||||
"current.preamble": "Goal: https://github.com/fabro-sh/fabro/issues/126\n",
|
||||
"current.preamble": "Goal: https://github.com/fabro-sh/fabro/issues/126\nRun ID: 01KM6S46Q7M6ZV6KP05576W499\nPipeline progress: 1 of 2 stages completed\n\n## Stage: plan\n- Status: success\n- Handler: agent\n- Model: claude-opus-4-6\n- Tokens: 25.0k in / 2.5k out\n- Files touched: /home/daytona/workspace/plan.md\n- Response:\n > The plan has been written to `/home/daytona/workspace/plan.md`.\n > \n > It covers:\n > - **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)\n > - **Step-by-step implementation** with exact line references and before/after code\n > - **Verification commands** to confirm correctness\n > - **Test case analysis** explaining why no test changes are needed\n",
|
||||
"graph.rankdir": "LR",
|
||||
"internal.retry_count.start": 1,
|
||||
"internal.fidelity": "compact",
|
||||
"internal.thread_id": "start",
|
||||
"internal.fidelity": "summary:high",
|
||||
"internal.thread_id": "plan",
|
||||
"graph.goal": "https://github.com/fabro-sh/fabro/issues/126",
|
||||
"internal.run_id": "01KM6S46Q7M6ZV6KP05576W499",
|
||||
"failure_signature": "",
|
||||
"response.plan": "The plan has been written to `/home/daytona/workspace/plan.md`.\n\nIt covers:\n- **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)\n- **Step-by-step implementation** with exact line references and before/after code\n- **Verification commands** to confirm correctness\n- **Test case analysis** explaining why no test changes are needed",
|
||||
"last_response": "The plan has been written to `/home/daytona/workspace/plan.md`.\n\nIt covers:\n- **4 files to modify**: `fabro-agent/Cargo.toml` (add `rand` dep), `subagent.rs` (replace UUID with 8-char hex), `cli.rs` (",
|
||||
"last_response": "The diff is now clean and minimal. Let me summarize the review and cleanup:\n\n## Summary\n\n**Three parallel review agents** examined the changes for code reuse, quality, and efficiency.\n\n### Issues foun",
|
||||
"thread.start.current_node": "plan",
|
||||
"internal.retry_count.implement": 1,
|
||||
"command.stderr": "",
|
||||
"command.output": "",
|
||||
"response.simplify_opus": "The diff is now clean and minimal. Let me summarize the review and cleanup:\n\n## Summary\n\n**Three parallel review agents** examined the changes for code reuse, quality, and efficiency.\n\n### Issues found and fixed:\n\n1. **Unnecessary `rand` dependency** — The `implement` stage added `rand` to `Cargo.toml` just for `rand::random::<u32>()`. But `uuid` was already a dependency (still used in `session.rs`). Fixed by replacing `rand::random::<u32>()` with `uuid::Uuid::new_v4().as_fields().0` which extracts the first u32 field from a UUIDv4 — same randomness, zero new dependencies.\n\n2. **Missing trailing newline in Cargo.toml** — The `implement` stage accidentally stripped the trailing newline. This was automatically resolved when the `rand` line was removed, restoring the file to its original state.\n\n### False positives / no-action items:\n- `short_id` in `pr.rs` is for `run_id` (unrelated to subagent IDs) — no change needed\n- Collision risk of 8-hex-char IDs (4B values) is negligible for subagent concurrency\n- The `format!` call on subagent spawn is not a hot path — no concern\n\n### Final state: 3 files changed, 0 new dependencies added\n- `subagent.rs`: UUID → 8-char hex ID using existing `uuid` crate\n- `cli.rs`: 5 `short_id` truncations removed\n- `run_progress.rs`: 2 `short_id` truncations removed",
|
||||
"thread.plan.current_node": "implement",
|
||||
"internal.node_visit_count": 1,
|
||||
"response.implement": "All 179 tests pass across the entire workspace. Let me do a final summary of all changes made:\n\n## Summary of changes\n\n**4 files modified** as specified in the plan:\n\n### 1. `lib/crates/fabro-agent/Cargo.toml`\n- Added `rand.workspace = true` dependency (needed for `rand::random::<u32>()`)\n\n### 2. `lib/crates/fabro-agent/src/subagent.rs` (line 67)\n- Replaced `uuid::Uuid::new_v4().to_string()` with `format!(\"{:08x}\", rand::random::<u32>())`\n- Subagent IDs are now 8-char lowercase hex strings (e.g. `a3f1b20c`) instead of 36-char UUIDs\n\n### 3. `lib/crates/fabro-agent/src/cli.rs` (5 places)\n- Removed all 5 `let short_id = &agent_id[..8.min(agent_id.len())];` truncation lines\n- Replaced `{short_id}` with `{agent_id}` in format strings for: `SubAgentSpawned`, `SubAgentCompleted`, `SubAgentFailed`, `SubAgentClosed`, and `SubAgentEvent` handlers\n\n### 4. `lib/crates/fabro-cli/src/commands/run_progress.rs` (2 places)\n- Removed both `let short_id = &agent_id[..agent_id.len().min(8)];` truncation lines\n- Replaced `{short_id}` with `{agent_id}` in format strings for: `SubAgentSpawned` and `SubAgentCompleted` handlers\n\n**Verification**: `cargo build --workspace` ✅, `cargo test --workspace` (179 passed) ✅, `cargo clippy --workspace -- -D warnings` ✅, `cargo fmt --check --all` ✅",
|
||||
"outcome": "success",
|
||||
"failure_class": "",
|
||||
"internal.retry_count.plan": 1
|
||||
|
|
@ -56,11 +64,25 @@
|
|||
"/home/daytona/workspace/plan.md"
|
||||
],
|
||||
"duration_ms": 88245
|
||||
},
|
||||
"implement": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
"response.implement": "All 179 tests pass across the entire workspace. Let me do a final summary of all changes made:\n\n## Summary of changes\n\n**4 files modified** as specified in the plan:\n\n### 1. `lib/crates/fabro-agent/Cargo.toml`\n- Added `rand.workspace = true` dependency (needed for `rand::random::<u32>()`)\n\n### 2. `lib/crates/fabro-agent/src/subagent.rs` (line 67)\n- Replaced `uuid::Uuid::new_v4().to_string()` with `format!(\"{:08x}\", rand::random::<u32>())`\n- Subagent IDs are now 8-char lowercase hex strings (e.g. `a3f1b20c`) instead of 36-char UUIDs\n\n### 3. `lib/crates/fabro-agent/src/cli.rs` (5 places)\n- Removed all 5 `let short_id = &agent_id[..8.min(agent_id.len())];` truncation lines\n- Replaced `{short_id}` with `{agent_id}` in format strings for: `SubAgentSpawned`, `SubAgentCompleted`, `SubAgentFailed`, `SubAgentClosed`, and `SubAgentEvent` handlers\n\n### 4. `lib/crates/fabro-cli/src/commands/run_progress.rs` (2 places)\n- Removed both `let short_id = &agent_id[..agent_id.len().min(8)];` truncation lines\n- Replaced `{short_id}` with `{agent_id}` in format strings for: `SubAgentSpawned` and `SubAgentCompleted` handlers\n\n**Verification**: `cargo build --workspace` ✅, `cargo test --workspace` (179 passed) ✅, `cargo clippy --workspace -- -D warnings` ✅, `cargo fmt --check --all` ✅",
|
||||
"command.stderr": "",
|
||||
"last_stage": "simplify_opus",
|
||||
"last_response": "The diff is now clean and minimal. Let me summarize the review and cleanup:\n\n## Summary\n\n**Three parallel review agents** examined the changes for code reuse, quality, and efficiency.\n\n### Issues foun",
|
||||
"response.simplify_opus": "The diff is now clean and minimal. Let me summarize the review and cleanup:\n\n## Summary\n\n**Three parallel review agents** examined the changes for code reuse, quality, and efficiency.\n\n### Issues found and fixed:\n\n1. **Unnecessary `rand` dependency** — The `implement` stage added `rand` to `Cargo.toml` just for `rand::random::<u32>()`. But `uuid` was already a dependency (still used in `session.rs`). Fixed by replacing `rand::random::<u32>()` with `uuid::Uuid::new_v4().as_fields().0` which extracts the first u32 field from a UUIDv4 — same randomness, zero new dependencies.\n\n2. **Missing trailing newline in Cargo.toml** — The `implement` stage accidentally stripped the trailing newline. This was automatically resolved when the `rand` line was removed, restoring the file to its original state.\n\n### False positives / no-action items:\n- `short_id` in `pr.rs` is for `run_id` (unrelated to subagent IDs) — no change needed\n- Collision risk of 8-hex-char IDs (4B values) is negligible for subagent concurrency\n- The `format!` call on subagent spawn is not a hot path — no concern\n\n### Final state: 3 files changed, 0 new dependencies added\n- `subagent.rs`: UUID → 8-char hex ID using existing `uuid` crate\n- `cli.rs`: 5 `short_id` truncations removed\n- `run_progress.rs`: 2 `short_id` truncations removed",
|
||||
"command.output": ""
|
||||
},
|
||||
"notes": "Child completed at cycle 23",
|
||||
"duration_ms": 1026808
|
||||
}
|
||||
},
|
||||
"next_node_id": "implement",
|
||||
"next_node_id": "exit",
|
||||
"node_visits": {
|
||||
"start": 1,
|
||||
"plan": 1
|
||||
"plan": 1,
|
||||
"implement": 1
|
||||
}
|
||||
}
|
||||
6
nodes/implement/status.json
Normal file
6
nodes/implement/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"status": "success",
|
||||
"notes": "Child completed at cycle 23",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-03-20T23:44:56.810839+00:00"
|
||||
}
|
||||
77
nodes/plan/diff.patch
Normal file
77
nodes/plan/diff.patch
Normal file
|
|
@ -0,0 +1,77 @@
|
|||
diff --git a/plan.md b/plan.md
|
||||
new file mode 100644
|
||||
index 00000000..45c8811e
|
||||
--- /dev/null
|
||||
+++ b/plan.md
|
||||
@@ -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