# Fix: Self-Skill Startup Race (step 14 false-negative)

## Symptom

`didactyl.service` / `simon.service` intermittently fail startup at step 14:

```
startup checklist [14] Subscribe self-skill cache: failed (no skill events found after EOSE)
```

…even though the required events (kind 31123/31124 skills and kind 10123 adoption list)
demonstrably exist on `relay.laantungir.net` (verified with `nak`, returned in well under
a second). On other restarts the identical setup succeeds: `ok (skills=3 adoptions=1)`.
The failure is timing-dependent.

## How startup relay polling works (mechanics)

- Each startup step opens its **own** subscription (its own `REQ` + filter). Step 14's
  filter is built in [`nostr_handler_subscribe_self_skills()`](src/nostr_handler.c:2800):
  `{"kinds":[31123,31124,10123],"authors":[<agent_pubkey>],"limit":300}`.
- That subscription is sent to **all** configured relays in parallel inside
  [`nostr_relay_pool_subscribe()`](nostr_core_lib/nostr_core/core_relay_pool.c:967).
- This is the **async/streaming** path: events arrive over time via
  [`on_self_skill_event()`](src/nostr_handler.c:2478) which upserts into the shared cache
  `g_self_skill_events`, and an EOSE-complete callback
  ([`on_self_skill_eose`](src/nostr_handler.c:2066)) flips `g_self_skill_eose_received`.
- Step 14 then immediately runs
  [`nostr_handler_validate_self_skill_cache()`](src/nostr_handler.c:3089) against whatever
  is in the cache at that instant.

## Root cause

The async EOSE-complete signal is decoupled from "did the relay that actually has the
events get queried and deliver yet." Two independent contributing factors:

1. **Relay connection timing.** The self-skill subscription is registered shortly after
   startup. If `relay.laantungir.net` is not yet fully connected when the subscription's
   completion is computed, the empty relays (`damus`, `primal`) EOSE almost instantly and
   the per-relay **timeout fallback**
   ([`core_relay_pool.c:2128`](nostr_core_lib/nostr_core/core_relay_pool.c:2128)) marks the
   remaining relay "done" after `relay_timeout_seconds` (=12) without ever receiving its
   events. EOSE-complete fires with an empty cache → validation fails.
2. **Validation fires the instant completion is signaled**, with no re-check, so any race
   where laantungir's events land microseconds later is lost.

This matches the `<1s` `nak` observation: when laantungir is connected and queried, it
answers almost instantly; the failures are the cycles where it wasn't
connected/queried/processed before the async completion fired.

### Why FIRST → FULL_SET alone does NOT fix it

`maybe_complete_subscription_eose()` waits for **all** relays in both modes; the only
difference is the payload passed to the callback. The cache is filled by the EVENT
callback, not the EOSE payload. Changing the mode changes neither connection timing nor
when events are available.

## Fix: use the synchronous query for the startup self-skill load

There is already a correct, synchronous "collect all events until EOSE/timeout, then
return them" primitive used elsewhere at startup (e.g. kind-10002 discovery):

- [`nostr_handler_query_json()`](src/nostr_handler.c:3395) →
  [`nostr_relay_pool_query_sync()`](nostr_core_lib/nostr_core/core_relay_pool.c:1340).
- A self-skill-specific wrapper already exists and reuses it:
  [`nostr_handler_refresh_self_skill_cache_from_relays()`](src/nostr_handler.c:3751),
  which queries kinds 31123/31124/10123 for the agent author (and also pulls adopted
  skills), populating the cache synchronously.

The synchronous query is the architecturally correct tool for "give me the complete
current set before proceeding," which is exactly what step 14 needs. It also drives the
poll/receive loop itself
([`core_relay_pool.c:1405`](nostr_core_lib/nostr_core/core_relay_pool.c:1405)) until all
connected relays EOSE or the timeout elapses.

### Important caveat to handle: connection readiness

`nostr_relay_pool_query_sync()` only queries relays that are **connected at call time**
(`if (connected_count == 0) return NULL;` at
[`core_relay_pool.c:1392`](nostr_core_lib/nostr_core/core_relay_pool.c:1392)). If
`relay.laantungir.net` is still mid-connect when step 14 runs, a sync query could still
miss it. Therefore the fix must ensure the relay holding the events is connected (or wait
briefly for it) before/within the query.

### Plan

1. **Replace the step-14 async wait+validate with a synchronous load.** In
   [`src/main.c`](src/main.c) step 14 (~line 1518), instead of relying solely on
   `nostr_handler_wait_for_self_skill_eose()` + an immediate
   `nostr_handler_validate_self_skill_cache()`:
   - Keep registering the live subscription (so ongoing skill updates still stream in via
     `on_self_skill_event`) — this is still wanted for runtime.
   - For the startup *gate*, call
     [`nostr_handler_refresh_self_skill_cache_from_relays(timeout_ms)`](src/nostr_handler.c:3751)
     with a sensible timeout (e.g. 8000–10000 ms). This performs the synchronous,
     connection-aware query and fills `g_self_skill_events`.
   - Then run `nostr_handler_validate_self_skill_cache(&skills, &adoptions)`.

2. **Wait for the self relay (laantungir) to be connected before the sync query.** Reuse
   the existing relay-readiness helper pattern (`wait_for_connected_relays`) — but ensure
   we wait specifically until the relay list discovered from kind 10002 (which includes
   `relay.laantungir.net`) is connected, not just "any relay." If we already have a
   connected-count wait, extend/bound it so the relay that owns the events has a fair
   chance to connect before the sync query runs.

3. **Bounded retry around the sync query.** If the first sync query returns an empty cache,
   retry the sync query a small, bounded number of times (e.g. up to ~5–8s total),
   pumping `nostr_handler_poll()` between attempts to let connections settle. Only fail
   step 14 after the bounded retries still yield an empty cache. This converts a transient
   connection-timing miss into a brief wait instead of a hard failure.

4. **Keep the async subscription for runtime updates.** Do not remove the live
   subscription; it's still responsible for picking up skill changes after startup. Only
   the *startup gate* switches to the synchronous load for determinism.

### Why this is the right layer

- It uses the library's intended synchronous collect-all primitive rather than racing a
  streaming subscription against an immediate validation.
- It is connection-aware and bounded, so a slow/late relay connect becomes a short wait,
  not a false "no skill events" failure.
- It does not require modifying `nostr_core_lib` (though if desired, a follow-up could add
  a `query_sync` variant that waits for a named relay to connect; not required here).

## Interaction with the failure-DM throttle (already shipped)

- On success, the READY path clears the throttle marker
  ([`src/main.c`](src/main.c) `startup_failure_marker_clear`). No change needed.
- After this fix, step-14 failures should be genuinely rare (real outage / events truly
  absent), and the (single, throttled) failure DM will carry relay states + needed event
  kinds for accurate diagnosis.

## Verification

1. Build (`make`) clean, then `build_static.sh`.
2. Deploy to VM410; restart both services many times in a row and confirm step 14 reaches
   `ok (skills=N adoptions=M)` **consistently** (no intermittent false negatives).
3. Inspect logs to confirm the synchronous query path is used and that, on a cold cycle,
   it waits for `relay.laantungir.net` to connect and then returns the events.
4. Negative test: configure only empty relays (no skill events anywhere) and confirm step
   14 still fails after the bounded retries (no false positive), sending exactly one
   throttled failure DM.
5. Confirm a normal successful startup still sends the startup DM and clears the marker.

## Out of scope (possible follow-ups)

- Adding a `nostr_core_lib` `query_sync` variant that explicitly waits for specific
  relays to connect before issuing the REQ.
- Softening step 14 into a degraded-start mode.
