mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-11 22:53:00 +00:00
parent
90529dc91d
commit
76bcffe4c0
5 changed files with 196 additions and 32 deletions
|
|
@ -1,42 +1,73 @@
|
|||
{
|
||||
"timestamp": "2026-04-01T13:09:09.324266Z",
|
||||
"current_node": "preflight_lint",
|
||||
"timestamp": "2026-04-01T13:16:56.030023Z",
|
||||
"current_node": "implement",
|
||||
"completed_nodes": [
|
||||
"start",
|
||||
"toolchain",
|
||||
"preflight_compile",
|
||||
"preflight_lint"
|
||||
"preflight_lint",
|
||||
"implement"
|
||||
],
|
||||
"node_retries": {},
|
||||
"context_values": {
|
||||
"command.stderr": "",
|
||||
"internal.retry_count.preflight_lint": 0,
|
||||
"internal.retry_count.toolchain": 0,
|
||||
"graph.model_stylesheet": "\n * { model: claude-opus-4-6; }\n ",
|
||||
"graph.goal": "# Add storage directory check to `fabro doctor`\n\n## Context\n\n`fabro doctor` validates the installation but doesn't check that the storage directory (where runs, store data, etc. live) exists and is usable. Adding this check surfaces misconfiguration early — e.g. a `storage_dir` override pointing to a nonexistent or read-only path.\n\n## Plan\n\n### 1. Add `check_storage_dir` pure function in `doctor.rs`\n\n**File:** `lib/crates/fabro-cli/src/commands/doctor.rs`\n\nAdd a function like the existing `check_config`:\n\n```rust\nfn check_storage_dir(path: &Path, readable: bool, writable: bool) -> CheckResult\n```\n\n- **Summary always shows the resolved path** (e.g. `/Users/you/.fabro`) — same pattern as `check_config` which puts the path in `summary`.\n- **Pass** — dir exists, readable, writable.\n- **Error** — dir doesn't exist, or not readable, or not writable. Remediation: create it or fix permissions.\n- Details (verbose): existence, read, write status as individual lines.\n\n### 2. Gather state in `run_doctor`\n\nBefore the pure-checks section, resolve the storage dir and probe it:\n\n```rust\nlet storage_dir = cli_settings.storage_dir();\nlet exists = storage_dir.is_dir();\nlet readable = std::fs::read_dir(&storage_dir).is_ok();\nlet writable = tempfile::tempfile_in(&storage_dir).is_ok(); // or write+remove a temp file\n```\n\nUse `std::fs` directly — no async/live probe needed for local filesystem checks.\n\n### 3. Add to \"Required\" section\n\nInsert `check_storage_dir` result into the \"Required\" section, after the \"Configuration\" check and before \"LLM providers\" — storage is fundamental.\n\n### 4. Add unit tests\n\nFollow the existing test pattern (pure function tests with synthetic inputs). Cover:\n- Dir exists + readable + writable → Pass\n- Dir doesn't exist → Error\n- Dir exists but not writable → Error\n\nUse a `tempdir` for real filesystem assertions in a couple of tests.\n\n### 5. Add integration tests in `it/cmd/doctor.rs`\n\n**File:** `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` (existing, has 4 tests)\n\nAdd 2 tests using the existing `test_context!()` + `fabro_snapshot!` pattern:\n\n- **`storage_dir_shown_in_output`** — `TestContext` already creates a temp `storage_dir` and sets `FABRO_STORAGE_DIR`. Run `doctor --dry-run`, snapshot-assert that \"Storage directory\" line appears with the path in the summary.\n- **`storage_dir_missing_shows_error`** — Override `FABRO_STORAGE_DIR` to a nonexistent path via `.env(\"FABRO_STORAGE_DIR\", \"/tmp/nonexistent-fabro-xyz\")`. Run `doctor --dry-run`, snapshot-assert that the check shows error status with the path and remediation text.\n\nBoth tests use `--dry-run` to skip live probes and `fabro_snapshot!` for inline snapshot assertions. Add a filter to normalize the temp dir path (e.g. `[STORAGE_DIR]`).\n\n## Files to modify\n\n- `lib/crates/fabro-cli/src/commands/doctor.rs` — new check function + gather state + wire into section + unit tests\n- `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` — 2 new integration tests\n\n## Verification\n\n- `cargo nextest run -p fabro-cli -- doctor` — unit + integration tests pass\n- `cargo clippy --workspace -- -D warnings` — no lint issues\n- `fabro doctor` — shows new \"Storage directory\" check with the resolved path\n- `fabro doctor -v` — shows detail lines for existence/read/write\n- `FABRO_STORAGE_DIR=/nonexistent fabro doctor` — shows error for missing dir\n",
|
||||
"failure_class": "",
|
||||
"internal.node_visit_count": 1,
|
||||
"internal.thread_id": "preflight_compile",
|
||||
"outcome": "success",
|
||||
"thread.start.current_node": "toolchain",
|
||||
"thread.preflight_compile.current_node": "preflight_lint",
|
||||
"internal.fidelity": "compact",
|
||||
"internal.retry_count.start": 0,
|
||||
"current.preamble": "Goal: # Add storage directory check to `fabro doctor`\n\n## Context\n\n`fabro doctor` validates the installation but doesn't check that the storage directory (where runs, store data, etc. live) exists and is usable. Adding this check surfaces misconfiguration early — e.g. a `storage_dir` override pointing to a nonexistent or read-only path.\n\n## Plan\n\n### 1. Add `check_storage_dir` pure function in `doctor.rs`\n\n**File:** `lib/crates/fabro-cli/src/commands/doctor.rs`\n\nAdd a function like the existing `check_config`:\n\n```rust\nfn check_storage_dir(path: &Path, readable: bool, writable: bool) -> CheckResult\n```\n\n- **Summary always shows the resolved path** (e.g. `/Users/you/.fabro`) — same pattern as `check_config` which puts the path in `summary`.\n- **Pass** — dir exists, readable, writable.\n- **Error** — dir doesn't exist, or not readable, or not writable. Remediation: create it or fix permissions.\n- Details (verbose): existence, read, write status as individual lines.\n\n### 2. Gather state in `run_doctor`\n\nBefore the pure-checks section, resolve the storage dir and probe it:\n\n```rust\nlet storage_dir = cli_settings.storage_dir();\nlet exists = storage_dir.is_dir();\nlet readable = std::fs::read_dir(&storage_dir).is_ok();\nlet writable = tempfile::tempfile_in(&storage_dir).is_ok(); // or write+remove a temp file\n```\n\nUse `std::fs` directly — no async/live probe needed for local filesystem checks.\n\n### 3. Add to \"Required\" section\n\nInsert `check_storage_dir` result into the \"Required\" section, after the \"Configuration\" check and before \"LLM providers\" — storage is fundamental.\n\n### 4. Add unit tests\n\nFollow the existing test pattern (pure function tests with synthetic inputs). Cover:\n- Dir exists + readable + writable → Pass\n- Dir doesn't exist → Error\n- Dir exists but not writable → Error\n\nUse a `tempdir` for real filesystem assertions in a couple of tests.\n\n### 5. Add integration tests in `it/cmd/doctor.rs`\n\n**File:** `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` (existing, has 4 tests)\n\nAdd 2 tests using the existing `test_context!()` + `fabro_snapshot!` pattern:\n\n- **`storage_dir_shown_in_output`** — `TestContext` already creates a temp `storage_dir` and sets `FABRO_STORAGE_DIR`. Run `doctor --dry-run`, snapshot-assert that \"Storage directory\" line appears with the path in the summary.\n- **`storage_dir_missing_shows_error`** — Override `FABRO_STORAGE_DIR` to a nonexistent path via `.env(\"FABRO_STORAGE_DIR\", \"/tmp/nonexistent-fabro-xyz\")`. Run `doctor --dry-run`, snapshot-assert that the check shows error status with the path and remediation text.\n\nBoth tests use `--dry-run` to skip live probes and `fabro_snapshot!` for inline snapshot assertions. Add a filter to normalize the temp dir path (e.g. `[STORAGE_DIR]`).\n\n## Files to modify\n\n- `lib/crates/fabro-cli/src/commands/doctor.rs` — new check function + gather state + wire into section + unit tests\n- `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` — 2 new integration tests\n\n## Verification\n\n- `cargo nextest run -p fabro-cli -- doctor` — unit + integration tests pass\n- `cargo clippy --workspace -- -D warnings` — no lint issues\n- `fabro doctor` — shows new \"Storage directory\" check with the resolved path\n- `fabro doctor -v` — shows detail lines for existence/read/write\n- `FABRO_STORAGE_DIR=/nonexistent fabro doctor` — shows error for missing dir\n\n\n## Completed stages\n- **toolchain**: success\n - 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`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n",
|
||||
"failure_signature": "",
|
||||
"thread.toolchain.current_node": "preflight_compile",
|
||||
"graph.rankdir": "LR",
|
||||
"current_node": "preflight_lint",
|
||||
"command.output": "",
|
||||
"thread.start.current_node": "toolchain",
|
||||
"last_stage": "implement",
|
||||
"current_node": "implement",
|
||||
"response.implement": "All clean. Here's a summary of all changes made:\n\n## Summary of Changes\n\n### Files modified\n\n**1. `lib/crates/fabro-cli/src/commands/doctor.rs`**\n\n- **Import additions**: Added `Path` to `std::path::{Path, PathBuf}` import and added `use fabro_config::FabroSettingsExt;` for the `storage_dir()` method on `FabroSettings`.\n\n- **New pure function `check_storage_dir`**: Added between `check_config` and `check_llm_providers`. Takes `path: &Path`, `readable: bool`, `writable: bool` and returns a `CheckResult`:\n - **Pass**: directory exists, readable, and writable. Summary shows the resolved path.\n - **Error (not exists)**: remediation suggests `mkdir -p <path>`.\n - **Error (permission issues)**: remediation suggests fixing permissions.\n - Verbose details always show existence/read/write status lines.\n\n- **State gathering in `run_doctor`**: After config path checks, resolves the storage directory using `globals.storage_dir` (CLI arg / env var) with fallback to `cli_settings.storage_dir()`. Probes readability via `std::fs::read_dir()` and writability via `tempfile::tempfile_in()`.\n\n- **Wired into \"Required\" section**: Inserted `check_storage_dir()` result after \"Configuration\" and before \"LLM providers\".\n\n- **3 unit tests added** in `mod tests`:\n - `check_storage_dir_pass` — real tempdir → Pass\n - `check_storage_dir_not_exists` — nonexistent path → Error with `mkdir -p` remediation\n - `check_storage_dir_not_writable` — exists but not writable → Error with \"Fix permissions\" remediation\n\n**2. `lib/crates/fabro-cli/tests/it/cmd/doctor.rs`**\n\n- **Updated `dry_run_flag` snapshot**: Added the new `[✓] Storage directory ([STORAGE_DIR])` line.\n\n- **2 new integration tests added**:\n - `storage_dir_shown_in_output` — runs `doctor --dry-run` and verifies \"Storage directory\" appears with `[STORAGE_DIR]` in the summary.\n - `storage_dir_missing_shows_error` — overrides `FABRO_STORAGE_DIR` to a nonexistent path, verifies `[✗]` error status and `mkdir -p` remediation text.",
|
||||
"thread.preflight_lint.current_node": "implement",
|
||||
"internal.retry_count.preflight_compile": 0,
|
||||
"internal.run_id": "01KN4JD6GJTC3PHC8G80EA950B"
|
||||
"graph.rankdir": "LR",
|
||||
"internal.fidelity": "compact",
|
||||
"failure_class": "",
|
||||
"current.preamble": "Goal: # Add storage directory check to `fabro doctor`\n\n## Context\n\n`fabro doctor` validates the installation but doesn't check that the storage directory (where runs, store data, etc. live) exists and is usable. Adding this check surfaces misconfiguration early — e.g. a `storage_dir` override pointing to a nonexistent or read-only path.\n\n## Plan\n\n### 1. Add `check_storage_dir` pure function in `doctor.rs`\n\n**File:** `lib/crates/fabro-cli/src/commands/doctor.rs`\n\nAdd a function like the existing `check_config`:\n\n```rust\nfn check_storage_dir(path: &Path, readable: bool, writable: bool) -> CheckResult\n```\n\n- **Summary always shows the resolved path** (e.g. `/Users/you/.fabro`) — same pattern as `check_config` which puts the path in `summary`.\n- **Pass** — dir exists, readable, writable.\n- **Error** — dir doesn't exist, or not readable, or not writable. Remediation: create it or fix permissions.\n- Details (verbose): existence, read, write status as individual lines.\n\n### 2. Gather state in `run_doctor`\n\nBefore the pure-checks section, resolve the storage dir and probe it:\n\n```rust\nlet storage_dir = cli_settings.storage_dir();\nlet exists = storage_dir.is_dir();\nlet readable = std::fs::read_dir(&storage_dir).is_ok();\nlet writable = tempfile::tempfile_in(&storage_dir).is_ok(); // or write+remove a temp file\n```\n\nUse `std::fs` directly — no async/live probe needed for local filesystem checks.\n\n### 3. Add to \"Required\" section\n\nInsert `check_storage_dir` result into the \"Required\" section, after the \"Configuration\" check and before \"LLM providers\" — storage is fundamental.\n\n### 4. Add unit tests\n\nFollow the existing test pattern (pure function tests with synthetic inputs). Cover:\n- Dir exists + readable + writable → Pass\n- Dir doesn't exist → Error\n- Dir exists but not writable → Error\n\nUse a `tempdir` for real filesystem assertions in a couple of tests.\n\n### 5. Add integration tests in `it/cmd/doctor.rs`\n\n**File:** `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` (existing, has 4 tests)\n\nAdd 2 tests using the existing `test_context!()` + `fabro_snapshot!` pattern:\n\n- **`storage_dir_shown_in_output`** — `TestContext` already creates a temp `storage_dir` and sets `FABRO_STORAGE_DIR`. Run `doctor --dry-run`, snapshot-assert that \"Storage directory\" line appears with the path in the summary.\n- **`storage_dir_missing_shows_error`** — Override `FABRO_STORAGE_DIR` to a nonexistent path via `.env(\"FABRO_STORAGE_DIR\", \"/tmp/nonexistent-fabro-xyz\")`. Run `doctor --dry-run`, snapshot-assert that the check shows error status with the path and remediation text.\n\nBoth tests use `--dry-run` to skip live probes and `fabro_snapshot!` for inline snapshot assertions. Add a filter to normalize the temp dir path (e.g. `[STORAGE_DIR]`).\n\n## Files to modify\n\n- `lib/crates/fabro-cli/src/commands/doctor.rs` — new check function + gather state + wire into section + unit tests\n- `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` — 2 new integration tests\n\n## Verification\n\n- `cargo nextest run -p fabro-cli -- doctor` — unit + integration tests pass\n- `cargo clippy --workspace -- -D warnings` — no lint issues\n- `fabro doctor` — shows new \"Storage directory\" check with the resolved path\n- `fabro doctor -v` — shows detail lines for existence/read/write\n- `FABRO_STORAGE_DIR=/nonexistent fabro doctor` — shows error for missing dir\n\n\n## Completed stages\n- **toolchain**: success\n - 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`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **preflight_lint**: success\n - Script: `cargo clippy -q --workspace -- -D warnings 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n",
|
||||
"command.stderr": "",
|
||||
"internal.retry_count.toolchain": 0,
|
||||
"internal.retry_count.implement": 0,
|
||||
"internal.node_visit_count": 1,
|
||||
"last_response": "All clean. Here's a summary of all changes made:\n\n## Summary of Changes\n\n### Files modified\n\n**1. `lib/crates/fabro-cli/src/commands/doctor.rs`**\n\n- **Import additions**: Added `Path` to `std::path::{",
|
||||
"command.output": "",
|
||||
"internal.thread_id": "preflight_lint",
|
||||
"thread.preflight_compile.current_node": "preflight_lint",
|
||||
"outcome": "success",
|
||||
"graph.goal": "# Add storage directory check to `fabro doctor`\n\n## Context\n\n`fabro doctor` validates the installation but doesn't check that the storage directory (where runs, store data, etc. live) exists and is usable. Adding this check surfaces misconfiguration early — e.g. a `storage_dir` override pointing to a nonexistent or read-only path.\n\n## Plan\n\n### 1. Add `check_storage_dir` pure function in `doctor.rs`\n\n**File:** `lib/crates/fabro-cli/src/commands/doctor.rs`\n\nAdd a function like the existing `check_config`:\n\n```rust\nfn check_storage_dir(path: &Path, readable: bool, writable: bool) -> CheckResult\n```\n\n- **Summary always shows the resolved path** (e.g. `/Users/you/.fabro`) — same pattern as `check_config` which puts the path in `summary`.\n- **Pass** — dir exists, readable, writable.\n- **Error** — dir doesn't exist, or not readable, or not writable. Remediation: create it or fix permissions.\n- Details (verbose): existence, read, write status as individual lines.\n\n### 2. Gather state in `run_doctor`\n\nBefore the pure-checks section, resolve the storage dir and probe it:\n\n```rust\nlet storage_dir = cli_settings.storage_dir();\nlet exists = storage_dir.is_dir();\nlet readable = std::fs::read_dir(&storage_dir).is_ok();\nlet writable = tempfile::tempfile_in(&storage_dir).is_ok(); // or write+remove a temp file\n```\n\nUse `std::fs` directly — no async/live probe needed for local filesystem checks.\n\n### 3. Add to \"Required\" section\n\nInsert `check_storage_dir` result into the \"Required\" section, after the \"Configuration\" check and before \"LLM providers\" — storage is fundamental.\n\n### 4. Add unit tests\n\nFollow the existing test pattern (pure function tests with synthetic inputs). Cover:\n- Dir exists + readable + writable → Pass\n- Dir doesn't exist → Error\n- Dir exists but not writable → Error\n\nUse a `tempdir` for real filesystem assertions in a couple of tests.\n\n### 5. Add integration tests in `it/cmd/doctor.rs`\n\n**File:** `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` (existing, has 4 tests)\n\nAdd 2 tests using the existing `test_context!()` + `fabro_snapshot!` pattern:\n\n- **`storage_dir_shown_in_output`** — `TestContext` already creates a temp `storage_dir` and sets `FABRO_STORAGE_DIR`. Run `doctor --dry-run`, snapshot-assert that \"Storage directory\" line appears with the path in the summary.\n- **`storage_dir_missing_shows_error`** — Override `FABRO_STORAGE_DIR` to a nonexistent path via `.env(\"FABRO_STORAGE_DIR\", \"/tmp/nonexistent-fabro-xyz\")`. Run `doctor --dry-run`, snapshot-assert that the check shows error status with the path and remediation text.\n\nBoth tests use `--dry-run` to skip live probes and `fabro_snapshot!` for inline snapshot assertions. Add a filter to normalize the temp dir path (e.g. `[STORAGE_DIR]`).\n\n## Files to modify\n\n- `lib/crates/fabro-cli/src/commands/doctor.rs` — new check function + gather state + wire into section + unit tests\n- `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` — 2 new integration tests\n\n## Verification\n\n- `cargo nextest run -p fabro-cli -- doctor` — unit + integration tests pass\n- `cargo clippy --workspace -- -D warnings` — no lint issues\n- `fabro doctor` — shows new \"Storage directory\" check with the resolved path\n- `fabro doctor -v` — shows detail lines for existence/read/write\n- `FABRO_STORAGE_DIR=/nonexistent fabro doctor` — shows error for missing dir\n",
|
||||
"thread.toolchain.current_node": "preflight_compile",
|
||||
"graph.model_stylesheet": "\n * { model: claude-opus-4-6; }\n ",
|
||||
"internal.run_id": "01KN4JD6GJTC3PHC8G80EA950B",
|
||||
"internal.retry_count.preflight_lint": 0
|
||||
},
|
||||
"node_outcomes": {
|
||||
"start": {
|
||||
"status": "success",
|
||||
"usage": null
|
||||
},
|
||||
"implement": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
"response.implement": "All clean. Here's a summary of all changes made:\n\n## Summary of Changes\n\n### Files modified\n\n**1. `lib/crates/fabro-cli/src/commands/doctor.rs`**\n\n- **Import additions**: Added `Path` to `std::path::{Path, PathBuf}` import and added `use fabro_config::FabroSettingsExt;` for the `storage_dir()` method on `FabroSettings`.\n\n- **New pure function `check_storage_dir`**: Added between `check_config` and `check_llm_providers`. Takes `path: &Path`, `readable: bool`, `writable: bool` and returns a `CheckResult`:\n - **Pass**: directory exists, readable, and writable. Summary shows the resolved path.\n - **Error (not exists)**: remediation suggests `mkdir -p <path>`.\n - **Error (permission issues)**: remediation suggests fixing permissions.\n - Verbose details always show existence/read/write status lines.\n\n- **State gathering in `run_doctor`**: After config path checks, resolves the storage directory using `globals.storage_dir` (CLI arg / env var) with fallback to `cli_settings.storage_dir()`. Probes readability via `std::fs::read_dir()` and writability via `tempfile::tempfile_in()`.\n\n- **Wired into \"Required\" section**: Inserted `check_storage_dir()` result after \"Configuration\" and before \"LLM providers\".\n\n- **3 unit tests added** in `mod tests`:\n - `check_storage_dir_pass` — real tempdir → Pass\n - `check_storage_dir_not_exists` — nonexistent path → Error with `mkdir -p` remediation\n - `check_storage_dir_not_writable` — exists but not writable → Error with \"Fix permissions\" remediation\n\n**2. `lib/crates/fabro-cli/tests/it/cmd/doctor.rs`**\n\n- **Updated `dry_run_flag` snapshot**: Added the new `[✓] Storage directory ([STORAGE_DIR])` line.\n\n- **2 new integration tests added**:\n - `storage_dir_shown_in_output` — runs `doctor --dry-run` and verifies \"Storage directory\" appears with `[STORAGE_DIR]` in the summary.\n - `storage_dir_missing_shows_error` — overrides `FABRO_STORAGE_DIR` to a nonexistent path, verifies `[✗]` error status and `mkdir -p` remediation text.",
|
||||
"last_stage": "implement",
|
||||
"last_response": "All clean. Here's a summary of all changes made:\n\n## Summary of Changes\n\n### Files modified\n\n**1. `lib/crates/fabro-cli/src/commands/doctor.rs`**\n\n- **Import additions**: Added `Path` to `std::path::{"
|
||||
},
|
||||
"notes": "Stage completed: implement",
|
||||
"usage": {
|
||||
"model": "claude-opus-4-6",
|
||||
"input_tokens": 74358,
|
||||
"output_tokens": 10734,
|
||||
"cache_read_tokens": 2888910,
|
||||
"cache_write_tokens": 80751,
|
||||
"cost": 1.92042
|
||||
},
|
||||
"files_touched": [
|
||||
"/home/daytona/workspace/lib/crates/fabro-cli/src/commands/doctor.rs",
|
||||
"/home/daytona/workspace/lib/crates/fabro-cli/tests/it/cmd/doctor.rs"
|
||||
]
|
||||
},
|
||||
"preflight_compile": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
"command.stderr": "",
|
||||
"command.output": ""
|
||||
"command.output": "",
|
||||
"command.stderr": ""
|
||||
},
|
||||
"notes": "Script completed: cargo check -q --workspace 2>&1",
|
||||
"usage": null
|
||||
|
|
@ -50,25 +81,22 @@
|
|||
"notes": "Script completed: 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",
|
||||
"usage": null
|
||||
},
|
||||
"start": {
|
||||
"status": "success",
|
||||
"usage": null
|
||||
},
|
||||
"preflight_lint": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
"command.stderr": "",
|
||||
"command.output": ""
|
||||
"command.output": "",
|
||||
"command.stderr": ""
|
||||
},
|
||||
"notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1",
|
||||
"usage": null
|
||||
}
|
||||
},
|
||||
"next_node_id": "implement",
|
||||
"next_node_id": "simplify_opus",
|
||||
"node_visits": {
|
||||
"implement": 1,
|
||||
"preflight_compile": 1,
|
||||
"toolchain": 1,
|
||||
"start": 1,
|
||||
"preflight_lint": 1,
|
||||
"toolchain": 1
|
||||
"preflight_lint": 1
|
||||
}
|
||||
}
|
||||
93
nodes/implement/prompt.md
Normal file
93
nodes/implement/prompt.md
Normal file
|
|
@ -0,0 +1,93 @@
|
|||
Goal: # Add storage directory check to `fabro doctor`
|
||||
|
||||
## Context
|
||||
|
||||
`fabro doctor` validates the installation but doesn't check that the storage directory (where runs, store data, etc. live) exists and is usable. Adding this check surfaces misconfiguration early — e.g. a `storage_dir` override pointing to a nonexistent or read-only path.
|
||||
|
||||
## Plan
|
||||
|
||||
### 1. Add `check_storage_dir` pure function in `doctor.rs`
|
||||
|
||||
**File:** `lib/crates/fabro-cli/src/commands/doctor.rs`
|
||||
|
||||
Add a function like the existing `check_config`:
|
||||
|
||||
```rust
|
||||
fn check_storage_dir(path: &Path, readable: bool, writable: bool) -> CheckResult
|
||||
```
|
||||
|
||||
- **Summary always shows the resolved path** (e.g. `/Users/you/.fabro`) — same pattern as `check_config` which puts the path in `summary`.
|
||||
- **Pass** — dir exists, readable, writable.
|
||||
- **Error** — dir doesn't exist, or not readable, or not writable. Remediation: create it or fix permissions.
|
||||
- Details (verbose): existence, read, write status as individual lines.
|
||||
|
||||
### 2. Gather state in `run_doctor`
|
||||
|
||||
Before the pure-checks section, resolve the storage dir and probe it:
|
||||
|
||||
```rust
|
||||
let storage_dir = cli_settings.storage_dir();
|
||||
let exists = storage_dir.is_dir();
|
||||
let readable = std::fs::read_dir(&storage_dir).is_ok();
|
||||
let writable = tempfile::tempfile_in(&storage_dir).is_ok(); // or write+remove a temp file
|
||||
```
|
||||
|
||||
Use `std::fs` directly — no async/live probe needed for local filesystem checks.
|
||||
|
||||
### 3. Add to "Required" section
|
||||
|
||||
Insert `check_storage_dir` result into the "Required" section, after the "Configuration" check and before "LLM providers" — storage is fundamental.
|
||||
|
||||
### 4. Add unit tests
|
||||
|
||||
Follow the existing test pattern (pure function tests with synthetic inputs). Cover:
|
||||
- Dir exists + readable + writable → Pass
|
||||
- Dir doesn't exist → Error
|
||||
- Dir exists but not writable → Error
|
||||
|
||||
Use a `tempdir` for real filesystem assertions in a couple of tests.
|
||||
|
||||
### 5. Add integration tests in `it/cmd/doctor.rs`
|
||||
|
||||
**File:** `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` (existing, has 4 tests)
|
||||
|
||||
Add 2 tests using the existing `test_context!()` + `fabro_snapshot!` pattern:
|
||||
|
||||
- **`storage_dir_shown_in_output`** — `TestContext` already creates a temp `storage_dir` and sets `FABRO_STORAGE_DIR`. Run `doctor --dry-run`, snapshot-assert that "Storage directory" line appears with the path in the summary.
|
||||
- **`storage_dir_missing_shows_error`** — Override `FABRO_STORAGE_DIR` to a nonexistent path via `.env("FABRO_STORAGE_DIR", "/tmp/nonexistent-fabro-xyz")`. Run `doctor --dry-run`, snapshot-assert that the check shows error status with the path and remediation text.
|
||||
|
||||
Both tests use `--dry-run` to skip live probes and `fabro_snapshot!` for inline snapshot assertions. Add a filter to normalize the temp dir path (e.g. `[STORAGE_DIR]`).
|
||||
|
||||
## Files to modify
|
||||
|
||||
- `lib/crates/fabro-cli/src/commands/doctor.rs` — new check function + gather state + wire into section + unit tests
|
||||
- `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` — 2 new integration tests
|
||||
|
||||
## Verification
|
||||
|
||||
- `cargo nextest run -p fabro-cli -- doctor` — unit + integration tests pass
|
||||
- `cargo clippy --workspace -- -D warnings` — no lint issues
|
||||
- `fabro doctor` — shows new "Storage directory" check with the resolved path
|
||||
- `fabro doctor -v` — shows detail lines for existence/read/write
|
||||
- `FABRO_STORAGE_DIR=/nonexistent fabro doctor` — shows error for missing dir
|
||||
|
||||
|
||||
## 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"
|
||||
}
|
||||
32
nodes/implement/response.md
Normal file
32
nodes/implement/response.md
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
All clean. Here's a summary of all changes made:
|
||||
|
||||
## Summary of Changes
|
||||
|
||||
### Files modified
|
||||
|
||||
**1. `lib/crates/fabro-cli/src/commands/doctor.rs`**
|
||||
|
||||
- **Import additions**: Added `Path` to `std::path::{Path, PathBuf}` import and added `use fabro_config::FabroSettingsExt;` for the `storage_dir()` method on `FabroSettings`.
|
||||
|
||||
- **New pure function `check_storage_dir`**: Added between `check_config` and `check_llm_providers`. Takes `path: &Path`, `readable: bool`, `writable: bool` and returns a `CheckResult`:
|
||||
- **Pass**: directory exists, readable, and writable. Summary shows the resolved path.
|
||||
- **Error (not exists)**: remediation suggests `mkdir -p <path>`.
|
||||
- **Error (permission issues)**: remediation suggests fixing permissions.
|
||||
- Verbose details always show existence/read/write status lines.
|
||||
|
||||
- **State gathering in `run_doctor`**: After config path checks, resolves the storage directory using `globals.storage_dir` (CLI arg / env var) with fallback to `cli_settings.storage_dir()`. Probes readability via `std::fs::read_dir()` and writability via `tempfile::tempfile_in()`.
|
||||
|
||||
- **Wired into "Required" section**: Inserted `check_storage_dir()` result after "Configuration" and before "LLM providers".
|
||||
|
||||
- **3 unit tests added** in `mod tests`:
|
||||
- `check_storage_dir_pass` — real tempdir → Pass
|
||||
- `check_storage_dir_not_exists` — nonexistent path → Error with `mkdir -p` remediation
|
||||
- `check_storage_dir_not_writable` — exists but not writable → Error with "Fix permissions" remediation
|
||||
|
||||
**2. `lib/crates/fabro-cli/tests/it/cmd/doctor.rs`**
|
||||
|
||||
- **Updated `dry_run_flag` snapshot**: Added the new `[✓] Storage directory ([STORAGE_DIR])` line.
|
||||
|
||||
- **2 new integration tests added**:
|
||||
- `storage_dir_shown_in_output` — runs `doctor --dry-run` and verifies "Storage directory" appears with `[STORAGE_DIR]` in the summary.
|
||||
- `storage_dir_missing_shows_error` — overrides `FABRO_STORAGE_DIR` to a nonexistent path, verifies `[✗]` error status and `mkdir -p` remediation text.
|
||||
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-04-01T13:16:56.023795Z"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue