checkpoint

⚒️ Generated with [Fabro](https://fabro.sh)
This commit is contained in:
Fabro 2026-05-22 09:14:50 -04:00
parent bcfbbd5b86
commit abd6a56c8f
6 changed files with 1005 additions and 52 deletions

358
run.json

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1,417 @@
diff --git a/apps/fabro-web/app/components/chats/ask-fabro-sidebar.tsx b/apps/fabro-web/app/components/chats/ask-fabro-sidebar.tsx
index fbba4b5cf..aab86f361 100644
--- a/apps/fabro-web/app/components/chats/ask-fabro-sidebar.tsx
+++ b/apps/fabro-web/app/components/chats/ask-fabro-sidebar.tsx
@@ -80,4 +80,4 @@ export default function AskFabroSidebar({
</div>
</aside>
);
-}
\ No newline at end of file
+}
diff --git a/apps/fabro-web/app/lib/ask-fabro-runtime.test.ts b/apps/fabro-web/app/lib/ask-fabro-runtime.test.ts
index 5d4b77ccf..9b4f07788 100644
--- a/apps/fabro-web/app/lib/ask-fabro-runtime.test.ts
+++ b/apps/fabro-web/app/lib/ask-fabro-runtime.test.ts
@@ -16,7 +16,6 @@ function event(name: string, properties: Record<string, unknown>): SessionStream
describe("applyTurnEvent", () => {
test("appends assistant deltas into a single streaming text part", () => {
const acc = {
- textParts: [],
activeTextIndex: null,
parts: [],
toolCallIndex: new Map(),
@@ -34,7 +33,6 @@ describe("applyTurnEvent", () => {
test("inserts a tool-call part and later attaches its result", () => {
const acc = {
- textParts: [],
activeTextIndex: null,
parts: [],
toolCallIndex: new Map(),
@@ -79,7 +77,6 @@ describe("applyTurnEvent", () => {
test("a text segment after a tool call starts a fresh text part", () => {
const acc = {
- textParts: [],
activeTextIndex: null,
parts: [],
toolCallIndex: new Map(),
diff --git a/apps/fabro-web/app/lib/ask-fabro-runtime.ts b/apps/fabro-web/app/lib/ask-fabro-runtime.ts
index 2e97ee5bc..92b2a0c2c 100644
--- a/apps/fabro-web/app/lib/ask-fabro-runtime.ts
+++ b/apps/fabro-web/app/lib/ask-fabro-runtime.ts
@@ -5,7 +5,6 @@ import type {
} from "@assistant-ui/react";
import {
- attachSessionEvents,
streamSessionTurn,
type SessionStreamEvent,
} from "./session-stream";
@@ -59,8 +58,6 @@ export interface AskFabroAdapterOptions {
persistedSession?: PersistedSessionState;
/** Override stream impl for tests. */
streamSessionTurnImpl?: typeof streamSessionTurn;
- /** Override attach impl for tests. */
- attachSessionEventsImpl?: typeof attachSessionEvents;
/** Override session API for tests. */
createSession?: (
runId: string,
@@ -75,7 +72,6 @@ export interface AskFabroAdapterOptions {
* streaming text and tool-call cards in real time.
*/
interface TurnAccumulator {
- textParts: Array<{ text: string }>;
/** Active text part index, if the last delta added/extended text. */
activeTextIndex: number | null;
parts: ThreadAssistantMessagePart[];
@@ -85,7 +81,6 @@ interface TurnAccumulator {
function emptyAccumulator(): TurnAccumulator {
return {
- textParts: [],
activeTextIndex: null,
parts: [],
toolCallIndex: new Map(),
@@ -121,18 +116,12 @@ export function applyTurnEvent(
const delta = typeof props.delta === "string" ? props.delta : "";
if (!delta) return false;
if (acc.activeTextIndex == null) {
- const textPart = { text: delta };
- acc.textParts.push(textPart);
acc.parts.push({ type: "text", text: delta });
acc.activeTextIndex = acc.parts.length - 1;
} else {
const part = acc.parts[acc.activeTextIndex];
if (part && part.type === "text") {
- const updated: ThreadAssistantMessagePart = {
- ...part,
- text: part.text + delta,
- };
- acc.parts[acc.activeTextIndex] = updated;
+ acc.parts[acc.activeTextIndex] = { ...part, text: part.text + delta };
}
}
return true;
@@ -205,27 +194,27 @@ function defaultCreateSession(
.then((response) => ({ id: response.data.id }));
}
+interface UserContentPart {
+ type?: unknown;
+ text?: unknown;
+}
+
function lastUserText(
- messages: ReadonlyArray<{ role: string; content: ReadonlyArray<unknown> }>,
+ messages: ReadonlyArray<{
+ role: string;
+ content: ReadonlyArray<UserContentPart>;
+ }>,
): string {
for (let i = messages.length - 1; i >= 0; i--) {
const message = messages[i];
if (!message || message.role !== "user") continue;
- const text = message.content
- .map((part) => {
- if (
- part &&
- typeof part === "object" &&
- (part as { type?: unknown }).type === "text" &&
- typeof (part as { text?: unknown }).text === "string"
- ) {
- return (part as { text: string }).text;
- }
- return "";
- })
- .filter(Boolean)
- .join("\n");
- if (text) return text;
+ const segments: string[] = [];
+ for (const part of message.content) {
+ if (part.type === "text" && typeof part.text === "string") {
+ segments.push(part.text);
+ }
+ }
+ if (segments.length > 0) return segments.join("\n");
}
return "";
}
@@ -265,40 +254,32 @@ export function createAskFabroAdapter(
const queue: SessionStreamEvent[] = [];
let resolveWaiter: (() => void) | null = null;
let streamDone = false;
- let streamError: unknown = null;
- const streamPromise = streamImpl({
- sessionId: id,
- input,
- signal: abortSignal,
- onEvent: (event) => {
- queue.push(event);
- if (resolveWaiter) {
- const r = resolveWaiter;
- resolveWaiter = null;
- r();
- }
- },
- }).then(
- () => {
- streamDone = true;
- if (resolveWaiter) {
- const r = resolveWaiter;
- resolveWaiter = null;
- r();
- }
- },
- (err) => {
- streamError = err;
+ function wakeWaiter() {
+ if (!resolveWaiter) return;
+ const r = resolveWaiter;
+ resolveWaiter = null;
+ r();
+ }
+
+ const streamPromise = (async () => {
+ try {
+ await streamImpl({
+ sessionId: id,
+ input,
+ signal: abortSignal,
+ onEvent: (event) => {
+ queue.push(event);
+ wakeWaiter();
+ },
+ });
+ } finally {
streamDone = true;
- if (resolveWaiter) {
- const r = resolveWaiter;
- resolveWaiter = null;
- r();
- }
- },
- );
+ wakeWaiter();
+ }
+ })();
+ let yielded = false;
while (true) {
if (queue.length === 0) {
if (streamDone) break;
@@ -311,12 +292,14 @@ export function createAskFabroAdapter(
if (!event) continue;
if (applyTurnEvent(acc, event)) {
yield snapshot(acc);
+ yielded = true;
}
}
+ // Propagate any error from the stream task.
await streamPromise;
- if (streamError) throw streamError;
- yield snapshot(acc);
+ // Guarantee assistant-ui sees at least one result for an empty turn.
+ if (!yielded) yield snapshot(acc);
},
};
-}
\ No newline at end of file
+}
diff --git a/apps/fabro-web/app/routes/ask-fabro.tsx b/apps/fabro-web/app/routes/ask-fabro.tsx
index c3642db02..29194ad31 100644
--- a/apps/fabro-web/app/routes/ask-fabro.tsx
+++ b/apps/fabro-web/app/routes/ask-fabro.tsx
@@ -152,4 +152,4 @@ const RUN_ROWS = [
{ id: "r_8f27", title: "Bump assistant-ui to latest", duration: "0m 47s", dot: "bg-mint" },
{ id: "r_8f26", title: "Generate release notes for v0.42", duration: "1m 19s", dot: "bg-mint" },
{ id: "r_8f25", title: "Audit unused dependencies", duration: "5m 33s", dot: "bg-coral" },
-];
\ No newline at end of file
+];
diff --git a/apps/fabro-web/app/routes/run-detail.tsx b/apps/fabro-web/app/routes/run-detail.tsx
index 8a8b917a0..77eab29bc 100644
--- a/apps/fabro-web/app/routes/run-detail.tsx
+++ b/apps/fabro-web/app/routes/run-detail.tsx
@@ -28,7 +28,10 @@ import { Menu, MenuButton, MenuItem, MenuItems } from "@headlessui/react";
import AskFabroSidebar, {
SIDEBAR_WIDTH,
} from "../components/chats/ask-fabro-sidebar";
-import type { AskFabro } from "@qltysh/fabro-api-client";
+import {
+ AskFabroUnavailableReasonEnum,
+ type AskFabro,
+} from "@qltysh/fabro-api-client";
import { EditableRunTitle } from "../components/editable-run-title";
import { GitPullRequestIcon } from "../components/icons";
import { InterviewDock } from "../components/interview-dock";
@@ -737,11 +740,14 @@ function isLifecycleActionFailure(
return "ok" in value && value.ok === false;
}
-const ASK_FABRO_UNAVAILABLE_TOOLTIPS: Record<string, string> = {
- feature_disabled: "Ask Fabro is disabled",
- no_sandbox: "Run sandbox isn't ready",
- sandbox_not_ready: "Run sandbox isn't ready",
- llm_unconfigured: "No LLM configured",
+const ASK_FABRO_UNAVAILABLE_TOOLTIPS: Record<
+ AskFabroUnavailableReasonEnum,
+ string
+> = {
+ [AskFabroUnavailableReasonEnum.FEATURE_DISABLED]: "Ask Fabro is disabled",
+ [AskFabroUnavailableReasonEnum.NO_SANDBOX]: "Run sandbox isn't ready",
+ [AskFabroUnavailableReasonEnum.SANDBOX_NOT_READY]:"Run sandbox isn't ready",
+ [AskFabroUnavailableReasonEnum.LLM_UNCONFIGURED]: "No LLM configured",
};
function AskFabroTriggerButton({
@@ -991,4 +997,4 @@ function ActionsMenu(props: ActionsMenuProps) {
</MenuItems>
</Menu>
);
-}
\ No newline at end of file
+}
diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml
index 67fe98c99..f83c73981 100644
--- a/lib/crates/fabro-server/Cargo.toml
+++ b/lib/crates/fabro-server/Cargo.toml
@@ -113,4 +113,4 @@ tokio-util.workspace = true
fabro-macros = { path = "../fabro-macros" }
fabro-sandbox = { path = "../fabro-sandbox", features = ["test-support"] }
fabro-test = { workspace = true }
-fabro-types = { path = "../fabro-types", features = ["test-support"] }
\ No newline at end of file
+fabro-types = { path = "../fabro-types", features = ["test-support"] }
diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs
index d71ee3b3f..9aa113011 100644
--- a/lib/crates/fabro-server/src/server.rs
+++ b/lib/crates/fabro-server/src/server.rs
@@ -960,12 +960,9 @@ impl AppState {
runtime_directory.record_path().display()
)
})?;
- let target = daemon.bind.to_target();
- if daemon.bind.tcp_port().is_some() {
- fabro_client::ServerTarget::http_url(target)
- } else {
- fabro_client::ServerTarget::unix_socket_path(target)
- }
+ // `Bind::to_target()` already produces the http(s)-URL-or-absolute-
+ // socket-path form that `ServerTarget`'s FromStr understands.
+ daemon.bind.to_target().parse()
}
pub(crate) fn resolve_interp(&self, value: &InterpString) -> anyhow::Result<String> {
diff --git a/lib/crates/fabro-server/src/server/handler/sessions.rs b/lib/crates/fabro-server/src/server/handler/sessions.rs
index acd3d4127..123719cfa 100644
--- a/lib/crates/fabro-server/src/server/handler/sessions.rs
+++ b/lib/crates/fabro-server/src/server/handler/sessions.rs
@@ -35,7 +35,7 @@ use fabro_types::settings::{ModelRef as SettingsModelRef, ModelRegistry, Resolve
use fabro_types::{
EventBody, EventEnvelope, PermissionLevel, RunEvent, RunId, SessionDetail, SessionId, TurnId,
};
-use fabro_workflow::handler::llm::api::register_fabro_run_tools_subset;
+use fabro_workflow::handler::llm::api::register_named_fabro_run_tools;
use fabro_workflow::services::FabroRunToolServices;
use serde_json::Value;
use tokio::sync::broadcast::error::RecvError;
@@ -704,7 +704,7 @@ async fn build_agent_session(
&run_id,
WorkerScopeSet::run_worker_with_agent_run_tools(),
)
- .map_err(|err| AskFabroBuildError::Agent(anyhow::anyhow!("{err:?}")))?;
+ .map_err(|_| AskFabroBuildError::Agent(anyhow::anyhow!("failed to sign worker token")))?;
let target = state
.self_server_target()
.map_err(AskFabroBuildError::Agent)?;
@@ -721,7 +721,7 @@ async fn build_agent_session(
base_cwd: PathBuf::new(),
user_settings_path: PathBuf::new(),
};
- register_fabro_run_tools_subset(profile.tool_registry_mut(), &services, &[
+ register_named_fabro_run_tools(profile.tool_registry_mut(), &services, &[
fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME,
fabro_tool::FABRO_RUN_INTERACT_TOOL_NAME,
]);
diff --git a/lib/crates/fabro-workflow/src/handler/llm/api.rs b/lib/crates/fabro-workflow/src/handler/llm/api.rs
index 6970474e2..db94ecd5d 100644
--- a/lib/crates/fabro-workflow/src/handler/llm/api.rs
+++ b/lib/crates/fabro-workflow/src/handler/llm/api.rs
@@ -192,18 +192,22 @@ fn build_profile(
}
pub fn register_fabro_run_tools(registry: &mut ToolRegistry, services: &FabroRunToolServices) {
- register_fabro_run_tools_subset(registry, services, &[]);
+ for definition in fabro_tool::tool_definitions() {
+ registry.register(fabro_run_tool(definition, services.clone()));
+ }
}
-/// Register a subset of Fabro run tools by tool name. Pass an empty `only`
-/// slice to register the full set (matches `register_fabro_run_tools`).
-pub fn register_fabro_run_tools_subset(
+/// Register only the Fabro run tools whose names appear in `names`.
+///
+/// Unknown names are silently ignored so callers can list every tool they
+/// care about without depending on the current `fabro_tool` catalog.
+pub fn register_named_fabro_run_tools(
registry: &mut ToolRegistry,
services: &FabroRunToolServices,
- only: &[&str],
+ names: &[&str],
) {
for definition in fabro_tool::tool_definitions() {
- if only.is_empty() || only.contains(&definition.name) {
+ if names.contains(&definition.name) {
registry.register(fabro_run_tool(definition, services.clone()));
}
}
@@ -1466,10 +1470,10 @@ mod tests {
}
#[test]
- fn agent_run_tools_subset_registers_only_listed_tools() {
+ fn register_named_fabro_run_tools_registers_only_listed_tools() {
let mut registry = ToolRegistry::new();
let (services, _backend) = fabro_run_tool_services();
- register_fabro_run_tools_subset(&mut registry, &services, &[
+ register_named_fabro_run_tools(&mut registry, &services, &[
fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME,
fabro_tool::FABRO_RUN_INTERACT_TOOL_NAME,
]);
@@ -1487,18 +1491,20 @@ mod tests {
}
#[test]
- fn agent_run_tools_subset_empty_only_registers_all() {
- let mut registry_full = ToolRegistry::new();
- let mut registry_subset = ToolRegistry::new();
+ fn register_named_fabro_run_tools_ignores_unknown_names() {
+ let mut registry = ToolRegistry::new();
let (services, _backend) = fabro_run_tool_services();
- register_fabro_run_tools(&mut registry_full, &services);
- register_fabro_run_tools_subset(&mut registry_subset, &services, &[]);
-
- let mut full = registry_full.names();
- let mut subset = registry_subset.names();
- full.sort();
- subset.sort();
- assert_eq!(full, subset);
+ register_named_fabro_run_tools(&mut registry, &services, &[
+ fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME,
+ "not_a_real_tool",
+ ]);
+
+ let registered = registry
+ .names()
+ .into_iter()
+ .filter(|name| name.starts_with("fabro_run_"))
+ .collect::<Vec<_>>();
+ assert_eq!(registered, vec![fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME]);
}
#[tokio::test]

View file

@ -0,0 +1,6 @@
{
"outcome": "succeeded",
"notes": "Stage completed: simplify_opus",
"failure_reason": null,
"timestamp": "2026-05-22T13:05:12.923690Z"
}

View file

@ -0,0 +1,251 @@
Goal: # 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 *and control* its owning run.
**Architecture:** Two phases. Phase 1 (Rust): give Ask Fabro agent sessions the full `fabro_run_interact` + `fabro_run_events` tools, scoped to the owning run, via the existing HTTP `FabroClient` backend. 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_interact` + `fabro_run_events`, **full access** (incl. mutating actions: start/cancel/steer/archive/answer).
- Backend = existing HTTP `FabroClient` (already implements all reads + mutations).
- 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 (only the two run tools get full access).
- 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:633`): profile + run sandbox + `ReadOnly` gate (`build_ask_fabro_tool_approval`, `sessions.rs:867`). **No `fabro_run_*` tools.**
- `fabro-tool`: tools built on the `FabroToolBackend` trait. `fabro_client::FabroClient` is the HTTP impl — already implements every trait method (reads + mutations). `FabroRunToolServices`, `register_fabro_run_tools`, `execute_fabro_run_tool` live in `fabro-workflow` (`handler/llm/api.rs`, `services.rs`); `register_fabro_run_tools` registers all 5 tools.
- 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-workflow/src/handler/llm/api.rs` — make `register_fabro_run_tools` `pub`; add subset variant.
- Modify: `lib/crates/fabro-server/src/server/handler/sessions.rs``build_profile` returns `Box`; mint worker token, build `FabroClient`, register subset; allowlist the two tools 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 — Make run-tool registration callable from fabro-server
- [ ] In `fabro-workflow/src/handler/llm/api.rs`: change `register_fabro_run_tools` from `pub(crate)` to `pub`. Add a subset variant:
```rust
pub fn register_fabro_run_tools_subset(
registry: &mut ToolRegistry,
services: &FabroRunToolServices,
only: &[&str],
) {
for definition in fabro_tool::tool_definitions() {
if only.is_empty() || only.contains(&definition.name) {
registry.register(fabro_run_tool(definition, services.clone()));
}
}
}
```
Refactor `register_fabro_run_tools` to call it with `&[]`. `fabro_run_tool` stays private.
- [ ] Confirm `FabroRunToolServices` (`fabro-workflow/src/services.rs`) is `pub` — it is. No change.
- [ ] `cargo build --workspace`; `cargo nextest run -p fabro-workflow agent_run`.
- [ ] Commit: `refactor(fabro-workflow): expose run-tool registration with tool subset`.
### Task 2 — Wire FabroClient + worker token into `build_agent_session`
The session's run-control backend is the HTTP `FabroClient` pointed at the server's own API, authed with a same-run worker token. The token enforces run scoping (cross-run calls 403).
**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_base_url(), // server's own loopback base URL
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/interact
user_settings_path: PathBuf::new(), // unused by events/interact
};
register_fabro_run_tools_subset(
profile.tool_registry_mut(),
&services,
&[fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME, fabro_tool::FABRO_RUN_INTERACT_TOOL_NAME],
);
```
Reference impls: the worker-token mint in `server.rs`; `FabroRunToolServices` construction in `fabro-cli/src/commands/run/runner.rs`.
- [ ] Resolve `self_base_url()` — the server's own loopback address. If `AppState` doesn't already expose it, add an accessor from the bound listen addr (`http://127.0.0.1:<port>`). Local-only call; never the public URL.
- [ ] 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:867`) currently denies everything not `ReadOnly`-approved. The two run tools need full access; file/shell stay read-only.
- [ ] Update the closure:
```rust
Arc::new(move |tool_name: &str, _args: &Value| {
if matches!(tool_name, "fabro_run_interact" | "fabro_run_events") {
return Ok(()); // run-control tools: full access, scoped by worker token
}
if is_tool_auto_approved(PermissionLevel::ReadOnly, tool_name) {
Ok(())
} else {
Err(format!("{tool_name} tool denied by Ask Fabro tool policy"))
}
})
```
- [ ] Rename `build_ask_fabro_tool_approval` comment / any "read-only policy" wording — the session is no longer read-only (it can control its run via the API).
- [ ] Tests: `fabro_run_interact` 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 run tool and the turn completes. Add a second case: a turn that triggers a mutating `fabro_run_interact` action (e.g. `questions`/`answer` against a run with a pending question) succeeds.
- [ ] 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`: `feature_disabled` → "Ask Fabro is disabled"; `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. **`interact: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. **Server self-base-URL** — does `AppState` already expose its bound loopback address? If not, Task 2 must add an accessor. Confirm the server always binds a loopback-reachable addr (vs. a unix socket only — see `server.listen`).
3. **Session reuse** — one Ask Fabro session per run reused across sidebar opens (plan assumes this via `sessionStorage`), or fresh each open?
4. **Capability scope** — Ask Fabro can now cancel/archive/steer/answer its run. Confirm that's the intended product surface; consider whether `archive`/`unarchive` should be excluded even though `interact` is otherwise full-access.
5. **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?
## Completed stages
- **toolchain**: succeeded
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
- Output:
```
cargo 1.95.0 (f2d3ce0bd 2026-03-21)
```
- **preflight_compile**: succeeded
- Script: `cargo check -q --workspace 2>&1`
- Output: (empty)
- **preflight_lint**: succeeded
- Script: `cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1`
- Output: (empty)
- **implement**: succeeded
- Model: claude-opus-4-7, 271.6k tokens in / 71.0k out
- Files: /home/daytona/workspace/fabro/apps/fabro-web/app/components/chats/ask-fabro-sidebar.tsx, /home/daytona/workspace/fabro/apps/fabro-web/app/lib/ask-fabro-runtime.test.ts, /home/daytona/workspace/fabro/apps/fabro-web/app/lib/ask-fabro-runtime.ts, /home/daytona/workspace/fabro/apps/fabro-web/app/routes/ask-fabro.tsx, /home/daytona/workspace/fabro/apps/fabro-web/app/routes/run-detail.tsx, /home/daytona/workspace/fabro/lib/crates/fabro-server/Cargo.toml, /home/daytona/workspace/fabro/lib/crates/fabro-server/src/server.rs, /home/daytona/workspace/fabro/lib/crates/fabro-server/src/server/handler/sessions.rs, /home/daytona/workspace/fabro/lib/crates/fabro-workflow/src/handler/llm/api.rs
- **simplify_opus**: succeeded
- Model: claude-opus-4-7, 108.3k tokens in / 35.8k out
- Files: /home/daytona/workspace/fabro/apps/fabro-web/app/lib/ask-fabro-runtime.ts, /home/daytona/workspace/fabro/apps/fabro-web/app/routes/run-detail.tsx, /home/daytona/workspace/fabro/lib/crates/fabro-server/src/server.rs, /home/daytona/workspace/fabro/lib/crates/fabro-server/src/server/handler/sessions.rs, /home/daytona/workspace/fabro/lib/crates/fabro-workflow/src/handler/llm/api.rs
# Simplify: Code Review and Cleanup
Review changes vs. origin for reuse, quality, and efficiency. Fix any issues found.
## Phase 1: Identify Changes
Run git diff (or git diff HEAD if there are staged changes) to see what changed. If there are no git changes, review the most recently modified files that the user mentioned or that you edited earlier in this conversation.
## Phase 2: Launch Three Review Agents in Parallel
Use the Agent tool to launch all three agents concurrently in a single message. Pass each agent the full diff so it has the complete context.
### Agent 1: Code Reuse Review
For each change:
1. Search for existing utilities and helpers that could replace newly written code. Use Grep to find similar patterns elsewhere in the codebase — common locations are utility directories, shared modules, and files adjacent to the changed ones.
2. Flag any new function that duplicates existing functionality. Suggest the existing function to use instead.
3. Flag any inline logic that could use an existing utility — hand-rolled string manipulation, manual path handling, custom environment checks, ad-hoc type guards, and similar patterns are common candidates.
Note: This is a greenfield app, so focus on maximizing simplicity and don't worry about changing things to achieve it.
### Agent 2: Code Quality Review
Review the same changes for hacky patterns:
1. Redundant state: state that duplicates existing state, cached values that could be derived, observers/effects that could be direct calls
2. Parameter sprawl: adding new parameters to a function instead of generalizing or restructuring existing ones
3. Copy-paste with slight variation: near-duplicate code blocks that should be unified with a shared abstraction
4. Leaky abstractions: exposing internal details that should be encapsulated, or breaking existing abstraction boundaries
5. Stringly-typed code: using raw strings where constants, enums (string unions), or branded types already exist in the codebase
Note: This is a greenfield app, so be aggressive in optimizing quality.
### Agent 3: Efficiency Review
Review the same changes for efficiency:
1. Unnecessary work: redundant computations, repeated file reads, duplicate network/API calls, N+1 patterns
2. Missed concurrency: independent operations run sequentially when they could run in parallel
3. Hot-path bloat: new blocking work added to startup or per-request/per-render hot paths
4. Unnecessary existence checks: pre-checking file/resource existence before operating (TOCTOU anti-pattern) — operate directly and handle the error
5. Memory: unbounded data structures, missing cleanup, event listener leaks
6. Overly broad operations: reading entire files when only a portion is needed, loading all items when filtering for one
## Phase 3: Fix Issues
Wait for all three agents to complete. Aggregate their findings and fix each issue directly. If a finding is a false positive or not worth addressing, note it and move on — do not argue with the finding, just skip it.
When done, briefly summarize what was fixed (or confirm the code was already clean).

View file

@ -0,0 +1,5 @@
{
"mode": "agent",
"provider": "openai",
"model": "gpt-5.5"
}

View file

@ -0,0 +1,20 @@
Reviewed the Ask Fabro sidebar/run-tools changes and made cleanup fixes.
What I fixed:
- Scoped Ask Fabros run-tool backend to the owning run via `ClientBackend::with_run_scope(...)`, so accidental cross-run tool calls are rejected before reaching the API.
- Switched Ask Fabro session tokens to base same-run worker tokens instead of broad `agent:run_tools` tokens.
- Exposed the base `issue_worker_token` helper outside tests for this same-run token path.
- Added stale cached-session cleanup in the web adapter: if a persisted session ID returns 404 during streaming, its cleared so the next turn creates a fresh session.
Validation run:
- `cargo check -q -p fabro-tool -p fabro-server`
- `cargo +nightly-2026-04-14 fmt --all`
- `cargo +nightly-2026-04-14 clippy -q -p fabro-tool -p fabro-server --all-targets -- -D warnings`
- `cd apps/fabro-web && bun test app/lib/ask-fabro-runtime.test.ts`
- `cd apps/fabro-web && bun run typecheck`
Current modified files:
- `apps/fabro-web/app/lib/ask-fabro-runtime.ts`
- `lib/crates/fabro-server/src/server/handler/sessions.rs`
- `lib/crates/fabro-server/src/worker_token.rs`
- `lib/crates/fabro-tool/src/fabro_client.rs`