mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-07 03:00:29 +00:00
parent
e9fef33b01
commit
626fe815f4
5 changed files with 214 additions and 7 deletions
File diff suppressed because one or more lines are too long
118
nodes/implement/prompt.md
Normal file
118
nodes/implement/prompt.md
Normal file
|
|
@ -0,0 +1,118 @@
|
|||
Goal: # Add Sentry panic reporting to fabro CLI
|
||||
|
||||
## Context
|
||||
|
||||
Fabro has no panic reporting. When the CLI panics, we lose visibility. We want Sentry-based panic reporting modeled on qlty's approach: serialize a Sentry event to a temp file, double-fork a detached subprocess to upload it.
|
||||
|
||||
The existing `__send_analytics` sender uses a simple `Command::spawn()` which is unreliable — the child can get killed when the parent exits or the terminal session ends. We'll fix both senders to use the double-fork pattern (fork → setsid → close_fd → fork → exec) from qlty, which fully detaches the subprocess.
|
||||
|
||||
## Files to modify
|
||||
|
||||
### 1. `Cargo.toml` (workspace root) — add deps
|
||||
```toml
|
||||
sentry = { version = "0.35", default-features = false, features = ["backtrace", "contexts", "ureq", "rustls"] }
|
||||
fork = "0.2"
|
||||
exec = "0.3"
|
||||
```
|
||||
|
||||
### 2. `lib/crates/fabro-util/Cargo.toml` — add deps
|
||||
```toml
|
||||
sentry.workspace = true
|
||||
fork.workspace = true
|
||||
exec.workspace = true
|
||||
```
|
||||
|
||||
### 3. `lib/crates/fabro-util/src/telemetry/mod.rs`
|
||||
- Add `pub mod panic;`
|
||||
- Add `pub mod spawn;`
|
||||
|
||||
### 4. `lib/crates/fabro-util/src/telemetry/spawn.rs` — NEW FILE
|
||||
|
||||
Extract a shared `spawn_detached(args: &[&str], env: &[(&str, &str)])` function used by both analytics sender and panic sender. Implements:
|
||||
- **Unix**: double-fork via `fork` crate (fork → setsid → close_fd → fork → exec)
|
||||
- **Windows**: `Command::new().creation_flags(DETACHED_PROCESS).spawn()`
|
||||
|
||||
### 5. `lib/crates/fabro-util/src/telemetry/sender.rs` — refactor to use `spawn_detached`
|
||||
|
||||
Replace the `Command::new().spawn()` call with `spawn::spawn_detached()`.
|
||||
|
||||
### 6. `lib/crates/fabro-util/src/telemetry/panic.rs` — NEW FILE
|
||||
|
||||
Core logic:
|
||||
|
||||
- `install_panic_hook()` — chains onto default hook (like qlty). Calls `report_panic()` then delegates to default.
|
||||
- `report_panic(info: &PanicHookInfo)` — checks telemetry level (Off → return), filters "Broken pipe" panics, builds `sentry::protocol::Event` with:
|
||||
- Exception with `mechanism.ty = "panic"`, `handled = false`
|
||||
- Panic message as `value`
|
||||
- `sentry_backtrace::current_stacktrace()`
|
||||
- OS context
|
||||
- Level = Fatal
|
||||
- `spawn_panic_sender(event)` — serialize event to `~/.fabro/tmp/fabro-panic-{id}.json`, double-fork `fabro __send_panic <path>` with `FABRO_TELEMETRY=off`.
|
||||
- `pub async fn send_panic_to_sentry(path: &Path)` — read JSON, init sentry client with `SENTRY_DSN` (compile-time `option_env!`), `sentry::capture_event()`, delete temp file.
|
||||
|
||||
### 7. `lib/crates/fabro-cli/src/main.rs` — two changes
|
||||
|
||||
**a) Install panic hook early in `main()`** (before `main_inner()`):
|
||||
```rust
|
||||
fabro_util::telemetry::panic::install_panic_hook();
|
||||
```
|
||||
|
||||
**b) Add `__send_panic` hidden subcommand** (same pattern as `__send_analytics`).
|
||||
|
||||
## Key design decisions
|
||||
|
||||
- **Double-fork for all background senders** — ensures subprocess survives parent exit and terminal close. Fixes existing `__send_analytics` reliability too.
|
||||
- **Shared `spawn_detached`** — avoids duplicating the fork logic between analytics and panic senders.
|
||||
- **Telemetry level respected** — `Off` skips panic reporting. `Errors` and `All` both report.
|
||||
- **Recursion prevention** — subprocess sets `FABRO_TELEMETRY=off`.
|
||||
- **Compile-time DSN** — `option_env!("SENTRY_DSN")`, no-ops when unset (dev builds).
|
||||
- **Filter broken pipe** — CLI tools get SIGPIPE from `| head` etc., not a real bug.
|
||||
|
||||
## Implementation order (red/green TDD)
|
||||
|
||||
Work bottom-up through the dependency chain. For each step, write the test first (red), then write the minimal code to make it pass (green).
|
||||
|
||||
### Step 1: `spawn_detached` (fabro-util)
|
||||
- **Red**: Test that `spawn_detached` constructs the right args (unit-testable parts: temp file writing, arg assembly). On Unix, test the double-fork integration by spawning a real process that writes a marker file.
|
||||
- **Green**: Implement `spawn.rs` with fork/setsid/close_fd/fork/exec on Unix, DETACHED_PROCESS on Windows.
|
||||
|
||||
### Step 2: Refactor `sender.rs` to use `spawn_detached`
|
||||
- **Red**: Existing `send_noops_without_write_key` test still passes. Add test verifying temp file is written with correct JSON.
|
||||
- **Green**: Replace `Command::spawn()` with `spawn_detached()`.
|
||||
|
||||
### Step 3: `panic.rs` — event building
|
||||
- **Red**: Test `build_panic_event()` produces correct Sentry event structure (exception type, mechanism, level, stacktrace present).
|
||||
- **Green**: Implement event builder.
|
||||
|
||||
### Step 4: `panic.rs` — hook + spawn
|
||||
- **Red**: Test that `report_panic` filters broken pipe. Test that it no-ops when telemetry is Off.
|
||||
- **Green**: Implement `install_panic_hook()`, `report_panic()`, `spawn_panic_sender()`.
|
||||
|
||||
### Step 5: `panic.rs` — Sentry sender
|
||||
- **Red**: Test `send_panic_to_sentry` no-ops without DSN (like analytics no-ops without write key).
|
||||
- **Green**: Implement the `__send_panic` worker function.
|
||||
|
||||
### Step 6: Wire up in `main.rs`
|
||||
- Add `__send_panic` subcommand + install panic hook.
|
||||
- Verify: `cargo build --workspace && cargo test --workspace && cargo clippy --workspace -- -D warnings`
|
||||
|
||||
|
||||
## Completed stages
|
||||
- **toolchain**: success
|
||||
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
|
||||
- Stdout:
|
||||
```
|
||||
cargo 1.94.0 (85eff7c80 2026-01-15)
|
||||
```
|
||||
- Stderr: (empty)
|
||||
- **preflight_compile**: success
|
||||
- Script: `cargo check -q --workspace 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
- **preflight_lint**: success
|
||||
- Script: `cargo clippy -q --workspace -- -D warnings 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
|
||||
|
||||
Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.
|
||||
5
nodes/implement/provider_used.json
Normal file
5
nodes/implement/provider_used.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"mode": "agent",
|
||||
"provider": "anthropic",
|
||||
"model": "claude-opus-4-6"
|
||||
}
|
||||
44
nodes/implement/response.md
Normal file
44
nodes/implement/response.md
Normal file
|
|
@ -0,0 +1,44 @@
|
|||
All tests pass across the entire workspace — zero failures. Here's a summary of all the changes made:
|
||||
|
||||
## Summary of Changes
|
||||
|
||||
### 1. `Cargo.toml` (workspace root) — Added dependencies
|
||||
- `sentry = { version = "0.35", default-features = false, features = ["backtrace", "contexts", "ureq", "rustls"] }`
|
||||
- `fork = "0.2"`
|
||||
- `exec = "0.3"`
|
||||
|
||||
### 2. `lib/crates/fabro-util/Cargo.toml` — Added dependencies
|
||||
- `sentry.workspace = true`
|
||||
- `fork.workspace = true`
|
||||
- `exec.workspace = true`
|
||||
|
||||
### 3. `lib/crates/fabro-util/src/telemetry/mod.rs` — Registered new modules
|
||||
- Added `pub mod panic;`
|
||||
- Added `pub mod spawn;`
|
||||
|
||||
### 4. `lib/crates/fabro-util/src/telemetry/spawn.rs` — **NEW FILE**
|
||||
Shared `spawn_detached()` function with:
|
||||
- **Unix**: double-fork pattern (`fork → setsid → close_fd → fork → exec`) via the `fork` and `exec` crates
|
||||
- **Windows**: `Command::new().creation_flags(DETACHED_PROCESS).spawn()`
|
||||
- Helper functions: `write_temp_json()`, `build_fabro_argv()`, `current_exe_str()`, `path_exists()`
|
||||
- **5 tests**: temp file creation, argv building, current exe, empty args no-op, Unix integration test with marker file
|
||||
|
||||
### 5. `lib/crates/fabro-util/src/telemetry/sender.rs` — Refactored to use `spawn_detached`
|
||||
- Replaced `Command::new().spawn()` with `super::spawn::spawn_detached()` and `super::spawn::write_temp_json()`
|
||||
- Simplified `spawn_sender()` to return `()` (no longer `Result`)
|
||||
- Existing `send_noops_without_write_key` test continues to pass
|
||||
|
||||
### 6. `lib/crates/fabro-util/src/telemetry/panic.rs` — **NEW FILE**
|
||||
Core panic reporting logic:
|
||||
- `install_panic_hook()` — chains onto default hook, calls `report_panic()` first
|
||||
- `build_panic_event()` — builds `sentry::protocol::Event` with exception (type=`"panic"`, mechanism `handled=false`), stacktrace, OS context, Fatal level, release version
|
||||
- `report_panic()` — checks telemetry level (Off → skip), filters "Broken pipe" panics, builds event, spawns sender
|
||||
- `spawn_panic_sender()` — serializes event to `~/.fabro/tmp/fabro-panic-{id}.json`, double-forks `fabro __send_panic <path>` with `FABRO_TELEMETRY=off`
|
||||
- `send_panic_to_sentry()` — reads JSON, inits Sentry client with compile-time `SENTRY_DSN`, captures event, flushes
|
||||
- **5 tests**: event structure validation, broken pipe filtering, telemetry-off no-op, DSN-missing no-op, JSON round-trip
|
||||
|
||||
### 7. `lib/crates/fabro-cli/src/main.rs` — Wired up
|
||||
- `install_panic_hook()` called at the very start of `main()` (before `main_inner()`)
|
||||
- Added `SendPanic` hidden subcommand variant (mirrors `SendAnalytics`)
|
||||
- Added command name mapping `"__send_panic"`
|
||||
- Added handler that reads the JSON, calls `send_panic_to_sentry()`, and cleans up the temp file
|
||||
6
nodes/implement/status.json
Normal file
6
nodes/implement/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"status": "success",
|
||||
"notes": "Stage completed: implement",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-03-16T05:15:43.423099+00:00"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue