GET /repos/{owner}/{repo}/installation returns 404 both when the App is
not installed for the owner and when the installation's repository
selection excludes the repository. The single-repository mint path
reported only the first cause, which misleads users whose App is
installed but not scoped to the repository. Name both causes and the
repository in the error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keep VACUUM snapshots private until permissions and durability are established. Refuse to recreate a missing rollback backup after import has begun, and preserve secondary cleanup failures in startup logs.
The activation module described itself as a temporary compatibility
bridge but bypassed the structure the migrations strategy prescribes: no
dated migrations/ file, no src/migrations.rs registry entry, no
REMOVAL_DEADLINE, and no removal_deadline log field. The strategy doc's
removal checklist (grep REMOVAL_DEADLINE, explicit registry ordering)
would never have surfaced it, letting the bridge silently outlive its
window as a second, parallel migration mechanism in serve.rs.
The module now lives at migrations/2026082301_sqlite_blob_activation.rs,
is registered and re-exported through src/migrations.rs like the two
existing server migrations, carries a REMOVAL_DEADLINE eligibility floor
(removal still requires the evidence and explicit approval in the module
docs), and logs removal_deadline on every activation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Neither the pre-activation backup nor the pre-migration snapshot fsynced
the staged file contents or the parent directory around the publishing
rename. A crash after the import committed could lose the retained
'.pre-blob-activation.bak' (whose directory entry was never made
durable), and the next activation would then write a new backup that
already contains the imported blobs, silently breaking the documented
pre-activation rollback boundary; a torn staging file could likewise
wedge later boots in backup validation.
write_snapshot_to_staging now syncs the staged file before handing it to
the caller, and both publishers sync the destination's parent directory
after their rename (fabro-db on a blocking task, activation inside its
existing blocking publication task).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
create_backup re-implemented the staging half of fabro-db's
pre-migration snapshot (remove stale staging file, UTF-8 check,
VACUUM INTO, private permissions), and remove_file_if_exists and
set_private_permissions had been made pub precisely to hand-copy that
sequence. Any future hardening of snapshot staging would have had to
land in two crates and could drift.
fabro-db now exposes write_snapshot_to_staging with a typed
SnapshotStagingError; both the pre-migration snapshot and the
pre-activation backup stage through it, and the hand-copied helpers are
private again. The publish halves stay separate on purpose: migrations
overwrite their snapshot, activation publishes with persist_noclobber
plus integrity validation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PRAGMA wal_checkpoint(TRUNCATE) returning busy=1 aborted server startup.
Any external reader that outlives the pool's five-second busy timeout (a
replication agent, a backup tool, an operator sqlite3 shell) would crash
the boot, and a supervisor restart would loop into the same abort while
the reader persisted, over a condition that threatens no data integrity.
A busy truncate now logs a warning and startup continues; a later
checkpoint truncates the WAL once the reader is gone. Adds the
failure-path coverage the relocated checkpoint lost: a held read
snapshot blocks the truncate and activation still succeeds.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
available_space_for_path returning None aborted startup with a fatal
UnknownFilesystem error, even on a fresh install with zero legacy rows.
Hosts with tmpfs or squashfs roots, network-filesystem data paths, or an
unreadable mount table would fail every boot with no operator override,
while the resource sampler already treats the identical condition as
benign (supported: false) and keeps running.
The preflight now logs a warning and is skipped when free space cannot
be determined; the import, verification, and integrity checks still run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The preflight demanded ~1.5x the full legacy inventory bytes free on
every startup, with no credit for rows already imported. Because the
first activation itself consumes about twice the legacy bytes (the
SQLite copy plus the retained backup) and the legacy keyspace stays in
place for the whole retention window, a successfully activated server
could fall below the requirement and become unable to restart until an
operator freed space the server would never write.
The legacy inventory now checks each row's hash against the SQLite blobs
table and reports pending rows and bytes, and the preflight requires
1.5x only the pending bytes plus the backup reserve and fixed headroom.
A warm restart with nothing left to import needs only the headroom.
Also updates the server operations doc for this and for the
verification pass now running only on boots that import rows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Startup previously scanned the legacy SlateDB keyspace three times and
SHA-256-hashed every value in each pass (inventory, import,
verification), then read and rehashed every row of the live SQLite blobs
table — on every boot, even a warm restart with nothing to import. With
a large object-store-backed legacy keyspace that makes restart time
proportional to total blob bytes for the whole retention window.
The inventory pass now only validates key shapes and sizes the keyspace;
digests are still validated by the import pass before any row persists.
The independent verification sweep now runs only on boots whose import
actually inserted rows: the import pass itself byte-compares every
already-present legacy row each boot, so a no-op restart is already
fully cross-checked without a third scan or a full-table rehash.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
import_legacy_blobs_into and verify_legacy_blobs_in took a &BlobStore and
extracted its pool through sqlite_pool_for_legacy_import, an Option that
was statically always Some in production (the None arm existed only for
the test-only Slate backend). That accessor forced a clippy
unnecessary_wraps suppression and two WrongTargetBackend error variants
no production caller could ever hit, and the activation path round-tripped
a pool it already owned through a BlobStore it had just built.
Both functions now take &SqlitePool, deleting the accessor, the
suppression, both unreachable variants, and their rejection test.
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>
Make append_to_path, remove_file_if_exists, and set_private_permissions
public so callers stop keeping verbatim private copies, and export the
blobs migration SQL so fixtures in other crates can install the blob
schema without a relative filesystem path into this crate's source tree.
set_private_permissions now returns io::Result so each caller owns its
own error context.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Apply cleanups from a reuse/simplification/efficiency review of the
bounded-tool-output changes:
- Share one MAX_RUN_EVENT_BODY_BYTES constant in fabro-types; the server
body limit, the agent's serialized-output reservation, and the event
headroom test all derive from it.
- Rework truncation.rs around one split_head_tail helper: drop the
hand-rolled ceil_char_boundary (std's is stable), the duplicate
truncate_plain_output splitter and its dead Tail arm, and the
head_bytes field with its sentinel values.
- Return Cow from preview_tool_output and take retain_tool_output's
input by value, so untruncated output crosses the pipeline without
full copies. Measure serialized JSON size with a counting writer
instead of materializing the payload.
- Reuse fabro-llm's byte-token estimate (now public) instead of a third
copy of the 4-bytes-per-token heuristic.
- Take retain_tool_result's ToolResult by value and mutate content in
place; extract the triplicated error retain-emit-truncate block into
finish_error_result.
- Share the shell retain-and-record sequence between the native and
kimi shell tools as retain_shell_output.
- Move OutputCaptureBuffer::into_parts to reuse the head allocation,
skip the buffer round-trip in replay_exec_result when output fits,
and replace daytona's byte-iterator suffix matching with contiguous
slice comparisons behind one retained_slices accessor.
- Make SessionBoundEmitter's fields private.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TK3QTWQHiXhRbFwTr57LzX
duration_to_minutes_i32 carried stacked docker and daytona cfg attributes,
which combine as AND, so building with the daytona feature alone failed to
find the function. fabro-workflow and fabro-cli enable daytona without
docker in their production dependencies, so that combination is real.
Removing the stray docker gate surfaced items that only docker-gated code
uses: the ResolveError import in from_environment and four exact-checkout
command builders in clone_source. Gate those on the docker feature, keeping
the command builders available to clone_source's own tests under cfg(test).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TK3QTWQHiXhRbFwTr57LzX
Cap closure expansion at 256 distinct workflow mounts. Mounts are keyed
by rebased path, so a small chain of stored versions that mounts a
shared dependency along two paths per level expands exponentially; a
single authenticated create request could stall the server before any
error was returned. The check also bounds the recursion depth.
Resolve file-form run goals through the certified version: expose
ValidatedWorkflowVersion::resolved_goal_file_content, which reuses the
exact grammar store validation certified, and drop the parallel
resolution (and its unreachable-for-stored-versions error variants) the
server had re-implemented. The certified entrypoint-presence invariant
replaces the MissingEntrypoint error the same way.
Destructure both environment layer types without `..` when pinning
server environment authority, so a new server-owned field becomes a
compile-time decision instead of silently escaping the pin.
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 selector grammar ran against a heads/-prefixed string, so its
leading-character rules saw the prefix instead of the branch: names git
itself rejects, like -foo or HEAD, passed admission and only failed
later at sandbox clone time. Check the bare branch name and reject a
literal HEAD explicitly.
Also build the Git projection's origin URL through
GitHubRepositorySlug::https_url so the URL grammar keeps one owner.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lowering and compiler rejections now carry the top-level error message
in the 422 detail, matching the diagnostic depth the legacy manifest
lane already returns for identical defects; the full source chain stays
in the server log.
Pre-persistence store failures stop claiming run_persistence_failed:
credential-store reads return credential_store_error and run-variable
snapshots return variable_store_error, so alerting keyed on codes
triages the failing subsystem instead of a persistence outage.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both lanes now deserialize the raw request bytes directly instead of
round-tripping through a serde_json::Value, which silently collapsed
duplicate JSON keys to last-key-wins on the legacy manifest lane and
stripped line/column locations from manifest parse errors.
When neither lane accepts the body, attribution now recognizes a
defective manifest by its required keys, so a legacy manifest carrying a
stray workflow_version_id keeps its 400 manifest error instead of being
misrouted to a 422 run_intent_invalid describing a schema the caller
never used.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
create_run_from_manifest and create_run_from_intent were byte-identical
apart from the body type; fold the shared request/retry plumbing into a
private submit_create_run(CreateRunRequest) so the two public entry
points stay thin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The create-run dispatcher deep-cloned the parsed JSON body once to
attempt the RunIntent shape and again for the RunManifest fallback,
so every legacy manifest request paid two full copies of a body that
carries entire workflow bundles. Deserialize both shapes from a
reference to the parsed value instead; routing and error attribution
are unchanged.
Also bind the lowered goal slot once in inline_goal_file rather than
re-navigating the settings layer and asserting the goal is still there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The intent and legacy-manifest create handlers each carried a full copy
of the same post-admission sequence: LLM readiness resolution, graph
compilation and model pinning, persistence, summary read, managed-run
registration, title-generation spawn, and the 201 response. The copies
had already drifted on when the run ID is resolved (before compilation
in one lane, after in the other).
Extract one finalize_created_run tail, with a small CreatedRunErrorStyle
carrying each lane's pinned error mapping and log lines so the wire
contracts are unchanged. Both lanes now resolve identity before
compilation and share the parent-link validation, which lets the
PinnedRun copy of PreparedRun's identity accessors be deleted.
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>
Preserve the SQLite auth-session release notes alongside main's July 26 fixes and retain all current changelog navigation entries. Make the refresh-token rotation timestamp assertion deterministic after the merged suite exposed its wall-clock race.
