Commit graph

17 commits

Author SHA1 Message Date
Bryan Helmkamp
53a92f75f4
test(llm): model streamed usage in OpenAI twin 2026-07-24 17:47:05 -04:00
Bryan Helmkamp
7c771fc5d7
test(twin-openai): accept tool call histories 2026-07-23 20:11:21 -04:00
Bryan Helmkamp
5d345cf393
Fix OpenAI-compatible parity fixture 2026-07-22 16:37:32 -04:00
Scott Werner
e4a85679bf
fix(test): make twin-openai streamed message items round-trip as input (#484)
## What

Fixes the `openai_twin_*` parity-matrix failures that have been on
`main` since #449: every multi-turn scenario whose scripted response
includes text fails on its second turn with 400 `"message input items
require supported content"`.

## Root cause

Two twin behaviors collided (bisected: passes at #447, fails at #449):

1. **The twin's streaming `response.output_item.done` for message items
omitted the `content` array** (`test/twin/openai/src/sse.rs`) — it sent
only `id`/`type`/`status`/`role`, where the real API sends the completed
item in full. The openai adapter preserves message output items verbatim
(`ContentPart::Other { kind: OPENAI_MESSAGE }`) and replays them as
assistant history on the next turn — required so reasoning items keep
their "required following item" in Responses round-trips. So the replay
arrived content-less.
2. **#449 tightened the twin's input validation** to also validate
explicit `type: "message"` items (previously only type-less items were
validated as messages; anything with an explicit type was accepted
unchecked). The twin started rejecting its own round-tripped output.

The new validation caught a real infidelity in the emitter — the emit
side is what's wrong.

Nobody noticed because **CI never runs the twin e2e suites**: `rust.yml`
runs `--profile ci` without `--run-ignored`, so the parity matrix only
runs when someone invokes the e2e profile locally.

## Fix

- The streamed message `output_item.done` now carries its `output_text`
content, matching the real API and the twin's own non-streaming
`responses_json()`.
- The input validator accepts `output_text` parts on **assistant**
message items (the real API allows these; the twin's non-streaming
responses already require it for faithful replay). Non-assistant
`output_text` parts get a dedicated rejection message.

## Tests

- New contract test
`responses_stream_message_item_done_round_trips_as_input`: streams a
response, asserts the completed message item carries its `output_text`
content, and replays the item verbatim as assistant-history input,
asserting the twin accepts its own output.
- `cargo nextest run -p twin-openai` — 56 passed
- `cargo nextest run -p fabro-agent -E 'test(parity)' --run-ignored
only` — **91/91 passed** (was 7 failing)
- `cargo nextest run -p fabro-llm --run-ignored only` — 10 passed
- `cargo nextest run --workspace` — green apart from two pre-existing
env-dependent `fabro-workflow` failures that reproduce on clean `main`
in shells with provider API keys exported (unrelated; CI is green on
them because it has no such keys)
- clippy `-D warnings` / fmt — clean

Found while reviewing #481 (whose parity runs surfaced this); #481
itself is unaffected — it doesn't touch the openai adapter or the twin,
and the failures exist on its merge-base.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

## CI (separate commit, drop if unwanted)

`ci: run twin-mode e2e suites on Linux` adds a step to the existing
Linux test job running the ignored twin-mode suites for the packages
that are fully green today (`fabro-agent`, `fabro-llm`, `twin-openai`) —
104 tests, ~1s on a warm build, no secrets needed (live-only tests
self-skip in twin mode). This is what would have caught the #449
regression. The remaining ignored suites (fabro-cli twin tests,
Docker/Daytona sandbox tests, fabro-spa asset test) need their own fixes
before joining; widen the `-E` filter as they're cleaned up. Note the
step deliberately avoids the `e2e` nextest profile, since
`NEXTEST_PROFILE=e2e` implies strict mode, which fails on missing
secrets.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-06-10 12:56:20 -04:00
Bryan Helmkamp
91d11eb04d
fix(llm): preserve raw compatible tool arguments (#448)
## Summary

Fixes #435.

Preserve raw non-JSON tool-call arguments for custom/freeform tools when
using the OpenAI-compatible Chat Completions adapter. This keeps
`apply_patch` receiving the raw patch text instead of `{}` when
LiteLLM/openai-compatible providers emit Codex-style freeform patch
calls.

Also extends the OpenAI twin so black-box tests can exercise the Chat
Completions path with raw tool-call arguments.

## Test Plan

- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo nextest run -p fabro-agent --test it
openai_compatible_twin_preserves_raw_apply_patch_arguments --run-ignored
only`
- `cargo nextest run -p fabro-llm`
- `cargo nextest run -p fabro-test`
2026-05-28 11:52:36 -04:00
Bryan Helmkamp
2e39dfc70e
fix(agent): align compaction preserve boundary (#449)
## Summary

Follow-up to fabro-sh/fabro#447. This keeps context compaction's
effective preserve boundary consistent between summary generation,
history mutation, and emitted telemetry so tool-call/result pairs that
remain in raw history are not also summarized.

The branch also tightens the OpenAI twin support added for this
regression: scripted usage is modeled as a single `TokenUsage`, SSE
completion payloads reuse the canonical Responses JSON shape, and
request validation now treats custom tool-call outputs as tool outputs
instead of spreading raw item-type string checks.

## Verification

- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo nextest run -p fabro-agent compaction`
- `cargo nextest run -p twin-openai`
- `FABRO_TEST_MODE=twin cargo nextest run -p fabro-agent --profile e2e
--run-ignored only --test it
openai_twin_compaction_preserves_tool_call_pairs`
- `git diff --check origin/main...HEAD`

---

[![Compound
Engineering](https://img.shields.io/badge/Compound_Engineering-6366f1)](https://github.com/EveryInc/compound-engineering-plugin)
🤖 Generated with GPT-5 via [Codex](https://openai.com/codex)
2026-05-28 11:26:52 -04:00
Bryan Helmkamp
81554581ca
fix(agent): preserve tool-call pairs during compaction (#447)
## Summary
Fixes OpenAI Responses requests after context compaction by ensuring
preserved tool results are not separated from the assistant tool calls
that produced them. The previous fixed-size preserved tail could retain
a `function_call_output` while dropping the matching `function_call`,
which OpenAI rejects as an orphaned tool result.

## Changes
- Extends `History::compact` so the preserved range moves backward until
every kept tool result has its matching assistant tool call.
- Adds a unit invariant test for compacted histories that serialize tool
results.
- Adds an OpenAI twin integration regression that forces compaction
during a tool-use loop.
- Teaches the OpenAI twin to validate orphaned `function_call_output`
items and script response usage counts for deterministic compaction
tests.

## Test Plan
- `cargo +nightly-2026-04-14 fmt --check --all`
- `git diff --check`
- `FABRO_TEST_MODE=twin cargo nextest run -p fabro-agent --test it
openai_twin_compaction_preserves_tool_call_pairs --run-ignored all`
- `cargo nextest run -p fabro-agent`
- `cargo nextest run -p twin-openai`
- `cargo nextest run -p fabro-test`
- `cargo +nightly-2026-04-14 clippy -p fabro-agent -p fabro-test -p
twin-openai --all-targets --no-deps -- -D warnings`

---

[![Compound
Engineering](https://img.shields.io/badge/Compound_Engineering-6366f1)](https://github.com/EveryInc/compound-engineering-plugin)
🤖 Generated with GPT-5 via [Codex](https://openai.com/codex)
2026-05-28 10:37:30 -04:00
Calen Pennington
596b105b06
fix(skills): Ensure the local server can access project skills (#383)
The local server wasn't setting git_root, and wasn't defaulting to the
workspace working directory, so it didn't find project-specific skills.

---------

Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
2026-05-24 13:22:00 -04:00
Bryan Helmkamp
80de5ca616 refactor(static): centralize env var names
Add fabro-static::EnvVars as the shared registry for fixed environment variable names and migrate env reads, clap env bindings, and subprocess/test allowlists to use it.

Add clippy bans for raw std::env lookup APIs so future dynamic env facades must be documented explicitly.
2026-04-24 12:29:51 -04:00
Bryan Helmkamp
e6e091fe8e
refactor(server): lock down server secrets 2026-04-23 07:15:27 -04:00
Bryan Helmkamp
ad0d532691
chore(clippy): require reasons on allow attributes
Enable clippy::allow_attributes_without_reason at the workspace level.
Add concise, callsite-specific reasons to existing allow attributes, including generated code paths.
2026-04-19 20:24:24 -04:00
Bryan Helmkamp
e828ce35a5 Disable debug UI auto-refresh for screenshot test 2026-04-12 15:06:04 -04:00
Bryan Helmkamp
6a87f0a071 fmt: apply nightly rustfmt after merge
Restore a clean nightly rustfmt baseline on the merged main branch so
cargo +nightly fmt --check --all passes again after bringing in
origin/main.
2026-04-11 13:43:30 -04:00
Bryan Helmkamp
007cfed240 refactor: remove backwards-compat error type aliases
No production deployments exist, so there's no need for migration shims.
Remove all six backwards-compat type aliases (AgentError, SdkError,
CoreError, GraphvizError, StoreError, FabroError) and migrate ~880
callsites to use the canonical Error name directly within each crate,
or qualified imports (e.g., `use fabro_llm::Error as LlmError`) for
cross-crate references. Also fix a pre-existing absolute-path clippy
lint in fabro-server error.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 12:51:42 -04:00
Bryan Helmkamp
5eeacd7864 fmt 2026-04-11 11:27:46 -04:00
Bryan Helmkamp
ddc57d458c Rename SessionConfig to SessionOptions and McpServerConfig to McpServerSettings
Aligns naming with the convention that "Config" is for file-level configuration
while "Options" and "Settings" describe runtime parameters. Also applies
rustfmt formatting fixes in web_auth.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-02 06:47:42 -07:00
Bryan Helmkamp
5b7eabee8f Add twin test mode for OpenAI E2E tests
Integrate twin-openai (fake OpenAI server) into the workspace and wire
it into the e2e_test macro so OpenAI tests can run without real API
credentials. The twin server starts in-process via OnceLock on first use
and provides per-test isolation through bearer-token namespacing.

Changes:
- Add Twin as default TestMode, replacing Off (gating now via #[ignore])
- Extend #[e2e_test] macro with `twin` requirement for twin-only,
  live-only, and dual-mode (twin + live) test gating
- Add e2e_openai!() macro returning (base_url, api_key)
- Convert openai_complete and openai_gpt_5_3_codex_complete to dual-mode
- Add new openai_server_error twin-only test with scripted 500 error
- Standardize axum 0.8 as workspace dependency across all crates
- Relax twin-openai ResponsesRequest to accept unknown fields via flatten

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-31 20:36:48 -04:00