Commit graph

28 commits

Author SHA1 Message Date
Bryan Helmkamp
467087998d
Read workflow graphs through Petri's DOT parser
Fabro's own DOT parser was left with one job after create-time compile
moved to Petri: walking a workflow's file references for the bundler and
the workflow-version store, and reading a name, a goal and two counts.
Petri's frontend parses the same language, so the parser goes and a small
crate reads the graph through Petri's.

`fabro-dot` is that crate: `WorkflowGraph::parse` over
`petri_frontend_attractor::dot` and its semantic model (defaults applied,
subgraphs flattened, chains expanded), `references(position)` as the one
walker over the static-reference vocabulary (each reference with its node
and position, file references checked to be template-free), and
`normalize_for_graphviz`, the re-emit of Fabro DOT with dotted attribute
keys quoted, which the SVG render needs. It sits beside `fabro-petri`
rather than inside it because `fabro-petri` depends on `fabro-workflow`,
which depends on `fabro-workflow-version`: the version store cannot reach
`fabro-petri` without a cycle, and the bundler should not pull the engine
in to read a graph.

Deleted: `fabro-graphviz`'s lexer, grammar, AST, semantic pass and
`parse_ast` (1,829 lines, plus the `nom` dependency); the DOT model in
`fabro-types::graph` (`Graph`, `Node`, `Edge`, `AttrValue`,
`shape_to_handler_type`), with only `ReferenceKind` kept, moved to
`fabro_types::reference`; `fabro-template`'s `visit_graph_references` and
the `GraphReference`/`GraphPosition` types, with the template-syntax rule
(`validate_static_reference`) kept there; the pull-request body's DOT
fallback summary, which was unreachable because the DOT source only
travels with the run spec whose display graph the summary already reads.
`fabro-graphviz` is now the render alone, over `fabro-dot`.

Parity: the old and new walkers were run over every `.fabro` and `.dot`
file in the repository (118) before the deletion. Every reference set is
identical. Five files differ in what Petri reads more correctly: a
backslash before a newline inside a quoted string is a line continuation
(four files, inline prompt text only), and a node named only by an edge
counts as a node (`test/edge_only_node.fabro`, 3 nodes rather than 2, so
the `fabro validate` snapshot moves). The checked-in bundles' shapes and
references are pinned by a snapshot in `fabro-dot`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-19 12:23:57 -04:00
Bryan Helmkamp
bd59f52e22
Build the run's display graph from Petri's admission
The rest of the change whose deletions the previous commit carries (its
`git add` stopped at an already-removed path): `fabro_types::RunGraph`
and the `fabro-petri` builder that reads it off the admitted graph, the
server's create, validate, preflight and render paths on Petri's check
alone, the consumers moved to the new shape, the OpenAPI `RunGraph`
schemas with their parity tests, the regenerated TS client, and the
docs naming Petri's diagnostic codes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-19 11:41:58 -04:00
Bryan Helmkamp
efb43b45aa
Delete the executor-era error and Git helpers in fabro-workflow
The failure classifiers, the handler and publish error builders, the
FailureDetail projections, and the LLM error conversions served the
deleted executor; Petri classifies failures now. Error keeps the
variants the create and read side construct, and the Engine variant
replaces the three-stage Stage shape. git_identity goes: the hooks
record git.identity through fabro_checkpoint. git.rs keeps the
observe, head, non-interactive push, and sync helpers the server and
fabro-manifest call, and loses the push half. fabro-llm loses the
failure signature hint whose only reader was the deleted classifier.
fabro-workflow drops regex, strum, and fabro-checkpoint.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-19 09:10:34 -04:00
Bryan Helmkamp
60b503322c
Delete the legacy run event log, its reducer and its types
Step 4 of the legacy executor deletion, fourth commit: with no writer
and no reader left, the legacy event log goes.

- `fabro-types`: `run_event` (`EventBody`, `RunEvent` and every props
  struct), `EventEnvelope` and the `RunEventDetail*` types are deleted.
  What the projection and the API still use moves out of the event
  vocabulary: `AgentEventProps`, `AgentSessionActivatedProps`,
  `AgentToolsAvailableProps`, `StagePromptProps`, `SessionCapability`
  and the coding event names to `agent_props`; `RunNoticeLevel` and
  `RunNoticeCode` to `notice`; `InterviewOption` beside the question
  types; `RunRunnableSource` beside the run status. `Checkpoint` is
  what Fabro records for a Petri run: `timestamp`, `current_node`,
  `git_commit_sha`; the conclusion's stage summaries derive from the
  projection's stages instead of the checkpoint's node maps.
- `fabro-store`: the Slate bridge (`RunDatabase`, the Slate `Database`,
  `keys`, `record`, `EventPayload`) and the reducer (`run_state`) are
  deleted. `Database` is the blob table and the run summary store over
  one pool; the blob store is SQLite only; the run summary store keeps
  the `runs` row a projector writes and lists, and finds the pull
  request creation candidates over `platform_records`; `build_summary`
  and `projected_usage` live in `run_summary`. The SlateDB dependency
  is gone. Test fixtures build the store from its two SQLite stores.
- `fabro-workflow`: the `event` module (the `Event` enum, its
  conversion, sink, emitter, redaction, stored fields and names),
  `runtime_store`, `StageScope` and the legacy seeding test helpers are
  deleted; the tests that seeded legacy runs read platform records or
  a projection instead.
- `fabro-sandbox` owns `GitRetryReason`.
- The server builds the store without an object store; the legacy
  `POST /runs/{id}/events` tests go, an interrupt answers
  `interrupt_unsupported` in the tests as it does in the handler, and
  the tests that read a run back through the Slate handle read its
  projection or its platform records. The projection folds a block
  that lands while the run is paused as the pause's prior block, and a
  pause or unpause clears the pending control it answers; a control
  request's check-and-append holds a per-run lock so two concurrent
  cancels record one request.
- The CLI's final output is the response of the last stage that
  produced one; the workflow tests read completed nodes from the
  succeeded stages.
- The spec's `RunCheckpoint` carries the three fields the type keeps.

Still failing until the next commits: the CLI tests that seed runs
through `POST /runs/{id}/events` or wait for legacy event names, and
the two Ask Fabro resume tests (the sandbox instance gap).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-18 14:09:17 -04:00
Bryan Helmkamp
af38d68942
Delete fabro-validate and fabro-acp; validate on Petri's check
Petri judges a workflow at admission, so Fabro's lint rules go:
`fabro-validate` (its 35 rules and the `LintRule` trait) is deleted, and
with it `fabro-acp` (only a rule and two legacy executor tests used it),
the model-resolution transform, the legacy `create`, `compile_create_run`
and `materialize_create_run` stages, and `fabro-graphviz`'s `condition`
and `fidelity` modules. `Diagnostic`, `RelatedDiagnostic` and `Severity`
move to `fabro_types::diagnostic`, the one shape every diagnostic takes.

Validation is now the same question the create handler asks. A new
server module, `petri_check`, builds Petri's check request from a
workflow bundle and the run's settings (every workflow of the bundle at
its bundle-relative path, the inputs, the run variables, the launch),
runs the check, and maps the diagnostics; Fabro adds one rule of its
own, `fabro.model.no_ready_provider`, refusing a model node when no
provider is ready. Admission, the validate and preflight endpoints and
the offline `fabro validate` all go through it:

- `validate_prepared_manifest` runs Fabro's structural pass (parse and
  transform, whose diagnostics stay) and then Petri's check, on the
  blocking pool from the handlers;
- the offline `validate_manifest` checks with no model client and with
  an unbound input as a warning (`CheckRequest.unbound_is_warning`), so
  a workflow validates before its inputs exist; a collected workflow
  before upload checks with unbound inputs as errors, as before;
- preflight resolves each LLM node's selector against the ready
  providers and the catalog for its probe, as the deleted transform did,
  and no longer probes a model Petri refused;
- the graph render endpoint needs only the structural pass;
- a run manifest now carries its `[run.goal] file`, which Petri reads
  from the bundle as it does for a version.

The transforms keep the authored model selector (`sonnet` stays
`sonnet`): Petri pins the catalog model in the admitted graph, not in
the graph Fabro displays or in the settings snapshot. Tests assert that,
and the CLI's validate, preflight and graph snapshots carry Petri's
diagnostics (`attractor.no_start`, `attractor.undeclared_node`,
`attractor.bad_on_failure`, ...) in place of the lint rules' text.

Known gaps, Petri's side: a `workflow.toml` whose `[run.environment]`
names an environment the server catalog defines but the file does not
is refused (`unsupported.workflow_toml.run.environment`), as admission
already refused it; an unbound input inside an included template
partial is a render error rather than the unbound-input warning.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-18 11:32:04 -04:00
Bryan Helmkamp
d90a5d9cbb
Delete fabro-core and the engine half of fabro-workflow
Every run executes on Petri, so the in-process legacy executor goes:
`fabro-core` and, in `fabro-workflow`, the handlers, lifecycle, pipeline
execution, routing, retry, conditions, node handlers, steering, agent
memory, artifacts, checkpoints, command log, and the `start`, `resume`,
`retry`, `fork`, `rewind` and `timeline` operations. The two are deleted
together because the engine half of `fabro-workflow` was the only user of
`fabro-core` and `fabro-core` the only runtime of that half; neither
compiles without the other.

Kept in `fabro-workflow`, narrowed: the parse/transform/validate/persist
pipeline and `create`, `archive`, `validate` (workflow definitions still
come from DOT and settings); the run tools (`run_tools`, moved from
`handler/llm/fabro_tools.rs`) for Ask Fabro, `fabro exec` and Petri's
host tools; the pull request pipeline (`pull_request`, moved from
`pipeline/`, for the step 0 port); Run Files' diff helpers in
`sandbox_git`; `git_identity`, `usage_rollup`, `run_status`,
`run_materialization`, `web_search` and `workflow_bundle`.

Server: `RegistryFactoryOverride` becomes `execute_in_process`;
`RunAnswerTransport::InProcess` carries only the interviewer; the
interrupt endpoint answers 501 `interrupt_unsupported` and every pair
endpoint 501 `pair_unsupported` (status lists none); rewind, fork, retry
and timeline handlers and routes are removed; the command log is served
from the stage output blob; usage rollups accumulate from the settled
projection after an in-process run as after a worker exit.

Ported while here:

- `materialize_admitted_run` materializes the goal and drops a disabled
  pull request block, as the legacy materializer did.
- A run whose admitted graph has an agent or prompt node is refused at
  create when no LLM provider is ready (`fabro.model.no_ready_provider`);
  a workflow of commands and gates needs no model and is admitted.
- The projection's question type falls back on the options, as the
  interview adapter does, so a gate with edge-label options answers as
  multiple choice.

Tests: the server scenarios (lifecycle, run completion, SSE, helpers)
run in process on Petri and assert Petri's stage labels and stream
names; the reconcile tests assert Petri's relaunch semantics; legacy
unit tests of the deleted executor are removed; three server unit tests
the removal took with it are restored; the pair fixtures go with the
pair feature. Petri test fixtures no longer name `[workflow] engine`.

Still red after this commit, all legacy consumers the next steps
delete or port: fabro-store's Slate/reducer fixtures and fabro-types
legacy JSON tests (step 4); server unit tests over legacy run events
(retry endpoints, list_run_events, artifacts, per-event pause/unpause,
run history activation, legacy sandbox fixtures) (steps 3-4); CLI tests
that parse legacy event envelopes, the legacy `events`/`attach`/`diff`/
`dump`/`inspect` snapshots, `run rewind`/`run fork`, the ACP and
git-identity workflow tests, and the runner tests that drive the legacy
worker by hand (steps 3-4); the web app's Petri fixtures still carry
`engine` (regenerate with `FABRO_CAPTURE_PETRI_FIXTURES` in step 4).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-18 10:44:41 -04:00
Bryan Helmkamp
ef86ad278a
Carry usage as lithos-llm's Usage and rename billing to usage
Re-pin lithos-llm to 55add4596b861a0623d00c3a54aa5c147c8d504b and
pebble to c91810fe51aece80359b9cd8efea971af0c46925, where token usage
and cost travel together as Usage { tokens: TokenCounts, cost:
Option<Cost> }. Fabro now carries that one type everywhere it used to
carry BilledTokenCounts, BilledModelUsage, UsdMicros, or a token count
beside a cost_usd_micros.

fabro-types: billing.rs is usage.rs with ModelRef, ModelUsage { model,
usage }, sum_usage, and usage_is_empty; billing_rollup.rs is
usage_rollup.rs with ProjectionUsageStage, ProjectionUsageByModel,
ProjectionUsageRollup, and usage_rollup_from_projection. Every usage
field is named usage: StageProjection.usage and usage_by_model,
Outcome<Option<ModelUsage>>, stage.completed and stage.failed usage and
usage_by_model, prompt.completed usage, run.completed and run.failed
usage (total_usd_micros is gone), Conclusion.usage, StageSummary.usage,
Run.usage. RunSize buckets by Cost.

fabro-workflow: model_usage_from_llm prices tokens from the catalog with
a Catalog cost source, with_reported_cost keeps a provider cost, and the
pebble handler's stage_usage groups pebble's accounts by model and sums
rows with Usage::saturating_add, so a total has a cost only when every
priced part was priced. The store fold's live usage is the agent's
usage plus its descendants'.

API: the OpenAPI spec deletes BilledTokenCounts, BilledModelUsage,
CompletionUsage, CompletionCost, TokenUsage, and RunBillingSummary,
adds TokenCounts, Cost, Usage, and ModelUsage, and renames every
billing schema, property, tag, path, and operation to usage. fabro-api
reuses lithos-llm's and fabro-types' types through with_replacement,
with a round-trip test per replacement.

Old stored runs get no migration: their pebble events in the old shape
read back with zero usage, and their rebuilt projections lose agent
usage.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-14 12:31:34 -06:00
Bryan Helmkamp
0c0e589a78 Bill a failed agent stage what it spent
An agent stage that failed billed nothing: the backend returned a bare
error and the outcome built from it carried no usage. A terminal failure
now becomes the stage's failed outcome from the same fold that bills a
completed stage, with the tree's usage, the rows by model, the files it
wrote, and its active time; stage.failed carries billing and
billing_by_model and the store keeps both. Cancellation and retryable
failures still go up as the error.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-13 08:35:47 -06:00
Bryan Helmkamp
e5d5c534ab
Merge remote-tracking branch 'origin/main' into remove/run-metadata-branches
Resolve conflicts between the metadata-branch removal and the
sandbox-driver adoption on main:

- fabro-sandbox docker.rs, sandbox.rs, daytona/mod.rs: take main's driver
  rewrite. The Sandbox trait is gone, so the PR's push_token_source
  removal now applies to RunSandbox instead; drop that accessor and the
  RepoCredentials::source helper that only served it.
- run_metadata.rs: keep deleted. Main's edits there were adaptations to
  the driver API and the run git identity field.
- lifecycle/git.rs, finalize.rs: keep the PR's removal of metadata
  snapshots and write_finalize_commit; carry main's RunSandbox,
  GitRetryPolicy, git_identity, local_sandbox, and test catalog changes.
- sandbox_git.rs: take main's version and drop the shadow_sha parameter
  and Fabro-Checkpoint trailer.
- git_integration.rs: remove meta_branch from the new git identity test.
- Cargo.toml: main's dependency set with fabro-dump kept as a
  dev-dependency.
- checkpoints.mdx: keep both the git identity paragraph and the durable
  execution state section.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-12 16:28:01 -06:00
Bryan Helmkamp
18a3c4741e
Run agent stages, Ask Fabro, and fabro exec on pebble's CodingAgent
Replace fabro's hand-written agent loop with pebble's `CodingAgent` and
delete the `fabro-agent` crate.

Workflow: `PebbleBackend` builds one agent per stage over `RunSandbox`,
binds the stage's hooks as tool middleware, the interviewer as the
human-input provider, and a durable `EventSink` that writes every agent
event through the run event log before the agent goes on. Full-fidelity
threads continue across stages through `export`/`resume_from_export`.
Model failover takes the session record after the failed prompt and
continues it on the next route with `ResumeMode::UseModel`, so no tool
effect repeats. The steering hub targets pebble's control handle, with
a steering lease holding completion open while a human is paired.

Events: `EventBody::Agent` carries pebble's `CodingAgentEvent` envelope;
the per-variant bodies, the transcript projection, and the fabro-only
context-window, tool-summary, and skill types are gone in favor of
pebble's. The OpenAPI schemas, generated Rust and TypeScript clients,
and web readers follow.

Ask Fabro: the session runs a `CodingAgent` under a read-only permission
policy and a system prompt transform. Its conversation lives in a new
`run_session_records` table and resumes on the recorded model with the
event cursor advanced past the run log.

`fabro exec` builds the same agent over a local sandbox with pebble's
permission middleware and an interactive approval service.

The catalog fills in `metadata.agent.profile` for operator providers
that declare none, so pebble's lookup is the one resolution path.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-11 14:19:15 -06:00
Bryan Helmkamp
fa3e485c95
Name built-in providers through lithos catalog::builtin
lithos-llm now ships the built-in provider ids and constructors, so
fabro-types drops its provider_ids module and every caller uses
lithos_llm::catalog::builtin directly. The crates that name a provider
now depend on lithos-llm themselves.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-10 09:48:37 -06:00
Bryan Helmkamp
87e1e3a00f
Replace fabro-llm helper modules with lithos-llm equivalents
Delete fabro-llm's attachments, reasoning, and structured modules and
the LlmError newtype and ErrorFacts trait. lithos-llm now provides all
of them: InlineLocalFiles under the local-files feature, ReasoningOutput
with Response::reasoning(), Client::complete_object, and the retry,
auth, cancel, and failover predicates directly on Error and ErrorData.
fabro-llm keeps only failure_signature_hint, which is Fabro's own loop
detection policy.

Store ErrorData directly in the agent and workflow error enums, boxed
where the variant would otherwise dominate the enum size. Repin
lithos-llm to a1e3fd3 for these additions.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-10 09:41:20 -06:00
Bryan Helmkamp
82bcafcfca
Port fabro-workflow, hooks, validate, and store to the lithos types
Workflow LLM handlers build lithos requests, bill from lithos usage and
cost, and classify failures from lithos `ErrorKind`. Model resolution and
fallback use the fabro-llm selection and catalog helpers. Validation
rules read the lithos catalog, and store fixtures use the new
`BilledModelUsage` shape.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-09 17:26:57 -06:00
Bryan Helmkamp
ec9ea5c7be
Remove Git run metadata branches 2026-09-05 12:31:03 -04:00
Bryan Helmkamp
61394ba2f1
Simplify run-event persistence failure plumbing
- Make the failure watch channel the single record of the latched
  failure; drop the worker task's mirrored local state.
- Replace the hand-rolled wait loop with watch::Receiver::wait_for.
- Extract race_persistence/flush_or_stop helpers so the select!/flush
  scaffolding in RunSession::run exists once instead of three times.
- Return RunEventPersistenceError from append_event_to_sink and add a
  From impl on Error, replacing four hand-written per-event message
  strings with the event name derived from the event itself.
- Dedupe the RunCreated test seed literal in initialize.rs and drop the
  dead BlockingHandler::simulate override.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ryyhtbc1eNtCLw8GjrFQXZ
2026-08-21 19:29:14 -04:00
Bryan Helmkamp
7d771e9b96
Merge remote-tracking branch 'origin/main' into pr-764
# Conflicts:
#	lib/components/fabro-sandbox/src/daytona/mod.rs
#	lib/components/fabro-sandbox/src/push_credentials.rs
#	lib/components/fabro-sandbox/src/sandbox.rs
2026-08-20 22:07:20 -04:00
Bryan Helmkamp
78cb0d1348
Clean up git push retry handling 2026-08-20 21:59:47 -04:00
Bryan Helmkamp
f88df59163
Classify sandbox state-change rejections as transient infra
A Daytona "Sandbox state change in progress" rejection surfacing
through the pipeline lifecycle path ("Pipeline lifecycle operation
failed") matched no transient-infra hint, so the run failure was
categorized deterministic. The condition is a provider lifecycle
transition that finishes on its own — the definition of transient
infrastructure — and the deterministic label misinforms retry
machinery and anyone reading the failure.

Add two transient-infra hints: the provider rejection ("state change
in progress") and the bounded-wait timeout an activation reports when
a stop transition outlives its budget ("sandbox stop still in
progress").

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-20 17:39:19 -04:00
Bryan Helmkamp
1688cd5b91
Retry git pushes with a pinned token and record attempt history
Run 01M0DH033P2XSTHAGVBHG6922F completed 2.8 hours of work, then failed
terminally because four consecutive publish pushes hit GitHub's
token-replication lag (404 "Repository not found") — the push path had no
retry, the failure was misclassified as deterministic, and the same
fresh-mint-then-push pattern silently disabled metadata snapshots. This
generalizes the clone retry machinery to pushes and makes attempt detail
durable.

- clone_retry -> git_retry: the classifier's boolean becomes a
  CredentialContext derived from the token snapshot (fresh App tokens retry
  404s as replication lag, mature ones as transient infra, static
  credentials fail fast), and the attempt/backoff limits become a RetryPlan
  with layered optional bounds. Clone behavior is preserved: Docker keeps
  its absolute five-minute deadline, Daytona keeps no deadline.
- Pushes take a scoped CredentialLease before the first attempt: it owns
  the embed mutex for the whole operation, pins the single successful
  resolve, retries only failed resolves, falls back to the last embedded
  token when a mint fails, and force-re-embeds the pinned token once after
  the first auth-shaped failure (drift repair). The margin invariant
  (REFRESH_MARGIN > every push plan's max_elapsed) guarantees the pinned
  token outlives the operation; a unit test asserts it.
- Sandbox::git_push_ref now takes a RetryPlan and returns PushReport /
  PushError with per-attempt records (classification, redacted output tail,
  token generation/provenance/age, credential action, refresh errors).
  Checkpoint pushes use a 90-second budget; the terminal publish push gets
  5 attempts over at most 4 minutes.
- The single durable git.push event per push gains a nested attempts array
  (GitPushAttemptProps, token snapshot flattened to flat fields); stored
  events without it still deserialize. Publish push failures now carry an
  explicit failure category — exhausted transient retries stay
  transient_infra instead of deterministic — plus one bounded cause line
  per attempt and the last successful push time in the message.
- Metadata snapshot degradation records why it degraded: push failures with
  retryable classifications leave the writer eligible to re-probe at each
  later checkpoint, and a successful snapshot clears the degraded state and
  re-arms the warning. Permanent failures keep today's latch.

Plan: .ai/plans/git-push-token-resilience.md (PR 2: items 1, 2, 4, 7 and
the metadata re-probe).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-20 10:16:10 -04:00
Release Repro
81d762aa9d
Merge remote-tracking branch 'origin/main' into feat/script-value-interpolation
# Conflicts:
#	lib/components/fabro-workflow/src/pipeline/transform.rs
2026-07-28 17:27:24 -04:00
Release Repro
b04684aec4
fix(workflow): harden script value interpolation 2026-07-28 17:21:35 -04:00
Bryan Helmkamp
fa6f7e5558
fix(workflow): verify remote head before generating PR content, tolerate replica lag
Three follow-ups from the efficiency review of the publish pipeline.

Check the branch before spending an LLM call:
`open_pull_request` generated the PR title and body first and only then
verified the remote branch pointed at the run's final commit. Every stale
branch therefore cost a full content generation before failing. The
verification is the cheap check, so it now runs first.

Tolerate GitHub read-after-write lag:
`GET /repos/{owner}/{repo}/branches/{branch}` is replica-served and can briefly
report the previous commit, or 404 for a branch that is new on the remote,
right after the push publish just made. It was read once with no retry. Since
publish failures are terminal, a replica that had not caught up yet would
discard a fully successful run. It is now read up to three times.

These two land together on purpose: the LLM call was the only thing buying
slack against the race, so reordering without the retry would have made it
more likely.

Keep commit SHAs out of failure classification:
`classify_failure_reason` substring-matches bare "500", "502", "503" and "504"
as transient-infra hints. Both publish messages embed a commit SHA, and a
40-char hex string contains one of those often enough to matter, so a
deterministic failure could be reported as transient. Long hex runs are now
masked before matching; the three-digit status codes those hints look for are
too short to be affected. The hex regex is shared with
`normalize_failure_reason`, which already had its own copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 15:34:18 -04:00
Bryan Helmkamp
73f48eeddb
refactor: simplify publish pipeline and collapse duplicated stage errors
Follow-up cleanup on the publish-failures change.

Error model:
- Collapse `Error::{Engine, Publish, Handler}` into one `Error::Stage` with an
  `ErrorStage` discriminator. The three shared a field shape and had to be
  edited together in four match groups; nine near-identical constructors
  become two private helpers.
- Add `Error::failure_reason()`, replacing the same error -> FailureReason
  mapping written out in four places.
- Publish errors are now terminal. Publish runs once, after execution, so no
  caller could ever act on the retryable classification.

Publish phase:
- Fix: a branch that was pushed is now still reported when pull request
  creation fails afterwards. `PublishOutcome` records what happened and
  carries the error separately, instead of hiding both behind a `Result`.
- Drop `PublishOutcome::NoChanges`, which no consumer distinguished from
  `Published { pr_url: None }`.
- Move publish onto `Concluded` as methods and replace three near-identical
  precondition guards with one `publish_target()`.

Pull requests:
- `maybe_open_pull_request` -> `open_pull_request` returning the record
  directly. Both callers already reject empty diffs, so the `Ok(None)` path
  was unreachable.
- Drop `CreatedPullRequest.head_sha`, which echoed back its own input.

GitHub client:
- Delete `branch_exists`, which had no callers and duplicated
  `branch_head_sha`. Give `branch_head_sha` the `_with_client` split every
  sibling has and port the tests to `MockHttpClient`.
- Collapse the copy-pasted credential match in `resolve_clone_credentials`.

Events:
- `PullRequestCreated.head_sha` is `Option<String>` instead of using an empty
  string to mean absent.
- Centralize the run-branch refspec in `lifecycle::git::push_run_branch`, so
  `git.push` reports a branch name from both emitters as documented.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 15:09:10 -04:00
Bryan Helmkamp
1c82bd9008
fix(workflow): make publish failures terminal 2026-07-27 11:25:18 -04:00
Release Repro
59d5b317dc
Simplify LLM error mapping and validate reasoning_effort parsing
- Return InvalidRequest (400) for unsupported speed too, matching the
  reasoning_effort check and the complete()/stream() doc comments
- Centralize fabro_llm::Error -> ApiError mapping in a From impl so the
  completions handler, playground handler, and Error::Llm arm agree on
  the InvalidRequest -> 400 / else -> 502 split
- Reject unparseable reasoning_effort values with 400 instead of
  silently dropping them
- Add classify_sdk_invalid_request test per fabro-workflow convention

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 08:29:12 -04:00
Bryan Helmkamp
5aebb17fa2
Return bad request for unsupported reasoning effort 2026-07-24 07:22:47 -04:00
Bryan Helmkamp
cdf1dce4f8
fix(agent): remove dead execution limits 2026-07-23 19:36:29 -04:00
Scott Werner
47bc772f7b refactor: organize crates into three layers 2026-07-23 17:59:34 -04:00
Renamed from lib/crates/fabro-workflow/src/error.rs (Browse further)