Share canonicalize_location and resolve_existing_workflow_location
between the local package resolver and the version collector, drop the
redundant package-root pre-check and the PackageFileReadError enum in
favor of anyhow context, and read HEAD's SHA from git2 instead of a
separate rev-parse subprocess.
Make GitRunTargetObservation a plain struct, replace the repo-info
tuple with a named struct, tighten the closure view trait, avoid
deep-cloning the root workflow during collected validation, remove the
unused into_closure accessor, and dedupe test helpers.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`DaytonaSnapshotSettings` carried two independent `Option`s (`image` and
`dockerfile`) that every consumer had to re-validate. Replace them with a
single `source: DaytonaSnapshotSource { Image, Dockerfile }` so the
both-set and neither-set states are unrepresentable at the sandbox layer.
This removes four unreachable error arms in `canonical_manifest` and
`create_snapshot_params`, the `.filter(...)` guard in `initialize`, and
the presence guard in `daytona_config_from_environment`. The
mutual-exclusion rule now lives only in fabro-config, which owns the
`image.docker` / `image.dockerfile` keys the old messages named.
Merge `ImageSnapshotManifest` into `SnapshotManifest` via a flattened
`SourceManifest` enum. The dockerfile case serializes to the same bytes
as before, so existing snapshot names are unchanged; the pinned identity
test still passes. Pin the image-case identity as well so a future
manifest change cannot silently orphan image snapshots.
Fold `validate_daytona_image_settings` into the existing Daytona arm of
`validate_provider_capabilities`; both callers already invoke it right
after `resolve_environment_fields`, so error order is unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Introduce a PinnedRevision enum in clone_source so the Docker and Daytona
providers run one fetch/checkout/verify sequence for both an exact commit
and a tag instead of two near-identical arms. Fold the tag-specific
command builders into the generic ones, share the bare-ref grammar check
between branch and tag validation, and derive the workflow clone source
from the validated Git target in a single match.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed node with an effective `succeed` policy and no explicit recovery
route now finishes as `succeeded` and follows normal success routing. The
original failure stays on the outcome so the stage.completed event and the
checkpoint keep the diagnostic, and the outcome notes record which scope
promoted it.
- OnFailure gains a Succeed variant; Node::on_failure resolves the
deprecated auto_status=true attribute as an alias, with an explicit
on_failure winning
- The core executor applies the policy before the lifecycle observes the
result, so the recorded outcome, context keys, goal gates, events, and
routing all see the effective outcome; this replaces AutoStatusLifecycle
- Explicit routes take priority: a matching condition, preferred label,
suggested next node, or handler jump keeps the outcome failed. A failed
outcome takes an unconditional edge only under route, so under succeed
any edge selection is an explicit route
- succeed applies only to failed, matching exit; the auto_status alias no
longer promotes partially_succeeded
- Parallel branches promote after their retry loop, so a failed succeed
branch counts as succeeded in the parent aggregate
- Validation accepts succeed and adds an auto_status_deprecated warning
that suggests on_failure="succeed"
- Document the policy table, semantics, and deprecation; add a changelog
entry
Closes#807
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A node can now set its own on_failure attribute to override the
graph-level failed-node routing policy in either direction: a
best-effort node can keep route inside an exit graph, and a critical
node can exit while the rest of the graph keeps the default. An absent
node attribute inherits the graph policy.
- Node::on_failure returns Option<OnFailure> so absence means inherit
- Graph::resolve_on_failure(node_id) is the single resolution point,
returning ResolvedOnFailure { policy, scope } so the executor's
end-of-run message names the scope that stopped routing
- The core Graph trait method becomes resolve_on_failure(node_id); the
graph-scope failure message is unchanged
- The failed-human-gate fallthrough block stays independent of a
node-level route override
- Validation now accepts and value-checks node-level on_failure (it
previously warned that node placement had no effect) and keeps the
edge-placement warning with updated wording
- Document precedence in transitions, failures, and the DOT reference,
and extend today's changelog entry
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Chraa21RK7i2KqHdZSJLb8
Apply cleanup review findings on the model stylesheet template branch:
- Move the root-only stylesheet rule into visit_graph_references via a
GraphPosition parameter, so the bundler and workflow-version stop
re-implementing the entrypoint guard with duplicated match arms
- Let ModelStylesheetTemplateTransform build its own template store and
skip the pass entirely when the graph has no stylesheet; drop its dead
Transform impl and the template_render_store re-export
- Parse fix-message namespaces with the typed Namespace enum, share the
vars/goal fix strings with script_interpolation_fix, and replace the
attribute_name magic-string check with a restricted-namespace fix the
stylesheet transform sets on its own render target
- Drop template_render_store's content parameter; the store's render
always overwrites it before rendering
- Trim redundant tests and add a transform_options() helper in
pipeline/validate.rs tests
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019FBHEs42qNHDeKmsqTSDSQ
Collapse the triple Folder dispatch in run-intent admission into a single
prepare_intent_target call that canonicalizes and observes Git under one
provider gate, and stop feeding target/git into the compiler input only to
overwrite them afterwards. In run start, hoist the duplicated Folder
rejection out of the Docker and Daytona arms, restore kind_name() for the
Git/None arm, and drop the unreachable absolute/symlink checks that follow
canonicalize. Dedupe the folder-target test fixtures in both crates.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Dedup Diagnostic construction in the on_failure_valid rule
- Use the shared node_with_attrs test helper
- Drop an executor test that duplicated existing retry-target coverage
- Build on_failure integration test graphs from DOT and share a run
harness, exercising the parser path for valid on_failure values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J6MnJri6oSEMZaYeADY5dP
Use /tmp/fabro/runtime for both Docker and Daytona instead of
provider-specific roots. A writable /tmp inside the sandbox is already
a dependency (commit-message files, exec stop-files), it needs no
root-level mkdir for non-root container users, and it makes the two
providers uniform.
The trailing runtime path component stays load-bearing: materialized
blobs at runtime/blobs/{hash}.json are recognized as managed blob
references and normalized back to blob:// in durable context.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kmn5jyrdpyCdvcfvmEDvA
Remote prompt-value materialization wrote demoted values to
{working_directory}/.fabro/blobs inside the repository checkout, so a
later checkpoint could commit them and leak them into the run pull
request.
Give each sandbox a run-scoped runtime directory outside the source
checkout as part of the Sandbox contract:
- Sandbox::runtime_directory() names the directory; host-local
sandboxes return None because the engine owns a host-side runtime
directory (RunScratch) for those runs.
- Docker creates /fabro/runtime at initialize with umask 077 and
uploads runtime files with mode 0600.
- Daytona creates /home/daytona/fabro/runtime with mode 0700.
- Both remote materialization paths in fabro-workflow share one
materialization-path helper built on the new contract. The paths keep
the runtime/blobs suffix, so durable context still normalizes to
blob://sha256/... references.
- Local materialization now writes owner-private directories and files
on Unix.
Regression coverage: an integration test runs remote-style prompt
demotion against a real git checkout, then a real checkpoint commit,
and asserts the checkout stays clean, the agent-facing file is
readable, and a deleted materialized file is recreated from the
durable blob store. A real-Docker test verifies the runtime directory
and blob file permissions inside a container.
Fixes#798
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kmn5jyrdpyCdvcfvmEDvA
- Use a derived deserializer for RunTarget by making `None` an empty struct
variant, which keeps `deny_unknown_fields` strict without a hand-rolled impl
- Make clone_source_for_run the single owner of the empty-workspace decision
and drop the duplicated target checks in RunSession::new
- Collapse duplicated target/provider compatibility matches in admission and
start into single matches, using a strum-derived kind name for messages
- Drop the redundant git override in persist_create_run
- Extract a shared helper for the duplicated unavailable-integration test loop
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_blob_store was a process-wide OnceLock singleton over one in-memory
SQLite connection, so content-addressed rows written by one test were
visible to every other test in the same process. nextest's
process-per-test model masked the bleed, but plain cargo test failed
(8/24 in fabro-workflow-version) because negative existence assertions
became order-dependent.
test_blob_store now builds a fresh isolated in-memory store per call,
and test_database gives every database its own blob authority.
Reopen-style tests that model one durable blob authority across several
store handles use the new test_blob_store_at, which keeps the blob table
in a SQLite file beside the store directory, plus
test_database_with_blobs to share it explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fabro-workflow's and fabro-server's src/test_support.rs import
fabro_store::test_support, but their test-support features never enabled
fabro-store/test-support. Workspace builds passed only through feature
unification from other members' dev-dependencies, while per-crate builds
such as `cargo check -p fabro-cli --tests` or
`cargo check -p fabro-server --features test-support` failed with E0432.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Blob activation cleanups:
- Reuse fabro-db's append_to_path, remove_file_if_exists, and
set_private_permissions instead of local duplicates.
- Return the store directly from activate_blob_storage; the report
wrapper existed only to be logged internally and then discarded.
- Collapse compute_disk_preflight to return the required free bytes
instead of echoing its inputs back through a struct.
- Deduplicate the "exactly one ok row" PRAGMA integrity_check protocol
into one executor-generic helper used by the backup and live checks.
- Skip re-validating a freshly published backup; the staging copy was
validated immediately before the atomic rename, so only a
concurrently published file needs its own validation.
- Replace the manual anyhow wrapping plus duplicate error log in
serve.rs with a plain .context(), matching other startup errors.
- Extract the disk-candidate enumeration in resource_sampler.rs that
available_space_for_path had copy-pasted from sample_disk_resources.
Test fixture cleanups:
- Route all hand-assembled Database::new(..., test_blob_store()) test
fixtures (32 sites) through fabro_store::test_support::test_database,
and make that helper infallible instead of returning an unconditional
Ok.
- Install the test blob schema from fabro_db::BLOBS_MIGRATION_SQL via a
test-support-gated optional dependency instead of a four-level
relative include_str! into fabro-db's migrations directory.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Start reconciled the persisted target against its stored GitContext
projection field by field and failed the run on any drift, which forced
every RunSpec writer to keep the pair in lockstep forever. The target is
validated at admission and owns the grammar, so derive the clone source
from it alone; the projection stays persisted as display metadata that
can no longer fail an otherwise-healthy start.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Git-target grammar (slug, branch, and SHA rules plus the derived
origin URL) was implemented twice with no shared code path: once in
server admission and again in sandbox start, so the two could drift and
disagree about which persisted targets are valid.
Own it once as RunTarget::validate() in fabro-types, next to the
primitives it uses, returning the canonical target together with its
derived GitContext projection. Admission consumes it directly, and the
start path re-derives the expected clone source from the same rules
before checking the persisted projection against it. The start path now
also moves the derived strings into the sandbox spec instead of cloning
them.
While reordering admission around the shared validator, run the pure,
in-memory checks (target grammar, environment id) before the blob-store
closure fetch and lowering so malformed requests no longer pay for
version-store I/O.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Apply the cleanup findings from a four-angle review (reuse,
simplification, efficiency, altitude) of the demotion change:
- Share one size gate: serialized_if_over now backs both offload_value
and demote_value_for_prompt, restoring the cheap short-string and
scalar pre-checks so per-node demotion no longer serializes every
small value just to measure it.
- Stop re-writing blobs every node: materialize_value_bytes writes the
sandbox file directly from the in-hand bytes and short-circuits on the
content-addressed file's existence, so an already-demoted value costs
one existence probe instead of a store round-trip per node visit. The
local file write is shared with materialize_blob_ref.
- Demote over the resolved snapshot map instead of re-snapshotting a
Context copy, making the context and outcome loops symmetric and
saving a full deep clone per node; the fidelity lifecycle builds the
Context after the pass.
- Skip the pass entirely for Full and Truncate fidelities (nothing
renders context values), except parallel nodes whose branch stash may
render at a richer fidelity.
- Build is_preamble_hidden_key on is_engine_internal_key instead of
restating its prefixes, and call it directly from the preamble
renderer rather than through a wrapper.
- Document that outcome updates are demoted wholesale and that
BranchWorkItem.item carries the prompt-ready (possibly demoted) item;
drop the item rebinding and redundant test assertions; restore the
local integration test's confinement assertion and make the remote
one non-vacuous.
Skipped by choice: unifying the crate's several truncation helpers and
rendering the marker through the "See:" pointer family (cross-module
coupling out of proportion to the preview cosmetics), per-branch
demotion inside parallel.results (wholesale demotion is what bounds the
total), and cross-node demotion memoization (the file-existence
short-circuit already reduces repeats to a stat).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FH8Jj9Y4E4Tu5g1jwDtHAb
