scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-08 03:10:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

refactor(agent): simplify reviewed changes

Browse source 
Use raw sandbox reads for memory and skills, keep line-numbered reads focused on display, and share retry-delay handling across agent and LLM code.

Trim task tool descriptions, bound multi-file read concurrency, restore Docker's text read path, and add the reviewed implementation plan docs.
This commit is contained in:
Bryan Helmkamp 2026-05-22 21:51:45 -04:00
parent d1cc47324d
commit eb4891b1b0
No known key found for this signature in database
19 changed files with 2028 additions and 214 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

163
docs/plans/2026-05-21-wall-and-active-time-metrics-plan.md Normal file
View file

@ -0,0 +1,163 @@
---
title: "feat: Wall and active time metrics"
type: feature
status: active
date: 2026-05-21
---
# feat: Wall and active time metrics
## Summary
Rename runtime duration concepts from ambiguous duration/runtime/elapsed fields
to explicit wall-time fields, then add first-class active timing.
Definitions:
- `wall_time_ms`: elapsed clock time from start to finish.
- `inference_time_ms`: Fabro-observed LLM request/stream elapsed time.
- `tool_time_ms`: tool or command execution elapsed time.
- `active_time_ms`: `inference_time_ms + tool_time_ms`.
This is greenfield API churn. Do not preserve old public run/stage timing
fields, aliases, or compatibility shims for `duration_ms`, `runtime_secs`, or
`elapsed_secs` on run/stage runtime surfaces.
Run-level active time is total work performed: sum active timing across stage
visits. Parallel work is summed, so run active time can exceed run wall time.
## Key Changes
- Add a shared timing value object in `fabro-types` for stage/run active timing:
- `wall_time_ms`
- `inference_time_ms`
- `tool_time_ms`
- serialized `active_time_ms` derived by constructors/accessors from
`inference_time_ms + tool_time_ms`; do not let callers supply an
independent active-time value.
- Replace run/stage public timing fields:
- stage/run terminal event props use `wall_time_ms` plus the active timing
breakdown.
- `StageProjection` stores the timing breakdown instead of stage
`duration_ms`.
- `RunTimestamps` keeps timestamps only; move elapsed values into a separate
run timing object.
- `/runs/{id}/stages` and `/runs/{id}/billing` expose timing in milliseconds,
not `runtime_secs`.
- Keep `duration_ms` only for unrelated subsystem-specific operational events
where the name is still local and unambiguous, such as sandbox setup,
metadata snapshot, devcontainer lifecycle, and hook execution. The cleanup
target is public run/stage runtime semantics.
- Update OpenAPI and regenerate the Rust and TypeScript API clients after
schema edits.
## Timing Behavior
- `prompt` nodes:
- inference = elapsed time spent in the one-shot LLM backend call.
- tool = 0.
- native `agent` nodes:
- inference = sum of elapsed time spent opening/consuming LLM streams for new
turns in the stage.
- tool = sum of elapsed time spent executing agent tool calls.
- retry backoff and waiting for steering are wall time, not active time.
- opaque external/ACP agent nodes:
- inference = 0 for v1 because Fabro cannot reliably separate model time from
process runtime.
- tool = external agent process wall time.
- `command` nodes:
- inference = 0.
- tool = command wall time from the sandbox command result.
- `human`, `wait`, `conditional`, `fan-in`, `start`, and `exit`:
- inference = 0.
- tool = 0.
- `parallel` container nodes:
- active = 0 on the container stage.
- child/branch stages carry work timing so rollups do not double count.
## Implementation
- In `fabro-types`, introduce the timing structs and replace the relevant fields
in `Outcome`, `NodeResult` consumers, `StageProjection`, `Conclusion`,
`RunTimestamps`, `RunCompletedProps`, `RunFailedProps`,
`StageCompletedProps`, `StageFailedProps`, `RunBillingStage`, and
`RunBillingTotals`.
- In `fabro-workflow`, rename run/stage execution fields from `duration_ms` to
`wall_time_ms` and thread timing through lifecycle events, terminal events,
conclusion building, pull request summaries, timeline/billing rollups, and
test support fixtures.
- In `fabro-agent`, add timing data to agent events or session results so
`fabro-workflow` can aggregate:
- LLM stream/request elapsed time per assistant response.
- tool call elapsed time per tool completion.
- preserve token billing behavior separately from timing.
- In `fabro-store`, update event projection to write stage `started_at`, timing
breakdowns, and run summary timing from the new event props.
- In `fabro-server`, replace runtime billing aggregation with a timing rollup
owned by workflow/projection code. Billing endpoints may include timing, but
billing logic should not define timing semantics.
- In `apps/fabro-web`, update run list/detail/stages/billing views and tests to
render wall time and active time from the new fields.
- Remove all run/stage public API references to old timing names from
`docs/public/api-reference/fabro-api.yaml` and regenerated clients.
## Test Plan
- `fabro-types`:
- run and stage event round trips serialize the new timing payloads.
- old public run/stage timing properties are absent from serialized fixtures.
- API-facing timing structs round trip through generated schemas.
- `fabro-store`:
- `stage.started` records `started_at`.
- stage terminal events store `wall_time_ms` and active breakdowns.
- run summaries expose timestamp fields and run timing without
`elapsed_secs`.
- retried stages reset per-attempt live wall-time state correctly.
- `fabro-workflow`:
- prompt stages report inference-only active timing.
- command stages report tool-only active timing.
- native agent stages sum LLM turn timing and tool timing.
- human/wait/conditional/fan-in/start/exit stages report zero active timing.
- parallel stage rollups sum child active work and avoid container double
counting.
- repeated node visits sum timing by node in rollups.
- `fabro-server`:
- `/runs/{id}/stages`, `/runs/{id}/billing`, run detail, and run list return
new timing fields only.
- aggregate billing/timing totals sum active work across completed runs.
- OpenAPI conformance passes after regeneration.
- `apps/fabro-web`:
- run list/detail/billing/stages render wall time and active time.
- in-flight wall-time ticking still uses `started_at`.
- no UI code reads `runtime_secs`, `elapsed_secs`, or run/stage
`duration_ms`.
## Validation
Run focused checks first:
```bash
cargo nextest run -p fabro-types -p fabro-store -p fabro-workflow -p fabro-server
cd apps/fabro-web && bun test && bun run typecheck
```
Then run full workspace checks before merging:
```bash
cargo build --workspace
cargo nextest run --workspace
cargo +nightly-2026-04-14 fmt --check --all
cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
git diff --check
```
## Assumptions
- Inference time is Fabro-observed LLM request/stream elapsed time, not
provider-reported model-only compute time.
- LLM retry backoff, queueing outside a request/stream, human waits, steering
waits, and scheduler gaps are wall time but not active time.
- Active timing is finalized-event based in v1; live active-time ticking can be
added later if it becomes necessary.
- No compatibility layer is required for existing API clients or stored run
event data.

309
docs/plans/2026-05-22-001-fix-mcp-create-schema-mismatch-plan.md Normal file
View file

@ -0,0 +1,309 @@
---
title: fix: Align fabro_run_create MCP schema and accepted input
type: fix
status: active
date: 2026-05-22
---
# fix: Align fabro_run_create MCP schema and accepted input
## Overview
Fix the mismatch where MCP clients see `fabro_run_create` as accepting
`runs: string[]`, while the running server currently deserializes
`runs` as an array of `CreateRunSpec` objects. The fix should make the
tool robust for agents that follow the advertised string shorthand while
also preserving the richer object form used by existing tests and callers.
## Problem Frame
During manual MCP testing, this call failed before tool validation:
```json
{ "runs": ["sleeper"] }
```
The server returned a deserialization error because it expected a
`CreateRunSpec` object. This is a poor agent-facing failure mode: the
client-visible schema implied the call was valid, but the runtime contract
rejected it. The object form still worked:
```json
{ "runs": [{ "workflow": "sleeper", "auto_approve": true, "start": true }] }
```
## Requirements Trace
- R1. `fabro_run_create` must accept the string shorthand advertised to MCP
clients, treating each string as the workflow selector.
- R2. Existing object-form `CreateRunSpec` calls must keep working with all
current optional fields.
- R3. MCP `tools/list` must advertise a truthful schema for `runs` so clients
can discover both accepted forms, or at minimum no longer advertise only a
shape that fails at runtime.
- R4. Local validation errors must remain tool errors and must still happen
before auth or network access.
- R5. Docs and QA notes should show the supported shapes so future manual
testing does not rediscover the mismatch.
## Scope Boundaries
- Do not change the HTTP run creation API.
- Do not change manifest resolution semantics for object-form create specs.
- Do not add new create-run options beyond accepting string shorthand.
- Do not make `client_message_id`, pair APIs, or other MCP tools part of this
fix.
## Context & Research
### Relevant Code and Patterns
- `lib/crates/fabro-tool/src/create.rs` owns `FabroRunCreateParams`,
`CreateRunSpec`, validation, and create-run execution.
- `lib/crates/fabro-mcp-server/src/server.rs` registers the MCP tool using
`Parameters<run_tools::FabroRunCreateParams>`.
- `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` already exercises object-form
`fabro_run_create`, schema listing, and pre-auth validation.
- `docs/internal/mcp-server-qa-test-plan.md` already records past
schema/runtime mismatches, especially the `inputs` schema narrowing.
- `docs/public/agents/mcp.mdx` lists the MCP tools but does not show concrete
`fabro_run_create` input examples.
### Institutional Learnings
- No `docs/solutions/` directory exists in this checkout.
- The MCP QA plan shows this class of bug has appeared before: schema/runtime
agreement for MCP parameters needs explicit tests, not only happy-path calls.
### External References
- None. The issue is internal schema/deserialization parity; local patterns are
sufficient.
## Key Technical Decisions
- Accept both string shorthand and object specs. This is additive, fixes the
observed failed call directly, and preserves existing rich create semantics.
- Normalize inputs before validation. Convert raw string/object specs into the
existing `ValidatedCreateRunSpec` path so all downstream manifest and run
creation behavior stays centralized.
- Add schema assertions near the MCP boundary. Unit validation alone cannot
catch a client-visible `tools/list` regression.
## Open Questions
### Resolved During Planning
- Should the object form remain supported? Yes. Existing tests and the MCP
design require optional create settings like `dry_run`, `auto_approve`,
`labels`, `parent_id`, and `start`.
- Should string shorthand be treated as workflow selector only? Yes. It maps
cleanly to the one required object-form field.
### Deferred to Implementation
- Exact schema shape: use `anyOf`/`oneOf`, inline schemas, or a manual
`JsonSchema` implementation depending on how `schemars` and `rmcp` emit the
final `tools/list` schema.
## Implementation Units
- [ ] **Unit 1: Characterize and lock the current schema mismatch**
**Goal:** Add failing coverage that proves `fabro_run_create` advertises and
accepts the intended `runs` item shapes.
**Requirements:** R1, R3, R4
**Dependencies:** None
**Files:**
- Modify: `lib/crates/fabro-mcp-server/src/server.rs`
- Modify: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs`
**Approach:**
- Add a server-level schema test for `fabro_run_create`, similar to the
existing `fabro_run_pair` schema leakage test.
- Assert the schema for `runs` does not collapse to string-only if the runtime
requires object fields.
- Add an MCP stdio integration case that calls `fabro_run_create` with
`runs: ["simple.fabro"]` against an unreachable server and verifies local
parameter validation succeeds far enough to require backend/auth, not fail
with `expected struct CreateRunSpec`.
**Execution note:** Characterization-first. Capture the failing schema/runtime
contract before changing deserialization.
**Patterns to follow:**
- `fabro_run_pair_tool_is_registered_with_stage_based_schema` in
`lib/crates/fabro-mcp-server/src/server.rs`.
- `mcp_create_validation_errors_happen_before_auth_or_network` in
`lib/crates/fabro-cli/tests/it/cmd/mcp.rs`.
**Test scenarios:**
- Integration: `tools/list` for `fabro_run_create` exposes `runs` as an array
whose items include the object form with a required `workflow` field.
- Error path: calling `fabro_run_create` with `runs: ["simple.fabro"]` no
longer returns an MCP deserialization error mentioning `CreateRunSpec`.
- Error path: malformed non-string/non-object run items still fail before
auth/network with an actionable tool or MCP parameter error.
**Verification:**
- The new tests fail against the current behavior and identify the mismatch
without requiring a live Fabro API server.
- [ ] **Unit 2: Add string shorthand normalization for run create specs**
**Goal:** Make `runs: ["workflow"]` behave like
`runs: [{ "workflow": "workflow" }]`.
**Requirements:** R1, R2, R4
**Dependencies:** Unit 1
**Files:**
- Modify: `lib/crates/fabro-tool/src/create.rs`
- Test: `lib/crates/fabro-tool/src/create.rs`
- Test: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs`
**Approach:**
- Introduce a raw input representation for create specs that can deserialize
either a string workflow selector or the current object form.
- Normalize both raw forms into the existing validated create spec structure
before calling manifest resolution or backend methods.
- Preserve all existing object-form field handling and validation.
- Treat blank string workflows as invalid local input with a clear tool error.
**Patterns to follow:**
- `AnswerValue` in `lib/crates/fabro-tool/src/interact.rs` for custom
schema/deserialization where the MCP surface needs a flexible input value.
- `RunInputValue` in `lib/crates/fabro-tool/src/create.rs` for schema-driven
input constraints and local conversion.
**Test scenarios:**
- Happy path: `runs: ["simple.fabro"]` creates the same validated spec as
`runs: [{ "workflow": "simple.fabro" }]`.
- Happy path: object form with `dry_run`, `auto_approve`, `labels`, and
`start` continues to pass through unchanged.
- Edge case: `runs: [" "]` returns a local validation error naming the
workflow value.
- Error path: `runs: []` and 51 entries retain the existing min/max errors.
- Integration: string shorthand reaches the backend path in the MCP integration
harness, proving it is not rejected by the MCP deserializer.
**Verification:**
- Existing object-form MCP create tests still pass.
- String shorthand can start a run in the same manual scenario that previously
failed.
- [ ] **Unit 3: Make the advertised MCP schema client-friendly**
**Goal:** Ensure MCP clients can discover the actual supported input contract.
**Requirements:** R2, R3
**Dependencies:** Unit 2
**Files:**
- Modify: `lib/crates/fabro-tool/src/create.rs`
- Modify: `lib/crates/fabro-mcp-server/src/server.rs`
- Test: `lib/crates/fabro-mcp-server/src/server.rs`
- Test: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs`
**Approach:**
- Prefer a schema where `runs.items` clearly advertises both supported forms:
a workflow string shorthand and the object-form create spec.
- If `schemars` emits `$defs` that client tooling misinterprets, inline the
relevant schema or provide a manual `JsonSchema` implementation for the raw
create-spec input.
- Keep the schema descriptive rather than loosening it to arbitrary JSON.
**Patterns to follow:**
- Manual `JsonSchema` implementations in `RunInputValue` and `AnswerValue`.
- Existing MCP schema assertions in `mcp.rs` that verify property schemas are
objects and startup listing remains fast.
**Test scenarios:**
- Happy path: `tools/list` schema for `fabro_run_create` contains the string
shorthand branch.
- Happy path: `tools/list` schema for `fabro_run_create` contains the object
branch with `workflow`.
- Error path: schema does not advertise unsupported array/object input values
for `inputs`; existing scalar-only assertion remains true.
- Integration: listing tools still does not construct the API client.
**Verification:**
- An MCP client inspecting `tools/list` can infer at least one valid shape that
the runtime accepts.
- [ ] **Unit 4: Update docs and QA checklist**
**Goal:** Record the supported `fabro_run_create` shapes and the regression
test so future manual testing uses the right contract.
**Requirements:** R5
**Dependencies:** Unit 2, Unit 3
**Files:**
- Modify: `docs/public/agents/mcp.mdx`
- Modify: `docs/internal/mcp-server-qa-test-plan.md`
**Approach:**
- Add a small `fabro_run_create` example showing both shorthand and object
form, with object form recommended when options are needed.
- Add a QA note that this schema/runtime mismatch was fixed and should remain
covered by schema-discovery and shorthand-call tests.
**Patterns to follow:**
- Existing terse MCP tool table in `docs/public/agents/mcp.mdx`.
- Existing resolved issue notes at the top of
`docs/internal/mcp-server-qa-test-plan.md`.
**Test scenarios:**
- Test expectation: none -- documentation-only unit.
**Verification:**
- Public docs show an input shape that works when pasted into an MCP client.
- QA plan names the regression and where it is covered.
## System-Wide Impact
- **Interaction graph:** MCP clients call `tools/list`, infer parameter shape,
and then call `tools/call`; this fix aligns both surfaces with the same
deserializer.
- **Error propagation:** Invalid local input should continue to return MCP tool
errors without killing the stdio server. Framework-level JSON type errors
should only remain for truly unsupported JSON shapes.
- **State lifecycle risks:** No persistent data migration. The only durable
effect is successful run creation for shorthand calls that previously failed.
- **API surface parity:** HTTP run creation remains unchanged. This is an MCP
tool input compatibility fix.
- **Integration coverage:** Unit tests cover normalization; MCP stdio tests
cover real schema discovery and tool-call deserialization.
- **Unchanged invariants:** Object-form create specs remain the full-fidelity
path for labels, parent links, options, and overrides.
## Risks & Dependencies
| Risk | Mitigation |
|------|------------|
| Schema becomes too loose and agents send unsupported values | Use an explicit string-or-object schema and keep local validation narrow |
| Object-form callers regress while adding shorthand | Keep existing tests and add object-form pass-through assertions |
| MCP client tooling still summarizes the schema poorly | Make runtime accept the string shorthand so the summarized `string[]` shape still works |
| Validation accidentally moves after auth/network setup | Keep validation tests using an unreachable server target |
## Documentation / Operational Notes
- This fix should be called out as an MCP UX/compatibility fix, not an HTTP API
change.
- Manual verification should include the exact previously failing call:
`fabro_run_create({ "runs": ["sleeper"] })`.
## Sources & References
- Related code: `lib/crates/fabro-tool/src/create.rs`
- Related code: `lib/crates/fabro-mcp-server/src/server.rs`
- Related tests: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs`
- Related QA doc: `docs/internal/mcp-server-qa-test-plan.md`
- Related docs: `docs/public/agents/mcp.mdx`

55
docs/plans/2026-05-22-002-remove-session-sandboxes-feature-flag-plan.md Normal file
View file

@ -0,0 +1,55 @@
# Remove `features.session_sandboxes`
## Summary
Remove the `session_sandboxes` feature flag and the now-empty `[features]` settings namespace entirely. Behavior should be as if `session_sandboxes = true` was always set: Ask Fabro is never disabled by a feature flag, and UI controls previously hidden behind the flag are always shown.
## Key Changes
- Remove the settings namespace from config:
- Delete `FeaturesNamespace`, `FeaturesLayer`, `resolve_features`, and `[features]` defaults.
- Remove `features` from resolved `ServerSettings` and `UserSettings`.
- Remove `features` from the top-level settings parser allow-list, so old `[features]` config is rejected as unknown.
- Remove the runtime gate:
- Simplify Ask Fabro readiness to check only sandbox presence/runtime and LLM configuration.
- Remove `AskFabroUnavailableReason::FeatureDisabled` and the "Ask Fabro is disabled" tooltip.
- Update frontend behavior:
- Run detail page no longer handles `FEATURE_DISABLED`.
- Start page always renders the project/branch controls and no longer fetches system info just for this flag.
- Remove public API surfaces:
- `/api/v1/settings` `ServerSettings` no longer includes `features`.
- `/api/v1/system/info` no longer includes `features`.
- OpenAPI removes `FeaturesNamespace`, `SystemFeatures`, `ServerSettings.features`, `SystemInfoResponse.features`, and `feature_disabled`.
- Regenerate Rust API types and TypeScript Axios client.
- Update current docs:
- Remove `[features]` from active configuration docs, generated options docs, API docs, and unknown-key guidance.
- Do not touch unrelated meanings of "features" such as Cargo features, LLM model features, or devcontainer features.
## Test Plan
- Update or remove tests that assert `features.session_sandboxes` in config, settings, system info, and Ask Fabro readiness.
- Add or adjust coverage for:
- Ask Fabro unavailable reasons are only `no_sandbox`, `sandbox_not_ready`, or `llm_unconfigured`.
- Settings parsing rejects top-level `[features]`.
- `/api/v1/settings` response contains only `server` at the top level.
- `/api/v1/system/info` has no `features` field.
- Start page renders project/branch controls without consulting `SystemInfo.features`.
- Run:
- `cargo build -p fabro-api`
- `cd lib/packages/fabro-api-client && bun run generate`
- `cargo dev docs refresh && cargo dev docs check`
- `cargo nextest run -p fabro-config -p fabro-api -p fabro-server -p fabro-cli`
- `cd apps/fabro-web && bun test && bun run typecheck`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings`
## Acceptance Checks
- `rg -n "session_sandboxes|FeaturesNamespace|SystemFeatures|feature_disabled|Ask Fabro is disabled" lib apps docs/public` returns no relevant matches.
- `rg -n "\\[features\\]" docs/public lib/crates/fabro-config/src lib/crates/fabro-types/src lib/crates/fabro-server/src apps/fabro-web/app` returns no settings-namespace matches.
- Existing sandbox runtime behavior remains unchanged; only the feature flag and schema surface are removed.
## Assumptions
- This is intentionally a breaking config/API cleanup: existing user config containing `[features]` should fail validation until removed.
- Historical internal plans may still contain old text unless they are part of active public docs; implementation should prioritize product code, generated clients, and current docs.

160
docs/plans/2026-05-22-ask-fabro-sidebar-wiring.md Normal file
View file

@ -0,0 +1,160 @@
# Ask Fabro Sidebar Wiring — Implementation Plan
> **For agentic workers:** Use superpowers:subagent-driven-development or superpowers:executing-plans. Steps use `- [ ]` checkboxes.
**Goal:** Ship the Ask Fabro sidebar on run pages — wired to real session APIs, with the agent able to inspect its owning run.
**Architecture:** Two phases. Phase 1 (Rust): give Ask Fabro agent sessions the read-only `fabro_run_get` + `fabro_run_events` tools, scoped to the owning run, via the existing run-tool service path. Reuse `register_named_fabro_run_tools`; do not add a second subset registration helper. Phase 2 (web): replace the scripted sidebar adapter with real session calls; drop `?ask=1`, gate on `run.ask_fabro.available`.
**Tech Stack:** Rust (fabro-server, fabro-workflow, fabro-tool), React 19 + assistant-ui (fabro-web), generated API clients.
**Decisions locked:**
- Reuse `fabro-tool` — no new tool.
- Subset = `fabro_run_get` + `fabro_run_events`, read-only run inspection.
- Backend = existing `FabroRunToolServices` registration path. Prefer in-process server/store access when adding new same-process backends; avoid new loopback URL plumbing.
- Scoped to owning run — enforced by a same-run worker token; the API 403s cross-run calls.
- File/shell tools stay read-only in Ask Fabro sessions.
- Gate the sidebar on `run.ask_fabro.available`; drop `?ask=1`.
---
## Background (current state)
- API endpoints exist (`296fbddec`): `SessionDetail`, `/sessions/{id}/events`, `/sessions/{id}/attach`, turn submission w/ `x-fabro-turn-id`, `Run.ask_fabro` readiness. Web helpers exist: `session-stream.ts`, `sessionsApi`.
- Sidebar (`ask-fabro-sidebar.tsx`) is a prototype: scripted adapter (`chats-runtime.ts`/`chats-script.ts`), no API calls, gated behind `?ask=1` (`run-detail.tsx:363`).
- Ask Fabro sessions built by `build_agent_session` (`fabro-server/.../handler/sessions.rs`): profile + run sandbox + a read-only gate.
- `fabro-tool`: tools built on the `FabroToolBackend` trait. `FabroRunToolServices`, `register_fabro_run_tools`, `register_named_fabro_run_tools`, and `execute_fabro_run_tool` live in `fabro-workflow` (`handler/llm/api.rs`, `services.rs`).
- Worker tokens: `worker_token.rs` — `issue_worker_token(keys, &run_id)` mints a base same-run token; `AppState::worker_token_keys()` exposes the keys. `server.rs` already mints tokens this way for dispatched workers.
- `register_fabro_run_tools` is `pub(crate)`; `fabro-server` already depends on `fabro-workflow`.
---
## File structure
**Phase 1 — Rust**
- Modify: `lib/crates/fabro-server/src/server/handler/sessions.rs` — register the named read-only run tools and allowlist them in the gate.
**Phase 2 — Web**
- Create: `apps/fabro-web/app/lib/ask-fabro-runtime.ts` — real session adapter.
- Modify: `apps/fabro-web/app/components/chats/ask-fabro-sidebar.tsx` — use real adapter, take `runId`.
- Modify: `apps/fabro-web/app/routes/run-detail.tsx` — drop `?ask=1`, gate on `run.ask_fabro`.
- Delete (verify orphaned first): `apps/fabro-web/app/lib/chats-script.ts` + scripted paths in `chats-runtime.ts`.
---
## Phase 1: Run tools for Ask Fabro sessions
### Task 1 — Reuse named run-tool registration
- [ ] Use `register_named_fabro_run_tools` from `fabro-workflow/src/handler/llm/api.rs`.
- [ ] Register only `fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME` and `fabro_tool::FABRO_RUN_GET_TOOL_NAME`.
- [ ] Do not add another subset helper or duplicate the tool-catalog filtering loop.
- [ ] `cargo build --workspace`; `cargo nextest run -p fabro-workflow agent_run`.
### Task 2 — Wire scoped run tools into `build_agent_session`
The session's run-inspection backend is scoped to the owning run. The same-run token remains the authorization backstop for HTTP-backed calls, and any future in-process backend must enforce the same run-id check before executing a tool.
**Files:** `fabro-server/src/server/handler/sessions.rs`
- [ ] Change `build_profile` to return `Box<dyn AgentProfile>` (currently `Arc`); the caller registers tools on `&mut` then `Arc::from`s.
- [ ] In `build_agent_session`, after `build_profile`, before `Session::from_record`:
```rust
let worker_token = issue_worker_token(state.worker_token_keys(), &run_id)
.map_err(|err| AskFabroBuildError::Agent(anyhow::Error::new(err)))?;
// fabro_client::Client = generated reqwest client from the `fabro-client` crate.
let api_client = fabro_client::Client::new_with_client(
state.self_server_target()?,
reqwest_client_with_bearer(&worker_token),
);
let backend = fabro_tool::fabro_client::FabroClient::new(Arc::new(api_client));
let services = FabroRunToolServices {
backend: Arc::new(backend),
current_run_id: run_id,
base_cwd: PathBuf::new(), // unused by events/get
user_settings_path: PathBuf::new(), // unused by events/get
};
register_named_fabro_run_tools(
profile.tool_registry_mut(),
&services,
&[fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME, fabro_tool::FABRO_RUN_GET_TOOL_NAME],
);
```
Reference impls: the worker-token mint in `server.rs`; `FabroRunToolServices` construction in `fabro-cli/src/commands/run/runner.rs`.
- [ ] Ensure the session prompt/context names the owning run id so the agent passes the correct `run_id` to the tools. (Backstop: wrong id → API 403, agent self-corrects.)
- [ ] `cargo build --workspace`.
- [ ] Commit: `feat(fabro-server): give Ask Fabro sessions run-control tools`.
### Task 3 — Allowlist the two run tools in the session gate
`build_ask_fabro_tool_approval` (`sessions.rs`) currently denies everything not `ReadOnly`-approved. The two read-only run tools should be allowed; file/shell stay read-only.
- [ ] Update the closure:
```rust
Arc::new(move |tool_name: &str, _args: &Value| {
if matches!(tool_name, "fabro_run_get" | "fabro_run_events") {
return Ok(()); // read-only run-inspection tools, scoped by run id
}
if is_tool_auto_approved(PermissionLevel::ReadOnly, tool_name) {
Ok(())
} else {
Err(format!("{tool_name} tool denied by Ask Fabro tool policy"))
}
})
```
- [ ] Tests: `fabro_run_get` and `fabro_run_events` approved; `write_file` and shell denied; `read_file` approved.
- [ ] `cargo +nightly-2026-04-14 fmt --all && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings`.
- [ ] `cargo nextest run -p fabro-server --features test-support api::sessions`.
- [ ] Commit: `feat(fabro-server): allow run tools through the Ask Fabro session gate`.
### Task 4 — E2E coverage
- [ ] E2E test (twin), mirroring `tests/it/api/sessions.rs`: create a run with a sandbox, open an Ask Fabro session, submit a turn asking about run stages — assert the agent calls a read-only run tool and the turn completes.
- [ ] Cross-run guard test: a tool call with a different `run_id` is rejected (worker-token scope).
- [ ] `cargo nextest run -p fabro-server --features test-support --test it api::sessions`.
- [ ] Commit: `test(fabro-server): Ask Fabro run-tool E2E coverage`.
---
## Phase 2: Wire the sidebar
### Task 5 — Real session adapter
**Files:** Create `apps/fabro-web/app/lib/ask-fabro-runtime.ts`
- [ ] assistant-ui adapter parameterized by `runId`:
- First turn: `sessionsApi.createRunSession(runId, { model })` (model from `run.ask_fabro.default_model`); persist session id in `sessionStorage` keyed by `runId` so reopen resumes.
- Open with existing session id: `sessionsApi.getSession(id)` → render `SessionDetail.messages`, then `attachSessionEvents(id, { sinceSeq: last_seq })`.
- Send: `streamSessionTurn(id, { input })`; map streamed `EventEnvelope`s (incl. `run.session.*` tool-call events) to assistant-ui messages.
- [ ] Route tool-call events through the existing `tool-fallback.tsx` renderer.
- [ ] `bun test app/lib/ask-fabro-runtime.test.ts` (mock SSE as `session-stream.test.ts` does).
- [ ] Commit: `feat(web): real session adapter for Ask Fabro sidebar`.
### Task 6 — Sidebar uses the adapter
- [ ] `ask-fabro-sidebar.tsx`: accept a `runId` prop; replace `createScriptedAdapter` with `ask-fabro-runtime`; remove `EMPTY_CHAT`/`scriptIndexRef`.
- [ ] `rg createScriptedAdapter` — if `chats-script.ts`/scripted paths are orphaned, delete them.
- [ ] `bun run typecheck`.
- [ ] Commit: `feat(web): drive Ask Fabro sidebar from session API`.
### Task 7 — Drop `?ask=1`, gate on readiness
- [ ] `run-detail.tsx`: remove `askEnabled`/`searchParams.get("ask")` (lines ~363-368, 635-648, 724-730).
- [ ] Render the Ask Fabro button always; `disabled={!run.ask_fabro.available}`. Disabled tooltip from `unavailable_reason`: `no_sandbox`/`sandbox_not_ready` → "Run sandbox isn't ready"; `llm_unconfigured` → "No LLM configured".
- [ ] Pass `runId={params.id}` to `<AskFabroSidebar>`.
- [ ] `bun run typecheck && bun test`.
- [ ] Commit: `feat(web): enable Ask Fabro sidebar on run pages`.
---
## Tests to run before each PR
- Rust: `cargo +nightly-2026-04-14 fmt --check --all` · `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` · `cargo build --workspace` · `cargo nextest run -p fabro-server -p fabro-workflow`
- Web: `cd apps/fabro-web && bun run typecheck && bun test`
## Unresolved questions
1. **`fabro_run_get` payload size** — `get` may return a large `RunProjection` (all stage data). If it blows agent context, consider a trimmed projection. Verify before Task 4.
2. **Session reuse** — one Ask Fabro session per run reused across sidebar opens (plan assumes this via `sessionStorage`), or fresh each open?
3. **Capability scope** — Ask Fabro is read-only in this plan. Mutating run-control tools should be a separate product decision.
4. **Phase split** — Phase 1 + 2 as two PRs (Phase 2 works without 1; agent just lacks run tools), or ship together so the feature only appears once useful?

2
docs/public/reference/sdk.mdx
View file

@ -130,6 +130,8 @@ The `Sandbox` trait abstracts where tools execute — local filesystem, Docker c
```rust
#[async_trait]
pub trait Sandbox: Send + Sync {
async fn read_file_bytes(&self, path: &str) -> Result<Vec<u8>, String>;
async fn read_file_text(&self, path: &str) -> Result<String, String>;
async fn read_file(&self, path: &str, offset: Option<usize>, limit: Option<usize>) -> Result<String, String>;
async fn write_file(&self, path: &str, content: &str) -> Result<(), String>;
async fn delete_file(&self, path: &str) -> Result<(), String>;

438
docs/superpowers/plans/2026-05-22-agent-context-observability-events.md Normal file
View file

@ -0,0 +1,438 @@
# Agent Context Observability Events Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Add durable API/backend events that report loaded memory files, discovered and activated skills, and per-server MCP tool names for agent runs.
**Architecture:** Keep this API-backend scoped. Emit typed `AgentEvent` variants from the existing `fabro-agent` initialization and skill activation paths, convert them through `fabro-workflow` into durable `fabro-types` run events, and document the event contracts. Do not add run projection fields in this pass; consumers can read the event stream/history.
**Tech Stack:** Rust, Serde, Fabro agent/session events, Fabro workflow event conversion, Fabro MCP connection manager, `cargo nextest`.
---
## Scope
Implement these event changes:
- Add `agent.memory.loaded` with memory file paths, byte counts, loaded byte counts, truncation flags, provider profile, total loaded bytes, and budget bytes.
- Add `agent.skills.discovered` with source directories, provider profile, and sorted skill summaries.
- Add persisted `agent.skill.activated` for slash skill expansion and successful `use_skill` tool calls.
- Enrich `agent.mcp.ready` with names-only tool summaries: qualified tool name and original server tool name.
Do not implement ACP-native equivalents in this pass. Do not include memory file contents in any event payload. Do not include MCP tool descriptions or schemas.
## Existing Patterns To Follow
- Read `docs/internal/events-strategy.md` before changing event variants, names, conversion, or progress JSONL behavior.
- Read `docs/internal/testing-strategy.md` before adding or reorganizing tests.
- Follow the current `AgentEvent` flow:
- `lib/crates/fabro-agent/src/types.rs`
- `lib/crates/fabro-agent/src/session.rs`
- `lib/crates/fabro-workflow/src/handler/llm/api.rs`
- `lib/crates/fabro-workflow/src/event/convert.rs`
- `lib/crates/fabro-workflow/src/event/names.rs`
- `lib/crates/fabro-types/src/run_event/agent.rs`
- `lib/crates/fabro-types/src/run_event/mod.rs`
- Follow Rust import style from `AGENTS.md`: import types by name, import functions through their parent module, and avoid glob imports in production code.
## File Map
- Modify `lib/crates/fabro-types/src/run_event/agent.rs`: add new prop structs and extend `AgentMcpReadyProps`.
- Modify `lib/crates/fabro-types/src/run_event/mod.rs`: add `EventBody` variants for the new event names.
- Modify `lib/crates/fabro-agent/src/types.rs`: add internal `AgentEvent` variants, trace output, and noise filtering decisions.
- Modify `lib/crates/fabro-agent/src/memory.rs`: return memory content plus metadata instead of bare strings.
- Modify `lib/crates/fabro-agent/src/session.rs`: emit memory, skills, skill activation, and enriched MCP events.
- Modify `lib/crates/fabro-agent/src/skills.rs`: emit tool-sourced skill activation from `use_skill`.
- Modify `lib/crates/fabro-mcp/src/connection_manager.rs`: expose or support deterministic names-only tool summaries per server.
- Modify `lib/crates/fabro-workflow/src/event/convert.rs`: convert new agent events to durable event bodies.
- Modify `lib/crates/fabro-workflow/src/event/names.rs`: add event names.
- Modify `lib/crates/fabro-workflow/src/event/events.rs` only if the agent event name mapping also lives there for these variants.
- Modify `lib/crates/fabro-workflow/src/event/stored_fields.rs` only if a new event needs non-standard stored fields; otherwise rely on existing `Event::Agent` handling.
- Modify `docs/internal/events.md`: document new event shapes and the richer MCP payload.
- Add or update tests in `lib/crates/fabro-agent`, `lib/crates/fabro-mcp`, `lib/crates/fabro-types`, and `lib/crates/fabro-workflow`.
---
### Task 1: Add Typed Durable Event Contracts
**Files:**
- Modify: `lib/crates/fabro-types/src/run_event/agent.rs`
- Modify: `lib/crates/fabro-types/src/run_event/mod.rs`
- Test: existing `fabro-types` run event serde tests, or add focused coverage near the existing run event tests.
- [ ] **Step 1: Add agent memory props**
Add event prop structs with this shape:
```rust
pub struct AgentMemoryLoadedProps {
pub provider_profile: String,
pub files: Vec<AgentMemoryFileProps>,
pub total_loaded_bytes: usize,
pub budget_bytes: usize,
pub visit: u32,
}
pub struct AgentMemoryFileProps {
pub path: String,
pub byte_count: usize,
pub loaded_bytes: usize,
pub truncated: bool,
}
```
- [ ] **Step 2: Add skill props**
Add skill discovery and activation props:
```rust
pub struct AgentSkillsDiscoveredProps {
pub provider_profile: String,
pub source_dirs: Vec<String>,
pub skills: Vec<AgentSkillSummary>,
pub visit: u32,
}
pub struct AgentSkillSummary {
pub name: String,
pub description: String,
}
pub enum AgentSkillActivationSource {
Slash,
Tool,
}
pub struct AgentSkillActivatedProps {
pub skill_name: String,
pub source: AgentSkillActivationSource,
pub visit: u32,
}
```
Use serde names `slash` and `tool` for `AgentSkillActivationSource`. If a local enum string pattern already exists, follow that pattern.
- [ ] **Step 3: Extend MCP ready props**
Extend `AgentMcpReadyProps` with a backwards-compatible field:
```rust
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub tools: Vec<AgentMcpToolSummary>,
```
Add:
```rust
pub struct AgentMcpToolSummary {
pub name: String,
pub original_name: String,
}
```
- [ ] **Step 4: Add EventBody variants**
Add `EventBody` variants using these serialized event names:
- `agent.memory.loaded`
- `agent.skills.discovered`
- `agent.skill.activated`
Keep existing `agent.mcp.ready` name unchanged and only enrich its props.
- [ ] **Step 5: Add serde tests**
Cover:
- New event names serialize to the expected dot names.
- `AgentSkillActivationSource` serializes as `slash` and `tool`.
- Old `agent.mcp.ready` JSON without `tools` still deserializes with `tools == []`.
---
### Task 2: Add Internal Agent Events And Conversion
**Files:**
- Modify: `lib/crates/fabro-agent/src/types.rs`
- Modify: `lib/crates/fabro-workflow/src/event/convert.rs`
- Modify: `lib/crates/fabro-workflow/src/event/names.rs`
- Modify: `lib/crates/fabro-workflow/src/event/events.rs` if needed by the existing name mapping.
- Test: `lib/crates/fabro-workflow` event conversion tests.
- [ ] **Step 1: Add internal AgentEvent variants**
Add variants equivalent to:
```rust
MemoryLoaded {
provider_profile: String,
files: Vec<MemoryFileSummary>,
total_loaded_bytes: usize,
budget_bytes: usize,
}
SkillsDiscovered {
provider_profile: String,
source_dirs: Vec<String>,
skills: Vec<SkillSummary>,
}
SkillActivated {
skill_name: String,
source: SkillActivationSource,
}
McpServerReady {
server_name: String,
tool_count: usize,
tools: Vec<McpToolSummary>,
}
```
Prefer small shared internal structs near `AgentEvent` if that matches the existing file organization.
- [ ] **Step 2: Persist skill activation**
Do not classify `SkillActivated` as streaming noise. The existing `SkillExpanded` event is currently filtered before persistence; replace slash expansion emissions with `SkillActivated { source: Slash }` or keep `SkillExpanded` internal-only if removing it would create unnecessary churn.
- [ ] **Step 3: Add trace behavior**
Update `AgentEvent::trace` so the new events emit concise tracing summaries:
- memory loaded: profile, file count, total loaded bytes, budget bytes
- skills discovered: profile, skill count, source dir count
- skill activated: name and source
- MCP ready: server, count, and summary count
- [ ] **Step 4: Convert to durable events**
Update `fabro-workflow` event conversion so the new agent events map to the new `fabro-types` props and include `visit`.
- [ ] **Step 5: Add conversion tests**
Cover each new event with a focused conversion assertion that checks:
- durable event name
- `visit`
- core fields
- no memory content in the converted payload
---
### Task 3: Emit Memory Loaded Metadata
**Files:**
- Modify: `lib/crates/fabro-agent/src/memory.rs`
- Modify: `lib/crates/fabro-agent/src/session.rs`
- Test: relevant `fabro-agent` memory/session tests.
- [ ] **Step 1: Change memory discovery return type**
Change memory discovery from bare `Vec<String>` to a document type carrying both prompt content and event metadata:
```rust
pub struct MemoryDocument {
pub path: String,
pub content: String,
pub byte_count: usize,
pub loaded_bytes: usize,
pub truncated: bool,
}
```
Keep existing behavior unchanged:
- provider profile filename candidates stay the same
- root-to-working-dir walk stays the same
- content dedupe stays the same
- empty files are skipped
- total budget remains 32 KiB
- truncated content keeps the existing truncation marker
- [ ] **Step 2: Preserve prompt assembly behavior**
Adjust session/profile prompt assembly to pass only memory contents where prompt assembly expects memory text. The system prompt should be byte-for-byte equivalent except where existing tests allow non-semantic ordering differences.
- [ ] **Step 3: Emit agent.memory.loaded**
In `Session::initialize()`, emit `AgentEvent::MemoryLoaded` immediately after memory discovery, before skills and MCP initialization.
Emit the event even when no memory files are loaded. That lets consumers distinguish "no memory" from "not reported."
- [ ] **Step 4: Add memory tests**
Cover:
- loaded file path appears in event metadata
- `byte_count` is the original file byte count
- `loaded_bytes` reflects bytes actually loaded into the prompt budget
- `truncated` is true only for truncated files
- event payload never contains memory file contents
- empty discovery still emits a memory-loaded event with `files == []`
---
### Task 4: Emit Skills Discovered And Skill Activated
**Files:**
- Modify: `lib/crates/fabro-agent/src/session.rs`
- Modify: `lib/crates/fabro-agent/src/skills.rs`
- Test: relevant `fabro-agent` skill/session tests.
- [ ] **Step 1: Emit skills discovered**
After `discover_skills(...)`, emit `AgentEvent::SkillsDiscovered` with:
- `provider_profile`
- `source_dirs`
- sorted `skills: [{ name, description }]`
Emit the event even when no skills are discovered.
- [ ] **Step 2: Emit slash activation**
Where slash skill expansion currently emits or creates `SkillExpanded`, emit:
```rust
AgentEvent::SkillActivated {
skill_name,
source: SkillActivationSource::Slash,
}
```
- [ ] **Step 3: Emit tool activation**
In `make_use_skill_tool`, use `ToolContext::emit_agent_event(...)` after a requested skill is found and before returning the skill template. Emit:
```rust
AgentEvent::SkillActivated {
skill_name: name.to_string(),
source: SkillActivationSource::Tool,
}
```
Do not emit activation for failed `use_skill` lookups.
- [ ] **Step 4: Add skill tests**
Cover:
- discovery event includes all discovered skills sorted by name
- discovery event includes configured source directories
- empty discovery emits `skills == []`
- slash expansion emits `source == slash`
- successful `use_skill` emits `source == tool`
- failed `use_skill` does not emit activation
---
### Task 5: Enrich agent.mcp.ready With Names-Only Tool Summaries
**Files:**
- Modify: `lib/crates/fabro-mcp/src/connection_manager.rs`
- Modify: `lib/crates/fabro-agent/src/session.rs`
- Test: relevant `fabro-mcp` or `fabro-agent` MCP tests.
- [ ] **Step 1: Add deterministic tool summaries**
Expose a helper on `McpConnectionManager` or compute in `Session` from `all_tools()`:
- filter tools by `server_name`
- return qualified tool name as `name`
- return server-provided tool name as `original_name`
- sort by qualified `name`
- [ ] **Step 2: Enrich ready emissions**
When emitting `AgentEvent::McpServerReady`, include the tool summaries for that server. Keep existing `server_name` and `tool_count`.
- [ ] **Step 3: Add MCP tests**
Cover:
- ready event includes only tools from the ready server
- summaries are sorted by qualified name
- `name` is the Fabro-qualified MCP tool name
- `original_name` is the server-provided tool name
- descriptions and input schemas are not included
---
### Task 6: Update Event Documentation
**Files:**
- Modify: `docs/internal/events.md`
- [ ] **Step 1: Document new events**
Add sections for:
- `agent.memory.loaded`
- `agent.skills.discovered`
- `agent.skill.activated`
For `agent.memory.loaded`, explicitly state that file contents are excluded.
- [ ] **Step 2: Update MCP ready docs**
Update `agent.mcp.ready` to show:
```json
{
"server_name": "github",
"tool_count": 2,
"tools": [
{
"name": "mcp__github__create_issue",
"original_name": "create_issue"
}
],
"visit": 1
}
```
- [ ] **Step 3: Record skill event replacement**
If `agent.skill.expanded` remains in internal code or docs, mark it internal-only or replaced by `agent.skill.activated`.
---
### Task 7: Verify
**Files:**
- No new files unless test placement requires it.
- [ ] **Step 1: Run focused tests**
Run:
```bash
cargo nextest run -p fabro-agent -p fabro-workflow -p fabro-types -p fabro-mcp
```
- [ ] **Step 2: Run formatting**
Run:
```bash
cargo +nightly-2026-04-14 fmt --all
```
- [ ] **Step 3: Run clippy for touched crates or workspace**
Prefer the workspace command if time permits:
```bash
cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
```
- [ ] **Step 4: Final sanity checks**
Confirm:
- memory events never contain file contents
- skills discovered and memory loaded are emitted even for empty lists
- skill activation is persisted rather than filtered as streaming noise
- `agent.mcp.ready` remains backwards-compatible for old events without `tools`
- docs match the serialized event names and payload shapes

140
docs/superpowers/plans/2026-05-22-run-agent-fabro-tools-opt-in.md Normal file
View file

@ -0,0 +1,140 @@
# Run Agent Fabro Tools Opt-In Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Add `[run.agent] fabro_tools = true/false`, defaulting to `false`, so workflow agents only get Fabro run tools and the `agent:run_tools` worker JWT scope when a run opts in.
**Architecture:** Treat `run.agent.fabro_tools` as the source of truth in resolved run settings. The server reads the effective run setting before spawning `__run-worker` and issues the worker token with or without `agent:run_tools`. The CLI worker decodes the already-present worker token and registers Fabro run tools only when the scope contains both `run:worker` and `agent:run_tools`. Do not add a second worker-side env flag or hidden CLI argument for this capability; the signed JWT scope is the worker-side authority.
**Tech Stack:** Rust, Serde TOML config layers, Fabro worker JWT scopes, `cargo nextest`.
---
## File Map
- Modify `lib/crates/fabro-types/src/settings/run.rs`: add resolved `RunAgentSettings::fabro_tools`.
- Modify `lib/crates/fabro-config/src/layers/run.rs`: add optional layered `[run.agent] fabro_tools` and options metadata.
- Modify `lib/crates/fabro-config/src/resolve/run.rs`: resolve missing config to `false`.
- Modify `lib/crates/fabro-config/src/tests/resolve_run.rs`: cover default, true, false, and layer override behavior.
- Modify `lib/crates/fabro-server/src/worker_token.rs`: make the base worker scope constructor available to production code and retain `run_worker_with_agent_run_tools`.
- Modify `lib/crates/fabro-server/src/server.rs`: compute the opt-in flag from run settings and choose worker JWT scopes.
- Modify `lib/crates/fabro-server/src/server/tests.rs`: cover default and opted-in worker token scopes.
- Modify `lib/crates/fabro-cli/src/commands/run/runner.rs`: register `FabroRunToolServices` from the decoded worker token scope.
- Modify docs generator/reference docs: `lib/crates/fabro-dev/src/commands/docs_options_reference.rs`, `docs/public/reference/user-configuration.mdx`, and `docs/public/execution/run-configuration.mdx`.
---
## Task 1: Add Resolved Run Config
**Files:**
- Modify: `lib/crates/fabro-types/src/settings/run.rs`
- Modify: `lib/crates/fabro-config/src/layers/run.rs`
- Modify: `lib/crates/fabro-config/src/resolve/run.rs`
- Test: `lib/crates/fabro-config/src/tests/resolve_run.rs`
- [ ] Add resolver tests for default `false`, explicit `true`, explicit `false`, and higher-layer override behavior.
- [ ] Add `fabro_tools: bool` to `RunAgentSettings`.
- [ ] Add `fabro_tools: Option<bool>` to `RunAgentLayer` with `#[serde(default, skip_serializing_if = "Option::is_none")]` and options metadata.
- [ ] Resolve `agent.fabro_tools.unwrap_or(false)`.
- [ ] Run `cargo nextest run -p fabro-config run_agent_fabro_tools`.
---
## Task 2: Gate Worker JWT Scope
**Files:**
- Modify: `lib/crates/fabro-server/src/worker_token.rs`
- Modify: `lib/crates/fabro-server/src/server.rs`
- Test: `lib/crates/fabro-server/src/server/tests.rs`
- [ ] Add or keep constructors:
```rust
WorkerScopeSet::run_worker()
WorkerScopeSet::run_worker_with_agent_run_tools()
```
- [ ] Update worker command tests:
- default run token scopes are exactly `run:worker`
- opted-in run token scopes are exactly `run:worker agent:run_tools`
- [ ] Do not set a separate worker env var for Fabro tools.
- [ ] Load the effective setting from the run spec/settings available at worker-spawn time. If the current spawn path only exposes full projected run state, prefer a narrow run-spec/settings accessor or cached run record field over scanning/projecting full run history just to read this static setting.
- [ ] Pass the boolean into worker-token scope selection.
- [ ] Run `cargo nextest run -p fabro-server worker_command`.
---
## Task 3: Gate CLI Worker Tool Registration From JWT Scope
**Files:**
- Modify: `lib/crates/fabro-cli/src/commands/run/runner.rs`
- Test: `lib/crates/fabro-cli/src/commands/run/runner.rs`
- [ ] Add focused tests for `fabro_run_tools_enabled_from_worker_token`:
- invalid token -> false
- missing `scope` claim -> false
- `run:worker` only -> false
- `agent:run_tools` only -> false
- unknown extra scope -> false
- `run:worker agent:run_tools` -> true
- [ ] Decode only the unsigned claim locally for registration convenience. The server remains responsible for signature and scope enforcement.
- [ ] Gate `build_fabro_run_tool_services(...)` on `fabro_run_tools_enabled_from_worker_token(worker_token)`.
- [ ] Keep token presence as a second local guard inside `build_fabro_run_tool_services`.
- [ ] Run:
```bash
cargo nextest run -p fabro-cli fabro_run_tools_enabled_token_requires_run_tools_scope
cargo nextest run -p fabro-cli --test it runner
```
---
## Task 4: Update Docs And Generated Reference Text
**Files:**
- Modify: `lib/crates/fabro-dev/src/commands/docs_options_reference.rs`
- Modify: `docs/public/reference/user-configuration.mdx`
- Modify: `docs/public/execution/run-configuration.mdx`
- [ ] Add `fabro_tools = true` to the `[run.agent]` generated sample.
- [ ] Document that the setting defaults to `false`.
- [ ] Document that the setting controls built-in Fabro run-management tools and is separate from ordinary agent `permissions` and `[run.agent.mcps]`.
- [ ] Run:
```bash
cargo dev docs refresh
cargo dev docs check
```
---
## Full Verification
```bash
cargo nextest run -p fabro-config
cargo nextest run -p fabro-server
cargo nextest run -p fabro-cli
cargo +nightly-2026-04-14 fmt --check --all
cargo +nightly-2026-04-14 clippy -p fabro-types -p fabro-config -p fabro-server -p fabro-cli -p fabro-dev --all-targets -- -D warnings
```
## Acceptance Criteria
- Default run:
- resolved `run.agent.fabro_tools == false`
- worker JWT scope is `run:worker`
- `StartServices.fabro_run_tools == None`
- Opted-in run:
- resolved `run.agent.fabro_tools == true`
- worker JWT scope is `run:worker agent:run_tools`
- `StartServices.fabro_run_tools` is present
- No private worker env var or hidden CLI flag controls Fabro tool registration.
- Server-side authorization remains the enforcement point for the worker token signature, run id, and scopes.
## Assumptions And Defaults
- `fabro_tools` is a per-run opt-in setting only; this plan does not add a separate server-wide allow/deny policy.
- Defaulting to `false` intentionally changes existing behavior: runs that need Fabro run tools must set `[run.agent] fabro_tools = true`.
- `run.agent.permissions` remains about ordinary agent tool permissions and does not imply Fabro API access.
- `[run.agent.mcps]` remains independent; MCP tools are not enabled or disabled by `fabro_tools`.
- `fabro mcp start` and standalone MCP exposure of Fabro tools are out of scope.

291
docs/superpowers/plans/2026-05-22-unified-agent-transcript-events.md Normal file
View file

@ -0,0 +1,291 @@
# Unified Agent Transcript Events Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:subagent-driven-development` or `superpowers:executing-plans` to implement this plan task-by-task.
**Goal:** Make the ordered Fabro event stream sufficient to recreate an API-mode agent session without adding a parallel transcript event family.
**Architecture:** Extend existing `agent.message`, `agent.tool.started`, and `agent.tool.completed` event semantics. Messages are communication (`system`, `user`, `reasoning`, `agent`). Tool calls and tool results are actions, not messages. Persist only committed events; partial stream deltas, retries, and interrupted output are not replay sources.
**Out of scope:** Request metadata, compaction semantics, and broad store refactors.
---
## Key Decisions
- Use one shared Fabro transcript model in `fabro-types`; do not create parallel DTOs for events, API, store projection, and runtime history.
- Treat reasoning as a first-class message kind, not a tool call and not part of the visible agent answer.
- Keep model-role semantics (`kind`) separate from audit/source semantics (`source`).
- Keep tool calls/results as enriched action lifecycle records.
- Use event `seq` as the ordering source of truth.
- Keep run/session lifecycle events for lifecycle only; transcript replay comes from `agent.message` and `agent.tool.*`.
- Preserve provider replay payloads as structured parts, not strings.
## Type Ownership
Promote provider-neutral replay primitives from `fabro-llm` into `fabro-types`, then make `fabro-llm` import or re-export the canonical types.
Canonical shared types:
- `ContentPart`
- `ThinkingData`
- `ToolCall`
- `ToolResult`
- `TranscriptMessage`
- `MessageKind`
- `MessageSource`
- `PairMessageRef`
- existing `Principal` for actor attribution
Name the durable transcript type `TranscriptMessage`, not bare `Message`, to avoid import ambiguity with `fabro_agent::Message` and `fabro_llm::types::Message`. Do not add `AgentTranscriptPart` as a second `{ kind, data }` model if `ContentPart` can own the role. Event props must embed the canonical `ToolCall`, `ToolResult`, and `ContentPart` types directly. OpenAPI replacements should point generated API types at these canonical Rust types and include type identity / JSON parity tests.
## Interface Changes
Add shared transcript types in `fabro-types`:
```rust
TranscriptMessage {
id,
turn_id,
kind, // system | user | reasoning | agent
source, // system_prompt | turn_input | followup | steer | pair | injected_system | injected_user | loop_detection
actor: Option<Principal>,
pair: Option<PairMessageRef>,
content: Vec<ContentPart>,
provider,
model,
response_id,
usage,
}
PairMessageRef {
pair_id,
message_id,
client_message_id,
}
```
`kind` captures provider/model-role semantics for replay. `source` captures audit/UI origin. Steering is a source, not a role: steering that currently replays to the LLM as user-role input must be stored as `kind=user, source=steer`.
Extend existing durable events:
- `agent.message`
- Add `message: TranscriptMessage`.
- This becomes the canonical replay source for committed system, user, reasoning, and agent messages.
- Keep narrow `text`, `model`, `billing`, and `tool_call_count` fields until web/server/client consumers are migrated.
- `agent.tool.started`
- Add `tool_call: ToolCall`.
- Add `turn_id` and `parent_message_id`.
- Keep narrow `tool_name`, `tool_call_id`, and `arguments` fields until consumers are migrated.
- `agent.tool.completed`
- Add `tool_result: ToolResult`.
- Add `turn_id`.
- Keep narrow `tool_name`, `tool_call_id`, `output`, and `is_error` fields until consumers are migrated.
Provider replay requirements:
- OpenAI `openai_reasoning` and `openai_message` opaque items remain exact `ContentPart::Other` payloads.
- Anthropic thinking and redacted thinking remain `ContentPart::Thinking` payloads with signatures preserved.
- Gemini `thoughtSignature` remains `ToolCall.provider_metadata`.
- Reasoning messages can contain cleartext, redacted, signed, encrypted, or opaque provider parts, but implementation must not collapse these into plain strings.
Identity requirements:
- Add a canonical `MessageId` in `fabro-types`.
- `fabro-agent::Session` mints a `TurnId` for every `run_single_input()` invocation unless the caller supplies one.
- Ask Fabro passes its existing API `TurnId` into the agent session before processing.
- Workflow API-mode stages let the agent session mint a `TurnId`.
- The assistant/agent message id is minted before emitting tool calls. Tool calls emitted from that response use `parent_message_id = agent_message.id`.
## Implementation Tasks
### 1. Add Typed Event Contracts
Modify:
- `lib/crates/fabro-types/src/run_event/agent.rs`
- `lib/crates/fabro-types/src/run_event/session.rs`
- `lib/crates/fabro-types/src/run_event/mod.rs`
- `docs/public/api-reference/fabro-api.yaml` if exposed wire shapes change
Tasks:
- Move or re-home provider-neutral `ContentPart`, `ThinkingData`, `ToolCall`, and `ToolResult` into `fabro-types`.
- Add canonical `TranscriptMessage`, `MessageKind`, `MessageSource`, and `PairMessageRef` types in `fabro-types`.
- Extend `AgentMessageProps` to carry the canonical message payload.
- Extend tool started/completed props to carry canonical tool call/result payloads plus turn/message linkage.
- Keep serde defaults where needed so old event payloads continue to deserialize.
- Add `fabro-api` replacement tests for type identity and JSON parity when OpenAPI schemas map to canonical Rust types.
### 2. Emit Committed Messages From `fabro-agent`
Modify:
- `lib/crates/fabro-agent/src/types.rs`
- `lib/crates/fabro-agent/src/session.rs`
- `lib/crates/fabro-agent/src/history.rs`
Tasks:
- Replace or extend the narrow assistant-only `AgentEvent::AssistantMessage` path with a general committed `AgentEvent::Message`.
- Emit `kind=system, source=system_prompt` after the exact rendered system prompt is assembled.
- Emit `kind=user, source=turn_input` after skill expansion/wrapping, using the exact user message sent to the model.
- Emit `kind=user, source=followup` for follow-up inputs.
- Emit `kind=user, source=steer` for steering-as-user.
- Emit `kind=user, source=loop_detection` for loop-detection steering.
- Emit `kind=system, source=injected_system` for injected system messages.
- Emit `kind=user, source=injected_user` for injected user-role messages.
- Emit `kind=user, source=pair` for pair chat messages that enter LLM history, with `pair` populated.
- Emit `kind=system, source=pair` for pair join/leave or other pair system messages that enter LLM history, with `pair` populated.
- Emit `kind=reasoning` only for completed provider reasoning blocks that must be preserved for replay, preserving exact structured parts.
- Emit `kind=agent` after provider `Finish`, using the completed response content.
- Do not emit committed messages for deltas, retries, or interrupted partial output.
- Ensure all message events carry `turn_id`, `source`, and optional `actor`/`pair` metadata where applicable.
### 3. Enrich Tool Action Events
Modify:
- `lib/crates/fabro-agent/src/session.rs`
- `lib/crates/fabro-agent/src/tool_execution.rs`
- provider adapters only where extra metadata is not currently surfaced
Tasks:
- Preserve `ToolCall.tool_type`, `raw_arguments`, and `provider_metadata`.
- Preserve `ToolResult` structured output, error state, and supported media/artifact fields.
- Link every tool call to the owning agent message with `parent_message_id`.
- Mint the agent message id before tool execution so tool events can link correctly.
- Keep tool calls/results out of message events.
### 4. Persist Unified Events In Both API Paths
Modify:
- `lib/crates/fabro-workflow/src/handler/llm/api.rs`
- `lib/crates/fabro-workflow/src/event/convert.rs`
- `lib/crates/fabro-workflow/src/event/names.rs`
- `lib/crates/fabro-server/src/server/handler/sessions.rs`
Tasks:
- Convert the unified agent message event through the existing workflow `Event::Agent` path.
- Convert Ask Fabro/server session agent events into the same durable `agent.message` and `agent.tool.*` shapes.
- Keep `run.session.created`, `run.session.turn.started`, and terminal turn events as lifecycle events.
- Keep old `run.session.user_message`, `run.session.assistant_message`, and `run.session.tool_call.*` projection support until all producers and consumers are migrated.
- Prefer a shared event persistence helper for workflow and server session paths so redaction behavior is consistent.
- Avoid creating new transcript-specific event families.
Migration order:
1. Add canonical types and event deserialization support.
2. Update projection to read both old narrow run-session events and new unified agent events.
3. Switch workflow and Ask Fabro producers to emit unified events while retaining compatibility fields.
4. Update web/server/client consumers to prefer unified payloads with narrow-field fallback.
5. Only then consider deprecating narrow transcript-bearing run-session events.
### 5. Define Pair Transcript Relationship
Modify:
- `lib/crates/fabro-workflow/src/steering_hub.rs`
- `lib/crates/fabro-types/src/pair.rs`
- `lib/crates/fabro-server/src/server/handler/sessions.rs`
- web consumers of pair transcript events
Tasks:
- Treat `agent.pair.user_message` and `agent.pair.system_message` as UI/audit projection events only.
- Do not use pair transcript events as replay-authoritative session history.
- For any pair message that affects LLM history, emit the corresponding canonical `agent.message` event with `source=pair` and a populated `PairMessageRef`.
- Store pair user chat as `kind=user, source=pair`.
- Store pair join/leave or other pair system items that enter model context as `kind=system, source=pair`.
- Keep existing pair API transcript types as projections over pair events and canonical message references, not as a second replay model.
### 6. Rebuild Session Projection From Events
Modify:
- `lib/crates/fabro-store/src/run_sessions.rs`
- `lib/crates/fabro-types/src/session.rs`
- `lib/crates/fabro-agent/src/history.rs`
Tasks:
- Project runtime context from ordered `agent.message` and `agent.tool.*` events scoped by envelope `session_id`.
- Add a session-id/sequence index or incremental per-session transcript projection before relying on replay for hydration. Do not scan the full run event history and inspect every event payload for each session load.
- Preserve provider-specific reasoning, opaque provider items, response ids, usage, and tool metadata.
- Keep best-effort fallback projection for legacy narrow session events.
- Ignore pair transcript events for replay except as a legacy fallback path; canonical `agent.message` with `source=pair` is the replay source.
- Ensure `Session::from_record()` can hydrate without dropping provider parts needed for same-provider replay.
- Preserve injected history sources: rendered system prompt, wrapped user input, follow-up input, steering, injected system messages, injected user-role messages, and loop-detection steering.
### 7. Redaction And Security Policy
Modify:
- workflow event persistence path
- server session event persistence path
- event redaction utilities
Tasks:
- Define raw replay fields explicitly: provider opaque parts, raw tool arguments, provider metadata, and structured tool outputs.
- Apply one shared redaction policy before durable storage for both workflow and server sessions.
- Preserve replay-critical opaque provider fields unless they match an existing secret redaction rule.
- Do not omit fields needed for same-provider replay silently; if a field must be redacted, preserve the shape and mark the value redacted.
- Add tests covering raw tool arguments and provider metadata through both persistence paths.
### 8. Consumer Compatibility
Modify:
- web event consumers that currently read narrow `properties.text`
- web pair transcript consumers that read `agent.pair.*`
- server/API projections that expose session detail or event detail
- generated clients if OpenAPI changes
Tasks:
- Keep narrow compatibility fields in emitted events until consumers are updated.
- Update consumers to prefer `properties.message` and fall back to narrow fields.
- Keep pair transcript rendering backed by pair projection events, while ensuring session replay and hydration consume canonical `agent.message` events.
- Add web/server tests that render both old and new event shapes.
- Document the deprecation path for narrow transcript fields after consumer migration.
## Test Plan
- `fabro-types`: serde round trips for `agent.message`, enriched `agent.tool.started`, and enriched `agent.tool.completed`.
- Type ownership:
- canonical `ToolCall`, `ToolResult`, `ContentPart`, `TranscriptMessage`, usage, and event prop types are reused rather than duplicated
- OpenAPI replacement tests prove type identity and JSON parity where API schemas expose these shapes
- `fabro-agent`: committed system/user/reasoning/agent messages emit once, while partial deltas and interrupted streams do not create committed messages.
- `fabro-agent`: followups, steering-as-user, injected system messages, and loop-detection steering emit committed messages with the correct `kind`, `source`, and `turn_id`.
- Role/source mapping:
- steering-as-user emits `kind=user, source=steer`
- loop-detection steering emits `kind=user, source=loop_detection`
- injected user-role messages emit `kind=user, source=injected_user`
- pair user chat emits `kind=user, source=pair` with `PairMessageRef`
- pair join/leave context emits `kind=system, source=pair` with `PairMessageRef`
- Identity/linkage: tool calls include the parent agent message id minted before tool execution.
- Provider replay:
- OpenAI encrypted reasoning and opaque message items survive event replay.
- Anthropic thinking signatures survive event replay.
- Gemini thought signatures survive enriched tool call replay.
- `fabro-store`: session projection from event `seq` order recreates runtime history including provider parts and tool metadata.
- Pair projection: pair transcript events render in the pair UI/audit surface but do not create duplicate replay history when the canonical `source=pair` message exists.
- Migration: old narrow run-session events and new unified events both hydrate session detail without duplicate transcript entries.
- `fabro-server`: Ask Fabro stores the wrapped model input, not only the raw UI question.
- Redaction: workflow and server session persistence apply the same redaction behavior to raw arguments, provider metadata, and tool outputs.
- Consumer compatibility: existing UI/server consumers render old narrow fields and new unified message payloads.
- API conformance: OpenAPI-generated Rust/TypeScript clients still match the spec after schema updates.
## Acceptance Criteria
- A completed API-mode session can be reconstructed from the event stream without losing committed system, user, reasoning, agent, tool call, or tool result state.
- New transcript state is stored through existing semantic events, not a separate transcript event family.
- Tool calls remain actions, not messages.
- Partial output remains non-authoritative for replay.
- The implementation introduces one canonical set of replay types, not duplicated event/API/runtime DTOs.
- Steering, pair, and injected inputs preserve provider-role semantics in `kind` and audit/source semantics in `source`.
- Pair transcript events are UI/audit projection events, not a replay-authoritative transcript source.
- Ask Fabro migration is backward compatible for existing session events and projections.

381
docs/superpowers/plans/2026-05-23-llm-input-token-counting.md Normal file
View file

@ -0,0 +1,381 @@
# LLM Input Token Counting Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Add an optional LLM adapter capability that returns current input/context token counts, using provider-native counting when available and a deterministic local estimate otherwise.
**Architecture:** Token counting belongs in `fabro-llm` because each provider adapter owns the final provider-specific request serialization. `Client::count_input_tokens` will resolve and validate the request through the same provider path as `complete` and `stream`, try the adapter count API when requested, and fall back to a local estimate only for explicitly fallback-eligible failures. The returned value reports input/context size only, not billing usage.
**Tech Stack:** Rust, async-trait, serde/serde_json, fabro-http, httpmock, existing `fabro-llm` provider adapters.
---
## Scope And Decisions
- Build the reusable `fabro-llm` capability only. Session-level context breakdown, API endpoints, and UI rendering are follow-up work.
- Count input/context tokens only: model-visible messages, system/developer instructions, tools, tool choice, response schemas, and structured input content.
- Do not reuse `TokenCounts`; it includes output, reasoning, cache-read, and cache-write billing buckets.
- Prefer provider-native counting when callers choose it, but do not hide deterministic configuration, credential, request-shape, model-availability, content-filter, or context-length errors behind a local estimate.
- `PreferProvider` falls back only for unsupported adapters, timeout/network errors, rate limits, and provider 5xx/server errors.
- `RequireProvider` never returns a local estimate. It returns a provider count or an error.
- `EstimateOnly` still resolves and validates the provider/model, but does not call the adapter or send request content upstream.
- Privacy: `PreferProvider` and `RequireProvider` send the model-visible request to the upstream provider's token-count endpoint. That includes messages, system/developer instructions, tools, schemas, structured content, and media metadata/content according to provider serialization. `EstimateOnly` is the privacy-preserving mode.
## File Structure
- Create `lib/crates/fabro-llm/src/token_count.rs`
- Public token-counting result/preference types.
- Deterministic local estimator.
- Unit tests for estimator behavior.
- Modify `lib/crates/fabro-llm/src/lib.rs`
- Export the new module and public types.
- Modify `lib/crates/fabro-llm/src/provider.rs`
- Add the optional adapter method with a default unsupported implementation.
- Modify `lib/crates/fabro-llm/src/client.rs`
- Add `Client::count_input_tokens`.
- Add tests for fallback and preference behavior.
- Modify provider adapter files under `lib/crates/fabro-llm/src/providers/`
- Anthropic: count via `/messages/count_tokens`.
- Gemini: count via `models/{model}:countTokens`.
- OpenAI: count via `/responses/input_tokens`.
- OpenAI-compatible and Fabro-server adapters keep the default unsupported path.
## Task 1: Add Public Types And Local Estimator
**Files:**
- Create: `lib/crates/fabro-llm/src/token_count.rs`
- Modify: `lib/crates/fabro-llm/src/lib.rs`
- [ ] Define these public types in `token_count.rs`:
```rust
use serde::{Deserialize, Serialize};
use crate::types::{ContentPart, Request, ToolDefinition, Warning};
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum InputTokenCountPreference {
PreferProvider,
RequireProvider,
EstimateOnly,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum InputTokenCountMethod {
ProviderApi,
LocalEstimate,
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct InputTokenCount {
pub input_tokens: i64,
pub method: InputTokenCountMethod,
pub provider: String,
pub model: String,
#[serde(default)]
pub warnings: Vec<Warning>,
}
```
- [ ] Add `estimate_input_tokens(request: &Request, provider: impl Into<String>) -> InputTokenCount`.
- Use `InputTokenCountMethod::LocalEstimate`.
- Set `model` from `request.model`.
- Add deterministic warning codes as needed:
- `local_token_estimate`: every local estimate.
- `media_token_estimate`: media content was counted by a fixed heuristic or embedded byte estimate.
- `opaque_context_estimate`: opaque provider-specific context was serialized or approximated without provider semantics.
- `provider_options_estimate`: provider options may affect model-visible context and were counted by JSON-size heuristic.
- De-duplicate warnings by code so repeated media or opaque parts do not produce noisy results.
- [ ] Implement deterministic estimator helpers:
- `estimate_text_tokens(text)`: `text.chars().count().div_ceil(4)`; empty text counts as 0.
- `estimate_json_tokens(value)`: compact `serde_json::to_string(value)` length rounded up at 4 chars/token.
- Message overhead: 4 tokens per message plus 1 token per content part.
- Tool overhead: 8 tokens per tool plus estimated name, description, and schema JSON.
- Tool choice and response format: estimate their serialized JSON values.
- `provider_options`: estimate serialized JSON and add `provider_options_estimate`.
- Images: 2,000 token media floor plus metadata text/URL estimate.
- Audio/documents: estimate embedded byte length at 4 bytes/token; URL-only media uses a 2,000 token media floor plus metadata.
- File IDs and URL-only media: count the ID/URL text plus the 2,000 token media floor and add `media_token_estimate`.
- Embedded media bytes: count byte length divided by 4, rounded up, and add `media_token_estimate`.
- Gemini cached content options: estimate serialized `provider_options.gemini.cached_content` and add `provider_options_estimate`.
- `ContentPart::Other`: estimate serialized JSON and add `opaque_context_estimate`.
- OpenAI opaque previous-response/message/reasoning items in `ContentPart::Other`: estimate serialized JSON and add `opaque_context_estimate`.
- [ ] Export the module and public types from `lib.rs`:
```rust
pub mod token_count;
pub use token_count::{
InputTokenCount, InputTokenCountMethod, InputTokenCountPreference, estimate_input_tokens,
};
```
- [ ] Add estimator tests in `token_count.rs`:
- text-only request returns a positive local estimate
- adding a tool increases the estimate
- adding response format schema increases the estimate
- image/document/audio content gets a media warning or media-sized estimate
- provider options produce `provider_options_estimate`
- opaque `ContentPart::Other` produces `opaque_context_estimate`
- estimator is deterministic for the same request
Run:
```bash
cargo nextest run -p fabro-llm token_count
```
Expected: estimator tests pass.
## Task 2: Add Adapter Capability And Client Fallback
**Files:**
- Modify: `lib/crates/fabro-llm/src/provider.rs`
- Modify: `lib/crates/fabro-llm/src/client.rs`
- [ ] Extend `ProviderAdapter` with this default method:
```rust
async fn count_input_tokens(
&self,
_request: &Request,
) -> Result<Option<InputTokenCount>, Error> {
Ok(None)
}
```
- [ ] Add imports in `provider.rs` for `InputTokenCount`.
- [ ] Add `Client::count_input_tokens(&self, request: &Request, preference: InputTokenCountPreference) -> Result<InputTokenCount, Error>`.
- Call `self.validate_request_controls(request)?`.
- Resolve provider with `self.resolve_provider(request)?`.
- Call `provider.validate_request(request)?`.
- If preference is `EstimateOnly`, return `estimate_input_tokens(request, provider.name())`.
- If preference is `PreferProvider` or `RequireProvider`, call `provider.count_input_tokens(request).await`.
- Return provider result when it is `Ok(Some(count))`.
- In `PreferProvider`, return local estimate with warning code `provider_token_count_unsupported` when it is `Ok(None)`.
- In `RequireProvider`, return `Err(Error::Configuration { .. })` when it is `Ok(None)`.
- In `PreferProvider`, fallback to local estimate with warning code `provider_token_count_failed` only when the adapter returns:
- `Error::Network`
- `Error::RequestTimeout`
- `Error::Provider { kind: ProviderErrorKind::RateLimit, .. }`
- `Error::Provider { kind: ProviderErrorKind::Server, .. }`
- In `PreferProvider`, return the original error for:
- provider resolution errors
- `provider.validate_request` errors
- provider 400 invalid request / `ProviderErrorKind::InvalidRequest`
- authentication / access denied
- not found / model unavailable
- context-length and content-filter errors
- quota exceeded and every other provider error kind not explicitly listed as fallback-eligible
- configuration, unsupported tool choice, interrupt, invalid tool call, no-object, and stream errors
- In `RequireProvider`, return the original adapter error for every error kind; never fallback.
- Do not run completion/stream middleware for token counting.
- [ ] Add client tests using a mock adapter:
- provider result is returned when adapter returns `Ok(Some(_))`
- `PreferProvider` unsupported adapter returns local estimate with `provider_token_count_unsupported`
- `RequireProvider` unsupported adapter returns `Err`
- `PreferProvider` falls back for timeout, network, rate-limit, and server errors
- `PreferProvider` returns `Err` for invalid request, auth, access denied, not found, context length, content filter, quota exceeded, configuration, and unsupported tool choice errors
- `RequireProvider` returns `Err` for fallback-eligible provider errors
- `EstimateOnly` does not call the adapter method
- validation errors still return `Err`
Run:
```bash
cargo nextest run -p fabro-llm client::tests::count_input_tokens
```
Expected: new client tests pass.
## Task 3: Implement Anthropic Provider Counting
**Files:**
- Modify: `lib/crates/fabro-llm/src/providers/anthropic.rs`
- [ ] Reuse the existing request translation from `build_api_request(adapter, request, false).await` so count requests match normal Anthropic serialization.
- [ ] Add a private count request/response shape:
```rust
#[derive(serde::Serialize)]
struct CountTokensRequest {
model: String,
messages: Vec<ApiMessage>,
#[serde(skip_serializing_if = "Option::is_none")]
system: Option<serde_json::Value>,
#[serde(skip_serializing_if = "Option::is_none")]
tools: Option<Vec<ApiToolDef>>,
#[serde(skip_serializing_if = "Option::is_none")]
tool_choice: Option<serde_json::Value>,
#[serde(skip_serializing_if = "Option::is_none")]
thinking: Option<serde_json::Value>,
}
#[derive(serde::Deserialize)]
struct CountTokensResponse {
input_tokens: i64,
}
```
- [ ] Add `count_tokens_url()` returning `"{base_url}/messages/count_tokens"`.
- [ ] Implement `ProviderAdapter::count_input_tokens` for Anthropic:
- Build the translated API request.
- Send only count-supported input fields: `model`, `messages`, `system`, `tools`, `tool_choice`, and `thinking`.
- Do not send generation-only fields from the normal request body: `max_tokens`, `temperature`, `top_p`, `stop_sequences`, `output_config`, `speed`, `metadata`, or `stream`.
- Apply the same auth, `anthropic-version`, default headers, and beta headers needed for the translated request.
- Parse `input_tokens`.
- Return `InputTokenCount { method: ProviderApi, provider: self.provider_name.clone(), model: request.model.clone(), warnings: vec![] }`.
- [ ] Add `httpmock` tests:
- request path is `/messages/count_tokens`
- body includes translated `model`, `messages`, `system`, and `tools`
- reasoning-effort requests that normally produce `output_config` do not include `output_config` in the count body
- extended thinking configured through provider options is included as `thinking` when the normal translated request includes it
- response `{ "input_tokens": 123 }` returns `ProviderApi` with `123`
- non-2xx provider response surfaces from the adapter so the client fallback test can handle it
Run:
```bash
cargo nextest run -p fabro-llm providers::anthropic::tests::count_input_tokens
```
Expected: Anthropic count tests pass.
## Task 4: Implement Gemini Provider Counting
**Files:**
- Modify: `lib/crates/fabro-llm/src/providers/gemini.rs`
- [ ] Reuse `build_api_request(request).await` to build the normal Gemini `generateContent` body.
- [ ] Add a private response shape:
```rust
#[derive(serde::Deserialize)]
#[serde(rename_all = "camelCase")]
struct CountTokensResponse {
total_tokens: i64,
}
```
- [ ] Implement `ProviderAdapter::count_input_tokens` for Gemini:
- Resolve API model with `common::api_model_id(self.catalog.as_deref(), &request.model)`.
- POST to `"{base_url}/models/{api_model}:countTokens"`.
- Body is `{ "generateContentRequest": api_body }`.
- Apply `x-goog-api-key` and default headers as in `complete`.
- Parse `totalTokens`.
- Return `InputTokenCount` with `ProviderApi`.
- [ ] Add `httpmock` tests:
- request path is `/models/<model>:countTokens`
- body nests the generated request under `generateContentRequest`
- body does not include top-level `contents`, because Gemini requires `contents` and `generateContentRequest` to be mutually exclusive
- response `{ "totalTokens": 456 }` returns `456`
- tool declarations and system instructions are included through the reused translator
Run:
```bash
cargo nextest run -p fabro-llm providers::gemini::tests::count_input_tokens
```
Expected: Gemini count tests pass.
## Task 5: Implement OpenAI Provider Counting
**Files:**
- Modify: `lib/crates/fabro-llm/src/providers/openai.rs`
- [ ] Reuse `build_request_body_with_catalog(request, false, self.codex_mode, self.catalog.as_deref()).await`, then filter the body to the `/responses/input_tokens` allow-list.
- [ ] Keep only these top-level fields when present: `conversation`, `input`, `instructions`, `model`, `parallel_tool_calls`, `previous_response_id`, `reasoning`, `text`, `tool_choice`, `tools`, and `truncation`.
- [ ] Strip generation/storage/response-shaping fields from the count request body: `background`, `include`, `max_output_tokens`, `max_tool_calls`, `metadata`, `prompt`, `prompt_cache_key`, `safety_identifier`, `service_tier`, `store`, `stream`, `temperature`, `top_logprobs`, `top_p`, `user`, `stop`, and any Codex-only generated fields not in the allow-list.
- [ ] Add a private response shape matching the current Responses input-token API:
```rust
#[derive(serde::Deserialize)]
struct InputTokensResponse {
input_tokens: i64,
object: String,
}
```
- [ ] Implement `ProviderAdapter::count_input_tokens` for OpenAI:
- POST to `"{base_url}/responses/input_tokens"`.
- Use `self.build_request(&url).json(&filtered_request_body)` so auth/org/project/default headers match completion behavior.
- Validate `object == "response.input_tokens"`; otherwise return a network parse error.
- Parse top-level `input_tokens`.
- Return `InputTokenCount` with `ProviderApi`.
- [ ] Add `httpmock` tests:
- request path is `/responses/input_tokens`
- request body is exactly the allow-listed count body for messages, instructions, tools, reasoning, and response format
- request body strips `store`, `include`, `stream`, `max_output_tokens`, `metadata`, `temperature`, `top_p`, and `stop`
- response `{ "object": "response.input_tokens", "input_tokens": 789 }` returns `789`
- response with the wrong `object` returns an error
- `codex_mode` count uses the same serialization choices as Codex-mode streaming requests
Run:
```bash
cargo nextest run -p fabro-llm providers::openai::tests::count_input_tokens
```
Expected: OpenAI count tests pass.
## Task 6: Integration, Docs, And Final Verification
**Files:**
- Modify: `lib/crates/fabro-llm/README.md`
- [ ] Add a short README section showing:
- `client.count_input_tokens(&request, InputTokenCountPreference::PreferProvider).await`
- `RequireProvider` for callers that need provider semantics and must not accept estimates
- `EstimateOnly` as the privacy-preserving mode
- provider fallback behavior
- privacy/data exposure for provider-native counting: model-visible request content is sent to the provider token-count endpoint
- the distinction between `InputTokenCount` and billing `TokenCounts`
- [ ] Run formatting check:
```bash
cargo +nightly-2026-04-14 fmt --check --all
```
Expected: passes. If it fails only because new files need formatting, run `cargo +nightly-2026-04-14 fmt --all` and re-check.
- [ ] Run focused tests:
```bash
cargo nextest run -p fabro-llm
```
Expected: all `fabro-llm` tests pass.
- [ ] Run workspace build:
```bash
cargo build --workspace
```
Expected: workspace builds successfully.
## Acceptance Criteria
- Callers can ask the LLM client for input/context token count without sending a completion request.
- Anthropic, Gemini, and OpenAI adapters use provider-native counting endpoints.
- Unsupported adapters and provider count failures return deterministic local estimates only when the failure is explicitly fallback-eligible.
- `RequireProvider` never returns a local estimate.
- Provider count requests are filtered to fields accepted by each provider's count endpoint.
- README documents privacy implications of provider-native counting.
- The result cannot be mistaken for billing totals because it uses new input-token-specific types.
- Existing completion and streaming behavior is unchanged.

6
lib/crates/fabro-agent/README.md
View file

@ -81,7 +81,9 @@ Built-in profiles:
```rust
pub trait Sandbox: Send + Sync {
async fn read_file(&self, path: &str, offset: Option<usize>, limit: Option<usize>) -> Result<String, String>;
async fn read_file_bytes(&self, path: &str) -> Result<Vec<u8>, String>;
async fn read_file_text(&self, path: &str) -> Result<String, String>;
async fn read_file(&self, path: &str, offset: Option<usize>, limit: Option<usize>) -> Result<String, String>; // line-numbered display
async fn write_file(&self, path: &str, content: &str) -> Result<(), String>;
async fn exec_command(&self, command: &str, timeout_ms: u64, ...) -> Result<ExecResult, String>;
async fn grep(&self, pattern: &str, path: &str, options: &GrepOptions) -> Result<Vec<String>, String>;
@ -232,4 +234,4 @@ profile.register_subagent_tools(manager, factory, 0);
- **Tool output truncation** -- Per-tool character and line limits with head/tail or tail-only truncation modes
- **Environment variable filtering** -- `LocalSandbox` strips secrets (`*_API_KEY`, `*_SECRET`, `*_TOKEN`, `*_PASSWORD`, `*_CREDENTIAL`) from subprocess environments
- **Command timeouts** -- Configurable per-command with process group cleanup (SIGTERM then SIGKILL)
- **Project doc discovery** -- Automatically discovers `AGENTS.md`, `CLAUDE.md`, `GEMINI.md`, or `.codex/instructions.md` based on provider, with a 32KB budget
- **Project doc discovery** -- Automatically discovers `AGENTS.md`, `CLAUDE.md`, `GEMINI.md`, or `.codex/instructions.md` based on provider, with a 32KB budget

2
lib/crates/fabro-agent/src/memory.rs
View file

@ -46,7 +46,7 @@ pub async fn discover_memory(
return Err(Error::Interrupted(InterruptReason::Cancelled));
}
let path = format!("{dir}/{filename}");
let read_result = env.read_file(&path, None, None).await;
let read_result = env.read_file_text(&path).await;
if cancel_token.is_cancelled() {
return Err(Error::Interrupted(InterruptReason::Cancelled));
}

20
lib/crates/fabro-agent/src/session.rs
View file

@ -1,6 +1,6 @@
use std::collections::{HashMap, VecDeque};
use std::sync::{Arc, Mutex, RwLock};
use std::time::{Duration, SystemTime};
use std::time::SystemTime;
use fabro_auth::CredentialSource;
use fabro_llm::client::Client;
@ -1008,22 +1008,6 @@ impl Session {
}
}
fn retry_delay_for_error(
retry_policy: &RetryPolicy,
err: &LlmError,
attempt: u32,
) -> Option<Duration> {
if let Some(retry_after) = err.retry_after() {
let retry_after = Duration::from_secs_f64(retry_after);
if retry_after > retry_policy.backoff.max_delay {
return None;
}
Some(retry_after)
} else {
Some(retry_policy.backoff.delay_for_attempt(attempt + 1))
}
}
#[must_use]
pub fn followup_queue_handle(&self) -> Arc<Mutex<VecDeque<String>>> {
self.followup_queue.clone()
@ -1421,7 +1405,7 @@ impl Session {
let can_retry = err.retryable() && stream_attempt < STREAM_CONSUME_RETRIES;
let retry_attempt = u32::try_from(stream_attempt).unwrap_or(u32::MAX);
let retry_delay = can_retry
.then(|| Self::retry_delay_for_error(&retry_policy, &err, retry_attempt))
.then(|| retry::retry_delay(&retry_policy, &err, retry_attempt))
.flatten();
if let Some(delay) = retry_delay {

2
lib/crates/fabro-agent/src/skills.rs
View file

@ -255,7 +255,7 @@ pub async fn discover_skills(
if cancel_token.is_cancelled() {
return Err(Error::Interrupted(InterruptReason::Cancelled));
}
let read_result = env.read_file(&path, None, None).await;
let read_result = env.read_file_text(&path).await;
if cancel_token.is_cancelled() {
return Err(Error::Interrupted(InterruptReason::Cancelled));
}

179
lib/crates/fabro-agent/src/todo_tools.rs
View file

@ -57,133 +57,16 @@ fn parse_status(value: &str, allow_deleted: bool) -> Result<TodoStatus, String>
Ok(status)
}
const TASK_CREATE_DESCRIPTION: &str = r#"Use this tool to create a structured task list for your current coding session. This helps you track progress, organize complex tasks, and demonstrate thoroughness to the user.
It also helps the user understand the progress of the task and overall progress of their requests.
const TASK_CREATE_DESCRIPTION: &str = "Create pending tasks in the current session. \
Use concise subjects, descriptions, optional activeForm text for in-progress display, \
and metadata when callers need structured labels.";
## When to Use This Tool
const TASK_UPDATE_DESCRIPTION: &str = "Update an existing task's status, text, owner, \
metadata, or dependencies. Valid statuses are pending, in_progress, completed, and \
deleted; deleted removes a task from active work.";
Use this tool proactively in these scenarios:
- Complex multi-step tasks - When a task requires 3 or more distinct steps or actions
- Non-trivial and complex tasks - Tasks that require careful planning or multiple operations
- Plan mode - When using plan mode, create a task list to track the work
- User explicitly requests todo list - When the user directly asks you to use the todo list
- User provides multiple tasks - When users provide a list of things to be done, either numbered or comma-separated
- After receiving new instructions - Immediately capture user requirements as tasks
- When you start working on a task - Mark it as in_progress BEFORE beginning work
- After completing a task - Mark it as completed and add any new follow-up tasks discovered during implementation
## When NOT to Use This Tool
Skip using this tool when:
- There is only a single, straightforward task
- The task is trivial and tracking it provides no organizational benefit
- The task can be completed in less than 3 trivial steps
- The task is purely conversational or informational
NOTE that you should not use this tool if there is only one trivial task to do. In this case you are better off just doing the task directly.
## Task Fields
- **subject**: A brief, actionable title in imperative form, such as "Fix authentication bug in login flow"
- **description**: What needs to be done
- **activeForm** (optional): Present continuous form shown in the spinner when the task is in_progress, such as "Fixing authentication bug". If omitted, the spinner shows the subject instead.
All tasks are created with status `pending`.
## Tips
- Create tasks with clear, specific subjects that describe the outcome
- After creating tasks, use TaskUpdate to set up dependencies with addBlocks or addBlockedBy if needed
- Check TaskList first to avoid creating duplicate tasks"#;
const TASK_UPDATE_DESCRIPTION: &str = r#"Use this tool to update a task in the task list.
## When to Use This Tool
**Mark tasks as resolved:**
- When you have completed the work described in a task
- When a task is no longer needed or has been superseded
- Mark tasks as in_progress when you start working on them
- Mark tasks as completed immediately after finishing them
- ONLY mark a task as completed when you have FULLY accomplished it
- If you encounter errors, blockers, or cannot finish, keep the task as in_progress
- When blocked, create a new task describing what needs to be resolved
- Never mark a task as completed if tests are failing, implementation is partial, unresolved errors remain, or required files/dependencies could not be found
**Delete tasks:**
- When a task is no longer relevant or was created in error
- Set status to `deleted` to permanently remove a task
**Update task details:**
- When requirements change or become clearer
- When establishing dependencies between tasks
- When assigning task ownership
## Fields You Can Update
- **status**: Task status. See Status Workflow below.
- **subject**: Change the task title in imperative form, such as "Run tests"
- **description**: Change the task description
- **activeForm**: Present continuous form shown in the spinner when in_progress, such as "Running tests"
- **owner**: Change the task owner
- **metadata**: Merge metadata keys into the task. Set a key to null to delete it.
- **addBlocks**: Mark tasks that cannot start until this one completes
- **addBlockedBy**: Mark tasks that must complete before this one can start
## Status Workflow
Status progresses: `pending` -> `in_progress` -> `completed`.
Use `deleted` to permanently remove a task.
## Examples
Mark task as in progress when starting work:
```json
{"taskId": "1", "status": "in_progress"}
```
Mark task as completed after finishing work:
```json
{"taskId": "1", "status": "completed"}
```
Delete a task:
```json
{"taskId": "1", "status": "deleted"}
```
Set up task dependencies:
```json
{"taskId": "2", "addBlockedBy": ["1"]}
```"#;
const TASK_LIST_DESCRIPTION: &str = r"Use this tool to list all tasks in the task list.
## When to Use This Tool
- To see what tasks are available to work on
- To check overall progress on the project
- To find tasks that are blocked and need dependencies resolved
- After completing a task, to check for newly unblocked work or the next available task
- Prefer working on tasks in ID order, lowest ID first, when multiple tasks are available because earlier tasks often set up context for later ones
## Output
Returns a summary of each task:
- **id**: Task identifier to use with TaskUpdate
- **subject**: Brief description of the task
- **status**: pending, in_progress, or completed
- **owner**: Owner if assigned
- **blockedBy**: List of open task IDs that must be resolved first. Tasks with blockedBy entries should not be started until dependencies resolve.
Use TaskUpdate to change task status, owner, details, or dependencies.";
const TASK_LIST_DESCRIPTION: &str = "List tasks for the current session, including \
status, owner, and blocking dependencies.";
/// Deterministic todo id derived from `<list_id>::<step>`. Codex identifies
/// a plan step by the exact step text, so the projection ID is the
@ -731,7 +614,7 @@ mod tests {
}
#[test]
fn anthropic_task_tool_descriptions_include_claude_code_guidance() {
fn anthropic_task_tool_descriptions_are_concise() {
let runtime = Arc::new(TodoRuntime::new());
let create = make_task_create_tool(runtime.clone());
let update = make_task_update_tool(runtime.clone());
@ -741,43 +624,23 @@ mod tests {
create
.definition
.description
.contains("structured task list")
.contains("Create pending tasks")
);
assert!(
create
.definition
.description
.contains("## When to Use This Tool")
);
assert!(create.definition.description.contains("## Task Fields"));
assert!(create.definition.description.contains("TaskUpdate"));
assert!(create.definition.description.contains("TaskList"));
assert!(update.definition.description.contains("## Status Workflow"));
assert!(
update
.definition
.description
.contains("ONLY mark a task as completed")
);
assert!(
update
.definition
.description
.contains("status to `deleted`")
);
assert!(create.definition.description.contains("activeForm"));
assert!(update.definition.description.contains("pending"));
assert!(update.definition.description.contains("deleted"));
assert!(
list.definition
.description
.contains("## When to Use This Tool")
);
assert!(list.definition.description.contains("blocked"));
assert!(
list.definition
.description
.contains("Prefer working on tasks in ID order")
.contains("blocking dependencies")
);
let total_description_bytes = create.definition.description.len()
+ update.definition.description.len()
+ list.definition.description.len();
assert!(total_description_bytes < 600);
assert!(!create.definition.description.contains("##"));
assert!(!update.definition.description.contains("```"));
}
#[tokio::test]

26
lib/crates/fabro-agent/src/tools.rs
View file

@ -6,13 +6,14 @@ use fabro_llm::client::Client;
use fabro_llm::types::{Message, Request, ToolDefinition};
use fabro_model::ModelHandle;
use fabro_static::EnvVars;
use futures::future::join_all;
use futures::{StreamExt, stream};
use crate::config::SessionOptions;
use crate::sandbox::GrepOptions;
use crate::tool_registry::{RegisteredTool, ToolRegistry};
const MAX_WEB_FETCH_BYTES: usize = 100 * 1024;
const MAX_READ_MANY_FILES_CONCURRENCY: usize = 8;
/// Configuration for the optional LLM-based summarizer used by `web_fetch`.
#[derive(Clone)]
@ -387,27 +388,34 @@ pub(crate) fn make_read_many_files_tool() -> RegisteredTool {
},
executor: Arc::new(|args, ctx| {
Box::pin(async move {
let paths: Vec<&str> = args["paths"]
let paths: Vec<String> = args["paths"]
.as_array()
.ok_or_else(|| "paths must be an array".to_string())?
.iter()
.map(|p| {
p.as_str()
.ok_or_else(|| "each path must be a string".to_string())
.map(str::to_string)
})
.collect::<Result<_, _>>()?;
let reads = paths.iter().map(|path| {
let env = Arc::clone(&ctx.env);
async move { (*path, env.read_file(path, None, None).await) }
});
let results = join_all(reads).await;
let results = stream::iter(paths)
.map(|path| {
let env = Arc::clone(&ctx.env);
async move {
let result = env.read_file(&path, None, None).await;
(path, result)
}
})
.buffered(MAX_READ_MANY_FILES_CONCURRENCY)
.collect::<Vec<_>>()
.await;
let mut output = String::new();
for (path, result) in results {
match result {
Ok(content) => {
ctx.env.mark_agent_read(path);
ctx.env.mark_agent_read(&path);
let _ = write!(output, "=== {path} ===\n{content}\n\n");
}
Err(err) => {
@ -732,7 +740,7 @@ mod tests {
},
)
.await;
assert_eq!(result.unwrap(), "3 | line3\n4 | line4\n");
assert_eq!(result.unwrap(), "2 | line2\n3 | line3\n");
}
#[tokio::test]

28
lib/crates/fabro-llm/src/retry.rs
View file

@ -33,16 +33,8 @@ where
return Err(err);
}
// Check Retry-After
let delay = if let Some(retry_after) = err.retry_after() {
let retry_after_dur = Duration::from_secs_f64(retry_after);
if retry_after_dur > policy.backoff.max_delay {
return Err(err);
}
retry_after_dur
} else {
// Convert from 0-indexed (fabro-llm convention) to 1-indexed (BackoffPolicy)
policy.backoff.delay_for_attempt(attempt + 1)
let Some(delay) = retry_delay(policy, &err, attempt) else {
return Err(err);
};
warn!(
@ -64,6 +56,22 @@ where
}
}
/// Return the delay for a retryable attempt, or `None` when `Retry-After`
/// exceeds the configured maximum delay.
#[must_use]
pub fn retry_delay(policy: &RetryPolicy, err: &Error, attempt: u32) -> Option<Duration> {
if let Some(retry_after) = err.retry_after() {
let retry_after_dur = Duration::from_secs_f64(retry_after);
if retry_after_dur > policy.backoff.max_delay {
return None;
}
Some(retry_after_dur)
} else {
// Convert from 0-indexed (fabro-llm convention) to 1-indexed (BackoffPolicy).
Some(policy.backoff.delay_for_attempt(attempt + 1))
}
}
#[cfg(test)]
mod tests {
use std::sync::Arc;

22
lib/crates/fabro-sandbox/src/docker.rs
View file

@ -32,7 +32,7 @@ use crate::sandbox::{StdioProcessControl, optional_timeout, resolve_path};
use crate::{
CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector,
StdioProcess, StdioProcessHandle, StdioProcessTermination, shell_quote,
StdioProcess, StdioProcessHandle, StdioProcessTermination, format_lines_numbered, shell_quote,
};
pub(crate) const WORKING_DIRECTORY: &str = "/workspace";
@ -1657,6 +1657,26 @@ impl Sandbox for DockerSandbox {
self.download_file_bytes(path).await
}
async fn read_file(
&self,
path: &str,
offset: Option<usize>,
limit: Option<usize>,
) -> crate::Result<String> {
let container_path = self.resolve_container_path(path);
let (stdout, stderr, exit_code) = self
.docker_exec(vec!["cat".to_string(), container_path.clone()], None, None)
.await?;
if exit_code != 0 {
return Err(crate::Error::message(format!(
"Failed to read {container_path}: {stderr}"
)));
}
Ok(format_lines_numbered(&stdout, offset, limit))
}
async fn write_file(&self, path: &str, content: &str) -> crate::Result<()> {
self.upload_bytes_to_container(path, content.as_bytes())
.await

8
lib/crates/fabro-sandbox/src/sandbox.rs
View file

@ -493,12 +493,12 @@ pub type SandboxEventCallback = Arc<dyn Fn(SandboxEvent) + Send + Sync>;
/// Formats file content with line numbers for display.
///
/// Applies optional offset (0-based lines to skip) and limit (max lines to
/// return). Line numbers are 1-based and right-aligned.
/// Applies optional offset (1-based starting line number) and limit (max lines
/// to return). Line numbers are 1-based and right-aligned.
#[must_use]
pub fn format_lines_numbered(content: &str, offset: Option<usize>, limit: Option<usize>) -> String {
let all_lines: Vec<&str> = content.lines().collect();
let skip = offset.unwrap_or(0);
let skip = offset.unwrap_or(1).saturating_sub(1);
let take = limit.unwrap_or(all_lines.len());
let selected: Vec<&str> = all_lines.into_iter().skip(skip).take(take).collect();
let width = (skip + selected.len()).to_string().len().max(1);
@ -1485,7 +1485,7 @@ mod tests {
#[test]
fn format_lines_numbered_with_offset_limit() {
let result = format_lines_numbered("a\nb\nc\nd\ne", Some(1), Some(2));
let result = format_lines_numbered("a\nb\nc\nd\ne", Some(2), Some(2));
assert!(result.contains("2 | b"));
assert!(result.contains("3 | c"));
assert!(!result.contains("1 | a"));

10
lib/crates/fabro-sandbox/src/worktree.rs
View file

@ -287,16 +287,6 @@ impl Sandbox for WorktreeSandbox {
self.inner.read_file_bytes(&resolved).await
}
async fn read_file(
&self,
path: &str,
offset: Option<usize>,
limit: Option<usize>,
) -> crate::Result<String> {
let resolved = self.resolve_path(path);
self.inner.read_file(&resolved, offset, limit).await
}
async fn write_file(&self, path: &str, content: &str) -> crate::Result<()> {
let resolved = self.resolve_path(path);
self.inner.write_file(&resolved, content).await