Skip to content

Initial release — Sprout Nostr relay with enterprise extensions#2

Merged
tlongwell-block merged 5 commits into
mainfrom
oss
Mar 9, 2026
Merged

Initial release — Sprout Nostr relay with enterprise extensions#2
tlongwell-block merged 5 commits into
mainfrom
oss

Conversation

@tlongwell-block
Copy link
Copy Markdown
Collaborator

All-Rust Nostr relay with structured channels, YAML-as-code workflows, MCP agent interface, tamper-evident audit log, and permission-aware full-text search.

13 crates: sprout-core, sprout-relay, sprout-db, sprout-auth, sprout-pubsub, sprout-search, sprout-audit, sprout-mcp, sprout-workflow, sprout-proxy, sprout-huddle, sprout-admin, sprout-test-client.

394 unit tests, 42 E2E integration tests, 16 MCP tools. Apache-2.0 licensed.

All-Rust Nostr relay with structured channels, YAML-as-code workflows,
MCP agent interface, tamper-evident audit log, and permission-aware
full-text search.

13 crates: sprout-core, sprout-relay, sprout-db, sprout-auth, sprout-pubsub,
sprout-search, sprout-audit, sprout-mcp, sprout-workflow, sprout-proxy,
sprout-huddle, sprout-admin, sprout-test-client.

394 unit tests, 42 E2E integration tests, 16 MCP tools.
Apache-2.0 licensed.
Comment thread .github/workflows/ci.yml Fixed
Comment thread .github/workflows/ci.yml Fixed
…ing licenses

- Remove deprecated 'vulnerability', 'unmaintained', 'yanked' fields from [advisories]
- Remove deprecated 'unlicensed' field from [licenses]
- Use structured ignore entries with reasons for advisory ignores
- Add CC0-1.0, CDLA-Permissive-2.0, MITNFA to license allow list
  (Bitcoin/Nostr ecosystem permissive licenses)
- Set wildcards = 'allow' (workspace crates use * for inter-crate deps)
- Add [licenses.private] ignore = true for unpublished workspace crates

All four cargo-deny checks pass: advisories ok, bans ok, licenses ok, sources ok
@tlongwell-block tlongwell-block merged commit 058c4b9 into main Mar 9, 2026
4 checks passed
@tlongwell-block tlongwell-block deleted the oss branch March 9, 2026 19:15
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 Apr 11, 2026
- #2: Document display_name reverse-lookup fragility in agents.rs
  with TODO for long-term fix (store internal name on PersonaRecord)
- #3: Add on_message to ResolvedHooks so it's not silently dropped
  during resolution
- #4: Fix misleading comment on ResolvedPack.description (was 'use
  pack name as fallback', now 'not yet wired')

156 crate tests pass, desktop compiles clean.
tlongwell-block added a commit that referenced this pull request May 8, 2026
When any tool call returns an error, append a short reflection prompt
to the result text before it enters history:

  [Reflect] Diagnose the error above before retrying.
  What went wrong? Why? What will you do differently?

This forces the LLM to analyze the failure before blindly repeating
the same failing command — the #2 cause of wasted rounds on benchmarks
after doom loops. Three lines of code, zero new modules.

Inspired by ForgeCode's 'forge-partial-tool-error-reflection.md' template
which requires structured reflection after every tool failure.
tlongwell-block pushed a commit that referenced this pull request May 13, 2026
…alse, idempotent legacy cleanup

Addresses Sami + Quinn reviewer-hat catches on the initial PR commit and a
P3 finding from codex review.

Changes
-------
1. Sidecar MODEL_LICENSE.txt wording: replace 'Used unmodified by Sprout'
   with Quinn's framing that distinguishes sherpa-onnx's PyTorch→ONNX/int8
   conversion (a CC-BY-4.0 §3(a)(1)(b) modification) from Sprout's verbatim
   shipping of that conversion. Same five §3(a)(1) bullets, no internal
   contradiction. Sami concurred on-thread.
2. `STT_NUM_THREADS` promoted to a named const at the top of stt.rs with a
   comment explaining the hold-at-1 rationale and pointing at the planned
   A/B follow-up. Easier to find and flip than a hardcoded literal.
3. `OfflineRecognizerConfig::model_config.debug = false` set explicitly.
   Defaults aren't part of the API contract, and per-VAD-chunk debug
   logging would be expensive in release builds.
4. Best-effort cleanup of the legacy `~/.sprout/models/moonshine-tiny/`
   directory (~70 MB). Reclaims disk space silently on upgrade.

   Codex caught a real edge case in the first sketch: a user who already
   had the new manifest-v2 Parakeet model installed (e.g. from an earlier
   pre-merge build) would short-circuit out of `start_stt_download`'s
   `is_ready` fast path and never trigger the cleanup. The fix factors
   the cleanup into a free `cleanup_legacy_moonshine_dir` helper and
   schedules it as a detached task from `start_stt_download` on every
   call — idempotent (no-op if dir absent), runs once per app launch
   regardless of whether a fresh download happens. Same helper still
   runs from the post-install success path.

Verification: cargo check + fmt + clippy + 66/66 huddle tests + file-size
check (bumped models.rs budget 850→900 with a descriptive-comment update;
net source growth is real and documented in the diff). codex review on
the uncommitted diff: 'No correctness issues were found.'

Notes on the other review points (no code change needed)
-------------------------------------------------------
- Quinn #1 (manifest v1→v2 upgrade): the new code has no path that
  resolves `moonshine-tiny/` anymore (the dir name was renamed, not just
  the version bumped), and `is_ready()` requires both manifest match and
  every expected file present — half-installs already fail closed.
- Quinn #2 (JSON field rename leakage): grep across the whole tree
  confirms `'moonshine'` and `moonshine-tiny` appear only in `models.rs`
  source comments. The `VoiceModelStatus.stt` field is a Tauri-invoke
  IPC type consumed only by HuddleBar.tsx at render time — not persisted
  in settings, electron-store, telemetry, or any other survives-restart
  surface. The rename is purely in-memory.
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 added a commit that referenced this pull request May 17, 2026
…nses auto-upgrade

Two follow-ups from review on #604.

1. Anthropic startup hardening (Max #1)
   `OPENAI_COMPAT_API` was parsed unconditionally, so a stray bad value
   in an Anthropic-only env broke startup. Parse it only inside the
   `Provider::OpenAi` arm of `Config::from_env`. Anthropic gets a
   placeholder `OpenAiApi::ChatCompletions` it never reads. New tests
   pin the parser behavior without touching process env.

2. One-shot chat→responses auto-upgrade (Max #2, Tyler "automatic
   detection/fallthrough")
   When `OPENAI_COMPAT_API=auto` and the provider replies to a Chat
   Completions request with a body that explicitly names `/v1/responses`
   (or the prose "use the Responses API"), latch a process-wide
   sticky-cached upgrade and re-issue the same request on `/v1/responses`.
   Subsequent calls skip the chat attempt entirely. Pinned values
   (`OPENAI_COMPAT_API=chat`|`responses`) never auto-upgrade.

   Signal matcher (`is_responses_required_error`) is intentionally
   narrow — only matches the literal path `/v1/responses` or specific
   prose phrases, so we don't get fooled by unrelated 4xx bodies.

   New `Config.openai_api_auto: bool` records whether the operator
   resolved-by-auto vs. pinned, so we know when to enable the upgrade.

   `Llm` gains an `AtomicBool` for the sticky upgrade, plus three
   small helpers (`effective_openai_api`, `should_try_auto_upgrade`,
   `latch_responses_upgrade`) so the dispatch reads straight through.

   Logged at WARN once per process: `"openai chat-completions endpoint
   reported that this model requires the Responses API; auto-upgrading
   subsequent OpenAI calls to /v1/responses for the rest of this
   process"`, with the provider error body attached.

Tests:
- 4 new unit tests for `is_responses_required_error` covering the
  Databricks GPT-5.5 signal, OpenAI prose phrasing, and explicit
  non-matches for `invalid_api_key`, generic `unsupported_parameter`,
  and empty body.
- 3 new unit tests for `parse_openai_api` covering unset-defaults-to-auto,
  case-insensitive explicit values with whitespace, and rejected garbage.
- New integration test `tests/openai_auto_upgrade.rs` spawns a fake
  provider that 400s on `/chat/completions` with the Databricks signal
  and 200s on `/responses`. Drives sprout-agent through ACP and asserts
  `stopReason=end_turn` plus chat-hit-once / responses-hit-once.

65 tests pass, 0 fail. clippy `-D warnings` clean. cargo fmt clean.
Live smoke against api.openai.com with gpt-5-mini still 3/3 PASS.

Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com>
tlongwell-block added a commit that referenced this pull request May 17, 2026
…nses auto-upgrade

Two follow-ups from review on #604.

1. Anthropic startup hardening (Max #1)
   `OPENAI_COMPAT_API` was parsed unconditionally, so a stray bad value
   in an Anthropic-only env broke startup. Parse it only inside the
   `Provider::OpenAi` arm of `Config::from_env`. Anthropic gets a
   placeholder `OpenAiApi::ChatCompletions` it never reads. New tests
   pin the parser behavior without touching process env.

2. One-shot chat→responses auto-upgrade (Max #2, Tyler "automatic
   detection/fallthrough")
   When `OPENAI_COMPAT_API=auto` and the provider replies to a Chat
   Completions request with a body that explicitly names `/v1/responses`
   (or the prose "use the Responses API"), latch a process-wide
   sticky-cached upgrade and re-issue the same request on `/v1/responses`.
   Subsequent calls skip the chat attempt entirely. Pinned values
   (`OPENAI_COMPAT_API=chat`|`responses`) never auto-upgrade.

   Signal matcher (`is_responses_required_error`) is intentionally
   narrow — only matches the literal path `/v1/responses` or specific
   prose phrases, so we don't get fooled by unrelated 4xx bodies.

   New `Config.openai_api_auto: bool` records whether the operator
   resolved-by-auto vs. pinned, so we know when to enable the upgrade.

   `Llm` gains an `AtomicBool` for the sticky upgrade, plus three
   small helpers (`effective_openai_api`, `should_try_auto_upgrade`,
   `latch_responses_upgrade`) so the dispatch reads straight through.

   Logged at WARN once per process: `"openai chat-completions endpoint
   reported that this model requires the Responses API; auto-upgrading
   subsequent OpenAI calls to /v1/responses for the rest of this
   process"`, with the provider error body attached.

Tests:
- 4 new unit tests for `is_responses_required_error` covering the
  Databricks GPT-5.5 signal, OpenAI prose phrasing, and explicit
  non-matches for `invalid_api_key`, generic `unsupported_parameter`,
  and empty body.
- 3 new unit tests for `parse_openai_api` covering unset-defaults-to-auto,
  case-insensitive explicit values with whitespace, and rejected garbage.
- New integration test `tests/openai_auto_upgrade.rs` spawns a fake
  provider that 400s on `/chat/completions` with the Databricks signal
  and 200s on `/responses`. Drives sprout-agent through ACP and asserts
  `stopReason=end_turn` plus chat-hit-once / responses-hit-once.

65 tests pass, 0 fail. clippy `-D warnings` clean. cargo fmt clean.
Live smoke against api.openai.com with gpt-5-mini still 3/3 PASS.

Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com>
tlongwell-block added a commit that referenced this pull request May 19, 2026
… arithmetic

Desktop smoke e2e was failing on the two click-to-select mention paths.
Root cause: `replacePlainTextRange` placed the post-transaction cursor at

  tr.mapping.map(fromPM + text.length)

The argument to `mapping.map` is a *pre-image* (old-doc) position, but
`fromPM + text.length` can be larger than the old doc's size — for a
3-char input replacing a 0-char range with 7 chars, the arithmetic
yields a position 7 past the original end, which ProseMirror rejects
with "Position N out of range". The thrown error bubbled out of the
mention click handler, the transaction never dispatched, and the
editor stayed at "Hey @ali".

The keyboard-Enter path passed only because the surrounding code-path
(`handleEditorKeyDown`) caught the error implicitly — but in fact it
was hitting the same bug; the test just asserted on a state that the
default Enter keymap also reaches.

Fix: map `toPM` (the right end of the replaced range, a valid old-doc
position) through `tr.mapping.map`. That returns the post-transaction
position right after the inserted text — which is what we want, and
is always valid:

  const cursorPM = tr.mapping.map(toPM);

This still satisfies Perci's robustness concern from the original
review (use the transaction's mapping, not raw arithmetic) but uses
the correct pre-image input.

Regression coverage:
  - All 4 existing mentions.spec.ts tests for the click-and-Enter
    selection paths now pass.
  - Added a new regression test for bug #2: type two lines separated
    by Shift+Enter, type "@ali", click "alice" — assert both lines
    survive *and* there's still exactly one `<br>` in the editor.
    This is the literal scenario from the bug report.

Verified locally:
  - smoke suite: 97/97 pass (was 94 + 2 new mentions tests + 1 new
    regression test).
  - node unit tests: 182/182.
  - typecheck + biome + file-size: clean.

Removed the temporary `console.warn` instrumentation I'd added while
diagnosing.
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 pushed a commit that referenced this pull request May 22, 2026
The architectural split Eva's live e2e caught: 30617 created the on-disk
bare repo but no S3 pointer, while the read path (info_refs/upload_pack)
now hydrates from the pointer. Pointer-absent ⇒ Ok(None) ⇒ 404 by Max's
intentional fail-closed, so announce-then-clone returned 404 for every
freshly-announced repo.

Fix: establish the invariant "repo announced ⟺ pointer exists" by
seeding an empty-manifest pointer at the end of `handle_git_repo_
announcement`. After seed:

- `info_refs` stays strictly fail-closed: pointer-absent means
  never-announced, not "announced but no pushes." Max's blocker
  preserved exactly.
- `live_hydrate_empty_repo` proves clone of an empty hydrated repo works.
- First push CASes the seeded pointer via `IfMatch(etag)`, no
  special-case branch. The `IfNoneMatchStar` arm of `cas_publish`
  becomes dead code for announced repos.

Two new private helpers in `side_effects.rs`:

- `seed_manifest_pointer(state, owner, repo)` — `put_manifest(empty)` +
  `put_pointer(IfNoneMatchStar)`. **Idempotency is constructive, not
  trusted**: a `CasOutcome::LostRace` is treated as success ONLY if the
  existing pointer body matches the empty manifest's digest. Any other
  value (stale from a prior lifecycle, real misconfiguration) surfaces
  as `anyhow!` — Max's guardrail #1.
- `emit_initial_ref_state(state, owner, repo)` — kind:30618 over the
  seeded empty manifest using Sami's `build_ref_state_event`. Pointer is
  the commit; this event is the derived "repo exists, empty" signal —
  Max's guardrail #2. Non-fatal on failure: manifest is truth, 30618 is
  just notification.

Empty manifests across all repos share canonical bytes (deterministic
serialization by construction) ⟹ same digest ⟹ `put_manifest` is
idempotent at the store level. One blob, many pointers. Pinned by
`empty_manifest_validates` test in manifest.rs with byte-stable canonical
bytes — locks the digest so future serde version bumps don't silently
shift it.

Rollback on seed failure: remove the on-disk repo + the name reservation,
same pattern as hook-install failure. A successful seed but failed emit
leaves the pointer in place (correctly — it's the source of truth).

239/239 relay tests (was 238 + 1 new), clippy + fmt clean.
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>
tlongwell-block added a commit that referenced this pull request May 22, 2026
The architectural split Eva's live e2e caught: 30617 created the on-disk
bare repo but no S3 pointer, while the read path (info_refs/upload_pack)
now hydrates from the pointer. Pointer-absent ⇒ Ok(None) ⇒ 404 by Max's
intentional fail-closed, so announce-then-clone returned 404 for every
freshly-announced repo.

Fix: establish the invariant "repo announced ⟺ pointer exists" by
seeding an empty-manifest pointer at the end of `handle_git_repo_
announcement`. After seed:

- `info_refs` stays strictly fail-closed: pointer-absent means
  never-announced, not "announced but no pushes." Max's blocker
  preserved exactly.
- `live_hydrate_empty_repo` proves clone of an empty hydrated repo works.
- First push CASes the seeded pointer via `IfMatch(etag)`, no
  special-case branch. The `IfNoneMatchStar` arm of `cas_publish`
  becomes dead code for announced repos.

Two new private helpers in `side_effects.rs`:

- `seed_manifest_pointer(state, owner, repo)` — `put_manifest(empty)` +
  `put_pointer(IfNoneMatchStar)`. **Idempotency is constructive, not
  trusted**: a `CasOutcome::LostRace` is treated as success ONLY if the
  existing pointer body matches the empty manifest's digest. Any other
  value (stale from a prior lifecycle, real misconfiguration) surfaces
  as `anyhow!` — Max's guardrail #1.
- `emit_initial_ref_state(state, owner, repo)` — kind:30618 over the
  seeded empty manifest using Sami's `build_ref_state_event`. Pointer is
  the commit; this event is the derived "repo exists, empty" signal —
  Max's guardrail #2. Non-fatal on failure: manifest is truth, 30618 is
  just notification.

Empty manifests across all repos share canonical bytes (deterministic
serialization by construction) ⟹ same digest ⟹ `put_manifest` is
idempotent at the store level. One blob, many pointers. Pinned by
`empty_manifest_validates` test in manifest.rs with byte-stable canonical
bytes — locks the digest so future serde version bumps don't silently
shift it.

Rollback on seed failure: remove the on-disk repo + the name reservation,
same pattern as hook-install failure. A successful seed but failed emit
leaves the pointer in place (correctly — it's the source of truth).

239/239 relay tests (was 238 + 1 new), clippy + fmt clean.

Co-authored-by: Dawn <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@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.

3 participants