Skip to content

Initial backend revisions, workflow expansion#5

Merged
tlongwell-block merged 16 commits into
mainfrom
initial_revisions
Mar 10, 2026
Merged

Initial backend revisions, workflow expansion#5
tlongwell-block merged 16 commits into
mainfrom
initial_revisions

Conversation

@tlongwell-block
Copy link
Copy Markdown
Collaborator

@tlongwell-block tlongwell-block commented Mar 9, 2026

Wires the end-to-end workflow execution pipeline: when a Nostr event arrives, matching workflows
now evaluate filters, build trigger context, create a run record, and spawn async execution —
completing the path from on_event() through to Completed/Failed status. Also consolidates
scattered kind constants into sprout-core, removes dead code, and hardens NIP-25 reaction handling.

- Centralize KIND_AUTH (22242), KIND_CANVAS (40100), EPHEMERAL_KIND_MIN/MAX
  into sprout-core/src/kind.rs; remove 3 duplicate local definitions
- Add helpers: is_ephemeral(), is_workflow_execution_kind(),
  event_kind_u32(), event_kind_i32() — eliminates 9 cast chains
- Add compile-time value assertions (kind constants fit in u16)
- Remove dead Redis subscriber spawn (zero consumers)
- Remove dead cron scheduler stub spawn (no-op loop)
- Wire TYPESENSE_COLLECTION env var in relay main.rs
- Document sprout-mcp re-export dependency for sprout-test-client
- Add TYPESENSE_COLLECTION to README config table

Zero behavior change. All unit, E2E, and integration tests pass.
…xecution

- on_event() fully wired: trigger matching → filter evaluation →
  TriggerContext build → DB run creation → async executor spawn
- Changed on_event() signature to self: &Arc<Self> for spawn compatibility
- Extracted build_trigger_context() as pub helper (StoredEvent → TriggerContext)
- Re-enabled cron spawn in relay main.rs (stub loop, WF-09)
- Added 6 unit tests for build_trigger_context
- Added 2 E2E tests: event-driven workflow execution + filter evaluation

Crossfire fixes (GPT-5-4 + Claude):
- Fix reaction message_id: extract target from NIP-25 'e' tag, not reaction event ID
- Fix pending-stuck runs: mark Failed if Running transition fails
- Wire run_semaphore: acquire permit before spawning execution
- Warn on trace serialization failure instead of silent fallback

All 280+ unit tests pass, all 6 E2E workflow tests pass, clippy clean.
- Fix double semaphore: remove from on_event() spawned task, keep single
  acquire in execute_run() (now uses .acquire().await for queuing)
- Enforce reaction emoji filter in on_event(): skip workflows whose
  configured emoji doesn't match the event content (NIP-25)
- Duration overflow: use checked_mul for hours*3600 and mins*60
- Verified: evalexpr conditions use context variables, NOT template
  substitution — no injection vector

Crossfire scores: GPT-5-4 6.5/10, Claude 6/10, Codex 4/10.
Deferred to issues: trace durability, unbounded fan-out, SSRF IPv6,
approval persistence (WF-08), stale run recovery, webhook HTTPS.

All 280+ unit tests pass, clippy clean.
@tlongwell-block tlongwell-block changed the title Initial backend revisions Initial backend revisions, workflow expansion Mar 9, 2026
…eaction scoping, schedule rejection

- Revert semaphore from acquire().await to try_acquire() for fail-fast backpressure
- Move Running status transition into execute_run() after permit acquired
- Replace zombie WaitingApproval with explicit Failed (approval gates deferred to WF-08)
- Add derive_reaction_channel() for NIP-25 reactions — DB lookup of target event's channel
- Reject schedule/interval triggers at create/update until cron scheduler is implemented
- Add debug logging for events skipped due to missing channel_id
…ing, NIP-25 e-tag ordering

- Remove Running pre-set from spawn_workflow_execution and approval resume (executor owns it)
- Add semaphore to execute_from_step via private execute_steps helper
- Fail approval gates consistently across all trigger paths (deferred to WF-08)
- Fix build_trigger_context to use last e-tag per NIP-25 threading convention
- Add regression test for multi-e-tag reaction scenarios
…atus, partial trace preservation

- Always derive reaction channel from target event DB lookup (prevent channel tag spoofing)
- Set RunStatus::Running in execute_from_step after semaphore (approval resume path)
- Preserve resume_index and existing trace on resume failure instead of resetting to 0
- Update lifecycle comment in workflow_helpers.rs to reflect current behavior
…on scoping

- Preserve existing execution trace when marking resumed run as Running
- Reject reactions when target event cannot be resolved (fail closed, not open)
- Prevents authorization downgrade on DB lookup errors for reactions
- Read existing trace/step from DB before marking run as Failed
- Prevents loss of partial progress when executor errors mid-run
- Applied to both event-triggered (lib.rs) and API-triggered (workflow_helpers.rs) paths
All 3 E2E test files used stale placeholder UUIDs (aaaaaaaa-...)
that don't exist in the database. The actual channels use UUID5-derived
IDs seeded at startup. Since the channel didn't exist, get_channel()
returned None, the open-visibility check defaulted to false, and the
API returned 403.

Changes:
- CHANNEL_GENERAL: aaaaaaaa-...-aaa1 → 9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50
- CHANNEL_PROJECTS → CHANNEL_ENGINEERING: 1c7e1c02-87bb-5e88-b2da-5a7a9432d0c9
- Updated doc comments and assertion messages

All 31 E2E tests pass (6 workflow + 18 REST API + 7 MCP).
…tics, error logging

Fixes all critical and important issues from the 3-model crossfire review:

1. Partial trace preservation (Codex + Gemini):
   - Added PartialProgress struct to carry step_index + trace in error path
   - execute_steps() now returns (WorkflowError, PartialProgress) on failure
   - All callers (on_event, spawn_workflow_execution, approval resume) use
     the executor's partial trace instead of re-reading stale DB state
   - Trace is never lost on mid-execution failures

2. Reaction channel semantics (Gemini + Opus):
   - derive_reaction_channel() now returns ReactionChannelResult enum with
     5 variants: Channel, NoChannel, NotFound, NoTarget, DbError
   - Caller distinguishes 'target exists but is global' (allow) from
     'target not found' (reject) and 'DB error' (reject with server error)
   - Reactions to global/DM messages are no longer incorrectly rejected

3. Silent error drop (Opus):
   - Replaced let _ = on approval gate DB update with if let Err(e) logging
   - Consistent with all other DB update error handling paths

4. Null trace consistency (Codex):
   - Changed Value::Null to json!([]) in definition-parse failure path
   - All trace fields now consistently use JSON arrays
… error logging

Fixes from round 2 crossfire (Codex 6/10, Opus 8/10):

1. Approval resume status guard (Codex):
   - Validate run.status == WaitingApproval before resuming
   - Prevents stale approval tokens from resurrecting cancelled/failed/completed runs

2. Reaction tag filtering alignment (Opus):
   - derive_reaction_channel now filters for 64-char hex inside find_map
   - Consistent with build_trigger_context's approach — skips UUID channel refs
   - Fixes edge case where last e-tag is a channel UUID

3. Silent DB error in execute_from_step (Opus):
   - Log warning when get_workflow_run fails during resume
   - Pre-approval trace loss is now visible in logs

4. Dead code removal (Opus):
   - Removed unused with_progress() helper on WorkflowError
Add run.status == WaitingApproval guard to deny_approval path, consistent
with the grant_approval path added in round 7.

Without this guard, a stale deny token could overwrite a terminal run
status (Failed/Completed/Cancelled) with Cancelled, corrupting state.
Now both grant and deny paths validate the run is still waiting before
mutating it.
Structural refactor addressing valid review feedback:

1. Extract WorkflowEngine::finalize_run() — single place for the 3-way
   executor result → DB status mapping. Eliminates ~170 lines of
   near-identical code duplicated across on_event, spawn_workflow_execution,
   and resume_workflow_after_approval.

2. Extract should_fire_workflow() — emoji filter and trigger filter
   expression evaluation pulled out of on_event's per-workflow loop body.
   on_event is now ~90 lines (was ~225).

3. Remove _workflow_id parameter from spawn_workflow_execution — was
   prefixed with underscore since approval gate removal, now gone entirely.

4. Remove handle_approval_suspension dead code — was #[allow(dead_code)]
   since round 6. The lifecycle logic now lives in finalize_run; WF-08
   will re-implement suspension there when needed.

All 280+ unit tests pass, all 44 E2E tests pass, zero clippy warnings.
* origin/main:
  Add desktop Home feed (#12)
  Add desktop Playwright e2e harness (#11)
  Update desktop icon and persist window state (#9)
  feat: add channel creation flow (#8)
  Improve message markdown display and formatting (#7)
  feat(desktop): connect chat to relay (#6)
  docs(readme): clarify desktop setup (#4)
  feat: add desktop app (#3)

# Conflicts:
#	crates/sprout-test-client/tests/e2e_rest_api.rs
… E2E coverage

- Fix delay/timeout race: MAX_DELAY_SECS 300→270 (must be < default_timeout_secs)
- Close auth gap on global workflows: get/update now check owner_pubkey when channel_id is NULL
- Return 202 Accepted from grant/deny approval endpoints (async work follows response)
- Normalize check_approver_spec to case-insensitive hex comparison
- Serialize trigger_ctx_json once before workflow loop (was redundant per-workflow)
- Fix ARCHITECTURE.md: only kind 40001 triggers workflows, not 40002
- Fix misleading KIND_JOB comments in feed.rs
- Add E2E test for approval gate stub (WF-08)
@tlongwell-block tlongwell-block merged commit cd3b45f into main Mar 10, 2026
8 checks passed
@tlongwell-block tlongwell-block deleted the initial_revisions branch March 10, 2026 14:46
tlongwell-block added a commit that referenced this pull request Mar 11, 2026
* origin/main:
  feat: soft-delete for events/channels, enriched API responses, NIP-29 group management (#17)
  feat: Channel management, messaging, threads, DMs, reactions, and NIP-29 support (#16)
  Improve chat scrolling and multiline composer (#14)
  chore: remove redundant inline comments across all crates (#13)
  Initial backend revisions, workflow expansion (#5)
  Add desktop Home feed (#12)
  Add desktop Playwright e2e harness (#11)
  Update desktop icon and persist window state (#9)
  feat: add channel creation flow (#8)
tlongwell-block added a commit that referenced this pull request Mar 12, 2026
…ers, robust tag selection

Critical fixes:
- Verify event.pubkey == AUTH pubkey and signature before translation (was: unverified)
- Unknown #e filter values → deny-all sentinel (was: fall back to all allowed channels)
- Channel tag selection by map lookup, not first-match (handles reordered tags)

Important fixes:
- ids filter uses local check instead of continue (was: skipping kind:41 for same channel)
- No-kind filters now also serve local kind:40/41 metadata (was: upstream-only)

Tests: 54 passing, clippy clean
tlongwell-block added a commit that referenced this pull request Mar 16, 2026
Crossfire round 1: codex 4/10, opus 8/10. All critical issues fixed:

Security (critical):
- Force channel_id=None for kind:1059 gift wraps — prevents channel-scoped
  storage that would bypass #p AUTH-gating (codex finding #1)

Correctness:
- NIP-50 pagination loop — keep fetching Typesense pages until limit met
  or result set exhausted, capped at MAX_SEARCH_PAGES=5 (codex finding #2)
- Push authors/since/until to Typesense filter_by — post-filtering is now
  a correction step, not the primary filter (codex + opus suggestion)
- NIP-10 root tag validation — reject events where client-supplied root
  diverges from server-resolved ancestry (codex finding #3)

Clarity:
- Consolidate #p gating into single P_GATED_KINDS check (opus suggestion #7)
- filter.clone() → std::slice::from_ref(filter) (opus suggestion #1)
- Remove no-op get_events_by_ids test, add debug_assert (opus #3, #5)
tlongwell-block added a commit that referenced this pull request May 14, 2026
Adds a Settings → Agent Provider panel to the desktop GUI for configuring
the `sprout-agent` provider, model, API key, and behavior knobs. Settings
are encrypted at rest with NIP-44 self-encryption (the user's own nostr
key) and injected into the sprout-agent child's env at spawn time. The
panel also gains automatic provider-URL detection based on the API key
format the user pastes.

## Backend (`desktop/src-tauri/src/commands/agent_provider_settings/`)

- `mod.rs` — IPC types + plaintext `StoredSettings` (Drop zeroizes
  `api_key`; no Debug derive on input/stored). v2 envelope binds the
  plaintext to its owner pubkey for rollback / envelope-swap protection.
- `storage.rs` — envelope read/write, NIP-44 encrypt/decrypt with
  `Zeroizing<String>` for plaintext, atomic-rename writes, file-size
  cap, `normalize_origin` (rejects non-loopback `http://`, userinfo,
  query, fragment), `validate_input` (provider whitelist, control-char
  rejection, size caps for key/model/base_url/system_prompt, positive-
  int knobs). `validate_stored` mirrors the same rules on the decrypted
  blob at spawn time — fails closed on a rolled-back pre-validation
  envelope so a redirected `http://api.example.com/v1` cannot escape.
- `commands.rs` — `get_*`, `save_*`, `delete_*`, `get_*_env_presence`
  Tauri commands. The save command trims whitespace + zeroes the input
  api_key before validation can early-return.
- `spawn.rs` — `LoadForSpawn` enum + `EnvPairs` newtype whose Drop
  zeroizes every value buffer. `apply_to_command` hands each env pair
  to `Command::env` by reference, zeroizing the local buffer after.
  Spawn policy: Ok → strip OWNED_AGENT_ENV_VARS + ACP-level vars then
  inject; None → no-op; IdentityMismatch / Error → fail closed (strip
  inherited owned vars, inject nothing).
- `tests.rs` — round-trip envelope I/O, identity-rotation, save-time
  validation (oversized prompt, zero timeouts, tiny history bytes,
  unknown provider, control chars, oversized fields, api-key whitespace
  trim, owner_pubkey v2), `apply_to_command` × `LoadForSpawn` matrix
  (Ok/None/IdentityMismatch-fails-closed/Error-fails-closed, openai
  dialect), `stored_to_env_pairs` for each dialect, R7 `validate_stored`
  coverage (non-loopback http, control chars in key/model/base_url,
  userinfo/query in base_url, unknown provider, oversized prompt,
  empty-key + loopback local accepted).

## Runtime integration (`desktop/src-tauri/src/managed_agents/runtime.rs`)

- `build_agent_command` calls `agent_provider_settings::apply_to_command`
  exactly when the harness is `sprout-agent`. ACP-level vars
  (SPROUT_AGENT_PROVIDER etc.) are stripped from inherited parent env
  before injection so a stale shell `ANTHROPIC_API_KEY` never shadows
  saved settings. `respond-to` gate env (`SPROUT_ACP_RESPOND_TO[_ALLOWLIST]`)
  threads through with the new `owner_hex: Option<&str>` parameter
  (origin/main merge).

## Frontend

- `lib/detectProvider.ts` — pure key-format detector. Recognizes
  Anthropic, OpenAI (legacy/proj/svcacct via fixed infix), OpenRouter,
  Groq, xAI, Cerebras, Together, Perplexity, Fireworks (medium), bare
  sk- → DeepSeek (low + ambiguity-aware), plus localhost/127.0.0.1
  patterns for Ollama / vLLM / llama.cpp. Includes ADMIN_ONLY_PROVIDER_ID
  sentinel for `sk-ant-admin01-` which we explicitly refuse to save.
  Key format wins over a prefilled default base URL; an explicit non-
  default base URL wins back for medium-confidence keys (e.g. Fireworks
  + api.openai.com host). Fixture strings construct the OpenAI infix
  via concat so GitHub's secret scanner doesn't regex-match an inline
  OpenAI-shaped service-account/project key (`detectProvider.test.mjs`,
  `settings-agent-provider.spec.ts`).
- `lib/providerCatalog.ts` — declarative catalog (id, label, dialect,
  isLocal, default model + base URL, key-shape hint). Drives the
  picker, the auto-fill on detection, the local-provider placeholder
  enforcement, and the per-provider model field default.
- `lib/agentProviderFormState.ts` — FormState shape + reducers.
  `applyProviderSwitch` is the single source of truth for what gets
  reset on a provider change (model when empty or still previous
  default; baseUrl when new provider has a default OR user hasn't
  edited it; clears previous default host for null-default providers;
  drops apiKey on switch TO a local provider). Used by both the
  manual picker and the auto-detect effect so the policy can't drift.
- `lib/agentProviderSettingsApi.ts` + `hooks/useAgentProviderSettings.ts`
  — typed IPC wrappers + React-Query hooks (load / save / delete /
  envPresence).
- `ui/AgentProviderSettingsCard.tsx` — the panel itself. Empty state
  with shell-env hint, identity-rotation banner, load-error banner,
  detected-provider badge, reveal/hide toggle, advanced section,
  inline provider-change warning, confirm-clear dialog. On save success
  the plaintext is wiped from form state + reveal toggles off,
  independent of any React-Query refresh effect (covers the structural-
  sharing identical-redacted-view edge case).
- `ui/AgentProviderAdvancedFields.tsx`, `AgentProviderBanners.tsx`,
  `AgentProviderClearDialog.tsx` — split components.

## Per-agent dialog (sprout-agent special case)

- `agents/ui/CreateAgentDialogSections.tsx` — Model + System prompt
  inputs are hidden for sprout-agent paths (those are owned globally).
  A note line points users to Settings → Agent Provider.
- `agents/ui/EditAgentDialog.tsx` — passes `selectedProviderId="custom"`
  to the shared runtime fields so the agent-command input stays
  editable for existing rows; the system-prompt hide still resolves
  via `isSproutAgentPath`'s `agentCommand` arm.
- `agents/ui/ManagedAgentRow.tsx` — "Model managed by Sprout settings"
  link is a span with role="button" + stopPropagation (was a nested
  `<button>` inside the row button — both invalid HTML and double-
  triggering).
- `agents/lib/resolveAcpProviderId.ts` — TS / Rust alignment for
  inline-args resolution (Rust `known_acp_provider` strips args; TS
  now matches).

## Tests

- Rust: 328 tests passing (R7 added 7; full agent_provider_settings
  suite at 44/44).
- TS / node-test: 55 cases for the form-state reducer + provider
  detector.
- Playwright integration: 12 settings-agent-provider scenarios (empty
  state + detection + save round-trip, identity-rotation banner,
  provider-change-warning, key-format-beats-prefilled-baseUrl, load-
  error banner, clear flow + Escape cancel, rotation banner a11y,
  local-provider switch with saved key, manual switch reset, detected-
  provider model reset, post-save key-input clear).

## just ci summary

- Rust: 321/321 + 7 new validate_stored tests
- Mobile: 336/336
- Desktop / web: format + biome + file-size + ts-check all green
- Playwright integration agents + settings-agent-provider: 19/19 green

## Codex review history

- Reviews #1, #2, #3, #5, #6 surfaced and fixed: non-loopback HTTP at
  save AND spawn, identity-mismatch fails closed, local-provider key
  leak prevention, inline-args resolver alignment, Zeroize on api_key
  before early-return, no Debug derive, control-char / length caps +
  trim, owner_pubkey v2 envelope, local-provider switch unblock,
  EditAgentDialog command field, model reset on detection switch,
  nested-button fix, `applyProviderSwitch` reducer extraction.
- Review #7 (P2-UI + P2-Rust): clear form.apiKey + revealKey on save
  success; validate decrypted settings at spawn time.
- Review #8: 9/10, no blocking findings.

Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com>
Co-authored-by: Dawn <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
tlongwell-block pushed a commit that referenced this pull request May 22, 2026
Adds crates/sprout-relay/src/api/git/cas_publish.rs — the pure async
function that turns a post-receive-pack workspace into a durable manifest
CAS. Composes Dawn's GitStore primitives (put_pack, put_manifest,
get_pointer, put_pointer) and Dawn's Manifest schema (canonical_bytes,
validate, Inv_Closed at compose time) into the spec's step 2-7 sequence:

  read pointer (e, d_before)            §step 3
  fetch + verify m_before                §step 3 + A1 detectability
  snapshot refs + symref-HEAD from disk (HEAD inherits parent on detach)
  pack new objects via pack-objects --revs    §step 1-2
  put_pack(bytes) -> packs/<sha256>          §step 2 (A1)
  compose m_after (parent packs + new pack, parent = digest only) §step 5
  m_after.validate()                          (Sami/Max/Perci #2-#4)
  put_manifest(canonical_bytes) -> manifests/<sha256>  §step 6
  put_pointer(IfMatch(e) | IfNoneMatchStar)  §step 7 (CAS)
    Won  -> CasSuccess { manifest, manifest_key }
    Lost -> CasError::Conflict { winner_manifest, winner_manifest_key }
            (→ HTTP 409, with winner for disk reconcile)

The function returns *before* a Response is constructed — it is called
from finalize_push, which is the unique site that builds a push 2xx, so
the structural seam still enforces Theorem 1 (success-after-CAS).

## Review fixes folded in

Sami's review (#1#6) + Perci's #1 + Max's pre-CAS-validation blocker
are all addressed in this commit:

- **parent = bare 64-hex digest, not full key** (Perci #1, Max). Pointer
  body is `<digest>`; `Manifest.parent` stores the same digest, matching
  `Inv_RefDerivedFromParent` literally. `read_parent` strips the
  `manifests/` prefix before assigning. Dawn's new `MalformedParent`
  validator catches any drift at the write seam.

- **Pre-CAS validation** (Sami #2, Max). `m_after.validate()?` runs
  between `compose_after` and `put_manifest`. Unsafe refnames, malformed
  oids, empty HEAD — all surface as `CasError::ManifestInvalid(...)`
  (4xx-class) before any S3 write, *not* as "valid CAS, un-clone-able
  output." Typed variant (not reused `ManifestReadFailed`) so logs /
  status mapping distinguish "client input rejected" from "stored parent
  failed A1" (Max + Dawn).

- **Detached-HEAD fallback** (Sami #3). `snapshot_workspace_state`
  returns empty `head` on detached HEAD; `cas_publish` falls back to
  `parent.head` if non-empty. `validate()` rejects the first-push-+-
  detached case (Sami #4 — no parent to inherit from, manifest is
  un-clone-able).

- **Conflict carries the winner** (Sami #5 + Dawn). `Conflict {
  winner_manifest, winner_manifest_key }` lets `finalize_push` invoke
  Eva's `reconcile_to_manifest` mechanically from the error arm, without
  a second pointer GET in the caller. `warn!()` at the `LostRace` site
  logs (pointer, expected etag, attempted manifest) for debugging
  concurrent-push patterns. Boxed for `clippy::result_large_err`.

- **Empty-pack comment** (Sami #6). Clarified `capture_pack` returns
  `None` in both the delete-all (`refs_after.is_empty()`) and refs-only
  (`pack-objects` empty stdout) cases.

- **`pointer_key` consolidated** in `manifest.rs` (Sami #1, Dawn,
  Max — Sami's "single source of truth" argument). `cas_publish`
  imports it; the duplicate definition is gone.

- **`validate-invocation` test added** in `cas_publish.rs` (Sami's
  recommendation). Pins that a future refactor dropping the `validate?`
  call between `compose_after` and `put_manifest` is caught by unit
  test, not by every subsequent un-clone-able read.

## What this deliberately does NOT do (each with citation)

- No retry on LostRace. Per Sami's TLA-action guidance: the receive-pack
  output is derived against a now-superseded parent; reusing it would
  violate Inv_RefDerivedFromParent. Client re-pushes, which re-hydrates
  + re-runs receive-pack against the advanced pointer — the only safe
  retry, which git already performs. Spec §Push step 7: 'GOTO 3 (retry)
  or respond non-ff' — both arms safe; we take non-ff.
- No kind:30618 emission. That is derived after CAS — finalize_push
  calls Sami's build_ref_state_event over m_after.refs / m_after.head
  on Ok. Spec §Implementation Correspondence: 'kind:30618 is derived
  after CAS, never the commit.'
- No advisory lock. Spec §Push 'no advisory lock in v1' — writer
  serialization is the CAS. A mutex would hide the contention Inv_NoFork
  proves safe.

## Tests

10 unit tests pin digest_from_key (manifest/<...> prefix invariant),
compose_after (Inv_Closed coverage, sort, dedupe, refs-only-no-new-pack,
first-push, parent-is-digest-not-key), validate invocation (unsafe
refname + first-push-empty-HEAD both rejected pre-CAS). 244 relay tests
green; clippy --tests -D warnings clean.

The integration into finalize_push lands separately — Eva owns the
AppState::git_store wiring + main.rs startup probe gate. This module
is callable today: cas_publish(&store, repo_path, owner, repo,
&refs_before) -> Result<CasSuccess, CasError>.

Refs:
- docs/git-on-object-storage.md §Push step 2-7, §Implementation
  Correspondence, §Mechanized Verification (Inv_NoFork,
  Inv_RefEffectApplied, Inv_RefDerivedFromParent, Inv_Closed).

Also makes transport::harden_git_env pub(crate) for reuse by
cas_publish's two subprocess sites (for-each-ref, pack-objects).

Co-authored-by: Tyler Longwell <tyler@block.xyz>
tlongwell-block pushed a commit that referenced this pull request May 22, 2026
Adds crates/sprout-relay/src/api/git/cas_publish.rs — the pure async
function that turns a post-receive-pack workspace into a durable manifest
CAS. Composes Dawn's GitStore primitives (put_pack, put_manifest,
get_pointer, put_pointer) and Dawn's Manifest schema (canonical_bytes,
validate, Inv_Closed at compose time) into the spec's step 2-7 sequence:

  read pointer (e, d_before)            §step 3
  fetch + verify m_before                §step 3 + A1 detectability
  snapshot refs + symref-HEAD from disk (HEAD inherits parent on detach)
  pack new objects via pack-objects --revs    §step 1-2
  put_pack(bytes) -> packs/<sha256>          §step 2 (A1)
  compose m_after (parent packs + new pack, parent = digest only) §step 5
  m_after.validate()                          (Sami/Max/Perci #2-#4)
  put_manifest(canonical_bytes) -> manifests/<sha256>  §step 6
  put_pointer(IfMatch(e) | IfNoneMatchStar)  §step 7 (CAS)
    Won  -> CasSuccess { manifest, manifest_key }
    Lost -> CasError::Conflict { winner_manifest, winner_manifest_key }
            (→ HTTP 409, with winner for disk reconcile)

The function returns *before* a Response is constructed — it is called
from finalize_push, which is the unique site that builds a push 2xx, so
the structural seam still enforces Theorem 1 (success-after-CAS).

## Review fixes folded in

Sami's review (#1#6) + Perci's #1 + Max's pre-CAS-validation blocker
are all addressed in this commit:

- **parent = bare 64-hex digest, not full key** (Perci #1, Max). Pointer
  body is `<digest>`; `Manifest.parent` stores the same digest, matching
  `Inv_RefDerivedFromParent` literally. `read_parent` strips the
  `manifests/` prefix before assigning. Dawn's new `MalformedParent`
  validator catches any drift at the write seam.

- **Pre-CAS validation** (Sami #2, Max). `m_after.validate()?` runs
  between `compose_after` and `put_manifest`. Unsafe refnames, malformed
  oids, empty HEAD — all surface as `CasError::ManifestInvalid(...)`
  (4xx-class) before any S3 write, *not* as "valid CAS, un-clone-able
  output." Typed variant (not reused `ManifestReadFailed`) so logs /
  status mapping distinguish "client input rejected" from "stored parent
  failed A1" (Max + Dawn).

- **Detached-HEAD fallback** (Sami #3). `snapshot_workspace_state`
  returns empty `head` on detached HEAD; `cas_publish` falls back to
  `parent.head` if non-empty. `validate()` rejects the first-push-+-
  detached case (Sami #4 — no parent to inherit from, manifest is
  un-clone-able).

- **Conflict carries the winner** (Sami #5 + Dawn). `Conflict {
  winner_manifest, winner_manifest_key }` lets `finalize_push` invoke
  Eva's `reconcile_to_manifest` mechanically from the error arm, without
  a second pointer GET in the caller. `warn!()` at the `LostRace` site
  logs (pointer, expected etag, attempted manifest) for debugging
  concurrent-push patterns. Boxed for `clippy::result_large_err`.

- **Empty-pack comment** (Sami #6). Clarified `capture_pack` returns
  `None` in both the delete-all (`refs_after.is_empty()`) and refs-only
  (`pack-objects` empty stdout) cases.

- **`pointer_key` consolidated** in `manifest.rs` (Sami #1, Dawn,
  Max — Sami's "single source of truth" argument). `cas_publish`
  imports it; the duplicate definition is gone.

- **`validate-invocation` test added** in `cas_publish.rs` (Sami's
  recommendation). Pins that a future refactor dropping the `validate?`
  call between `compose_after` and `put_manifest` is caught by unit
  test, not by every subsequent un-clone-able read.

## What this deliberately does NOT do (each with citation)

- No retry on LostRace. Per Sami's TLA-action guidance: the receive-pack
  output is derived against a now-superseded parent; reusing it would
  violate Inv_RefDerivedFromParent. Client re-pushes, which re-hydrates
  + re-runs receive-pack against the advanced pointer — the only safe
  retry, which git already performs. Spec §Push step 7: 'GOTO 3 (retry)
  or respond non-ff' — both arms safe; we take non-ff.
- No kind:30618 emission. That is derived after CAS — finalize_push
  calls Sami's build_ref_state_event over m_after.refs / m_after.head
  on Ok. Spec §Implementation Correspondence: 'kind:30618 is derived
  after CAS, never the commit.'
- No advisory lock. Spec §Push 'no advisory lock in v1' — writer
  serialization is the CAS. A mutex would hide the contention Inv_NoFork
  proves safe.

## Tests

10 unit tests pin digest_from_key (manifest/<...> prefix invariant),
compose_after (Inv_Closed coverage, sort, dedupe, refs-only-no-new-pack,
first-push, parent-is-digest-not-key), validate invocation (unsafe
refname + first-push-empty-HEAD both rejected pre-CAS). 244 relay tests
green; clippy --tests -D warnings clean.

The integration into finalize_push lands separately — Eva owns the
AppState::git_store wiring + main.rs startup probe gate. This module
is callable today: cas_publish(&store, repo_path, owner, repo,
&refs_before) -> Result<CasSuccess, CasError>.

Refs:
- docs/git-on-object-storage.md §Push step 2-7, §Implementation
  Correspondence, §Mechanized Verification (Inv_NoFork,
  Inv_RefEffectApplied, Inv_RefDerivedFromParent, Inv_Closed).

Also makes transport::harden_git_env pub(crate) for reuse by
cas_publish's two subprocess sites (for-each-ref, pack-objects).

Co-authored-by: Tyler Longwell <tyler@block.xyz>
wpfleger96 added a commit that referenced this pull request May 22, 2026
…iew findings

The original implementation created a second parallel Tauri command
(discover_all_acp_providers) alongside the existing one to avoid
changing the return type. This produced two commands, two hooks, two
query keys, and two raw type converters. Consolidates into a single
command returning the full catalog, with a useAvailableAcpProviders
hook that type-narrows for callers needing non-null command/binaryPath.

Also fixes: pipe deadlock in install command (#1), UTF-8 truncation
panic (#2/#4), adds install concurrency guard (#11), exact provider ID
match (#15), error display stdout fallback (#5), success banner
suppression when already available (#12), misleading re-run text (#13),
IIFE refactor in PersonaDialog (#14), hidden internal query lift (#7),
configurable e2e mocks (#9), shared raw type exports (#8), and
classify_provider unit tests (#10).
tlongwell-block added a commit that referenced this pull request May 22, 2026
Adds crates/sprout-relay/src/api/git/cas_publish.rs — the pure async
function that turns a post-receive-pack workspace into a durable manifest
CAS. Composes Dawn's GitStore primitives (put_pack, put_manifest,
get_pointer, put_pointer) and Dawn's Manifest schema (canonical_bytes,
validate, Inv_Closed at compose time) into the spec's step 2-7 sequence:

  read pointer (e, d_before)            §step 3
  fetch + verify m_before                §step 3 + A1 detectability
  snapshot refs + symref-HEAD from disk (HEAD inherits parent on detach)
  pack new objects via pack-objects --revs    §step 1-2
  put_pack(bytes) -> packs/<sha256>          §step 2 (A1)
  compose m_after (parent packs + new pack, parent = digest only) §step 5
  m_after.validate()                          (Sami/Max/Perci #2-#4)
  put_manifest(canonical_bytes) -> manifests/<sha256>  §step 6
  put_pointer(IfMatch(e) | IfNoneMatchStar)  §step 7 (CAS)
    Won  -> CasSuccess { manifest, manifest_key }
    Lost -> CasError::Conflict { winner_manifest, winner_manifest_key }
            (→ HTTP 409, with winner for disk reconcile)

The function returns *before* a Response is constructed — it is called
from finalize_push, which is the unique site that builds a push 2xx, so
the structural seam still enforces Theorem 1 (success-after-CAS).

## Review fixes folded in

Sami's review (#1#6) + Perci's #1 + Max's pre-CAS-validation blocker
are all addressed in this commit:

- **parent = bare 64-hex digest, not full key** (Perci #1, Max). Pointer
  body is `<digest>`; `Manifest.parent` stores the same digest, matching
  `Inv_RefDerivedFromParent` literally. `read_parent` strips the
  `manifests/` prefix before assigning. Dawn's new `MalformedParent`
  validator catches any drift at the write seam.

- **Pre-CAS validation** (Sami #2, Max). `m_after.validate()?` runs
  between `compose_after` and `put_manifest`. Unsafe refnames, malformed
  oids, empty HEAD — all surface as `CasError::ManifestInvalid(...)`
  (4xx-class) before any S3 write, *not* as "valid CAS, un-clone-able
  output." Typed variant (not reused `ManifestReadFailed`) so logs /
  status mapping distinguish "client input rejected" from "stored parent
  failed A1" (Max + Dawn).

- **Detached-HEAD fallback** (Sami #3). `snapshot_workspace_state`
  returns empty `head` on detached HEAD; `cas_publish` falls back to
  `parent.head` if non-empty. `validate()` rejects the first-push-+-
  detached case (Sami #4 — no parent to inherit from, manifest is
  un-clone-able).

- **Conflict carries the winner** (Sami #5 + Dawn). `Conflict {
  winner_manifest, winner_manifest_key }` lets `finalize_push` invoke
  Eva's `reconcile_to_manifest` mechanically from the error arm, without
  a second pointer GET in the caller. `warn!()` at the `LostRace` site
  logs (pointer, expected etag, attempted manifest) for debugging
  concurrent-push patterns. Boxed for `clippy::result_large_err`.

- **Empty-pack comment** (Sami #6). Clarified `capture_pack` returns
  `None` in both the delete-all (`refs_after.is_empty()`) and refs-only
  (`pack-objects` empty stdout) cases.

- **`pointer_key` consolidated** in `manifest.rs` (Sami #1, Dawn,
  Max — Sami's "single source of truth" argument). `cas_publish`
  imports it; the duplicate definition is gone.

- **`validate-invocation` test added** in `cas_publish.rs` (Sami's
  recommendation). Pins that a future refactor dropping the `validate?`
  call between `compose_after` and `put_manifest` is caught by unit
  test, not by every subsequent un-clone-able read.

## What this deliberately does NOT do (each with citation)

- No retry on LostRace. Per Sami's TLA-action guidance: the receive-pack
  output is derived against a now-superseded parent; reusing it would
  violate Inv_RefDerivedFromParent. Client re-pushes, which re-hydrates
  + re-runs receive-pack against the advanced pointer — the only safe
  retry, which git already performs. Spec §Push step 7: 'GOTO 3 (retry)
  or respond non-ff' — both arms safe; we take non-ff.
- No kind:30618 emission. That is derived after CAS — finalize_push
  calls Sami's build_ref_state_event over m_after.refs / m_after.head
  on Ok. Spec §Implementation Correspondence: 'kind:30618 is derived
  after CAS, never the commit.'
- No advisory lock. Spec §Push 'no advisory lock in v1' — writer
  serialization is the CAS. A mutex would hide the contention Inv_NoFork
  proves safe.

## Tests

10 unit tests pin digest_from_key (manifest/<...> prefix invariant),
compose_after (Inv_Closed coverage, sort, dedupe, refs-only-no-new-pack,
first-push, parent-is-digest-not-key), validate invocation (unsafe
refname + first-push-empty-HEAD both rejected pre-CAS). 244 relay tests
green; clippy --tests -D warnings clean.

The integration into finalize_push lands separately — Eva owns the
AppState::git_store wiring + main.rs startup probe gate. This module
is callable today: cas_publish(&store, repo_path, owner, repo,
&refs_before) -> Result<CasSuccess, CasError>.

Refs:
- docs/git-on-object-storage.md §Push step 2-7, §Implementation
  Correspondence, §Mechanized Verification (Inv_NoFork,
  Inv_RefEffectApplied, Inv_RefDerivedFromParent, Inv_Closed).

Also makes transport::harden_git_env pub(crate) for reuse by
cas_publish's two subprocess sites (for-each-ref, pack-objects).

Co-authored-by: Tyler Longwell <tyler@block.xyz>

Co-authored-by: Quinn <quinn@users.noreply.sprout>
Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
wpfleger96 added a commit that referenced this pull request May 22, 2026
…iew findings

The original implementation created a second parallel Tauri command
(discover_all_acp_providers) alongside the existing one to avoid
changing the return type. This produced two commands, two hooks, two
query keys, and two raw type converters. Consolidates into a single
command returning the full catalog, with a useAvailableAcpProviders
hook that type-narrows for callers needing non-null command/binaryPath.

Also fixes: pipe deadlock in install command (#1), UTF-8 truncation
panic (#2/#4), adds install concurrency guard (#11), exact provider ID
match (#15), error display stdout fallback (#5), success banner
suppression when already available (#12), misleading re-run text (#13),
IIFE refactor in PersonaDialog (#14), hidden internal query lift (#7),
configurable e2e mocks (#9), shared raw type exports (#8), and
classify_provider unit tests (#10).
wpfleger96 added a commit that referenced this pull request May 22, 2026
…iew findings

The original implementation created a second parallel Tauri command
(discover_all_acp_providers) alongside the existing one to avoid
changing the return type. This produced two commands, two hooks, two
query keys, and two raw type converters. Consolidates into a single
command returning the full catalog, with a useAvailableAcpProviders
hook that type-narrows for callers needing non-null command/binaryPath.

Also fixes: pipe deadlock in install command (#1), UTF-8 truncation
panic (#2/#4), adds install concurrency guard (#11), exact provider ID
match (#15), error display stdout fallback (#5), success banner
suppression when already available (#12), misleading re-run text (#13),
IIFE refactor in PersonaDialog (#14), hidden internal query lift (#7),
configurable e2e mocks (#9), shared raw type exports (#8), and
classify_provider unit tests (#10).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant