REVIEW CHANGES PENDING: specialist discovery foundation (seat specialty + read-only buyer directory + discover_sellers) - #986
Open
maxy-player wants to merge 9 commits into
Conversation
added 8 commits
September 8, 2026 15:58
A buyer that has never met a seat has no way to read what that seat is FOR. Adds one bounded UTF-8 field, `[seat] specialty`, carried on the existing announcement and read back by `parse_heartbeat`. It lands in the DISPLAY-ONLY half of `SeatCapability`, beside `hardware` and `harness_variant`, and that placement is the design rather than a convenience. The provenance rule there is *filterable <=> machine-sourced*: a buyer commits sats at award, so only a field the seat MEASURED may gate one. Nothing the daemon can run measures whether an operator is good at Rust, so the field is absent from `filterable_tags` and therefore from every kind-3402 claim. That is the structural form of "no mandatory string-match claim gate": there is no path from this text to anything an award predicate consults. - Bounded at 1024 bytes by `bounded_specialty`, called by the emitter AND the reader, so a reader can never render more than a seat could publish. - Over-long is TRUNCATED on a char boundary, never refused. Refusing at config would brick a boot over cosmetic text; refusing at parse would make an over-talkative seat invisible. The field degrades, the seat survives. - Absent/all-whitespace is UNSTATED, never "generalist". Legacy beats parse unchanged and stay discoverable; old config still loads. - Bounded at the ONE config-to-wire seam (`Advertisement::capability`), so no publish path can carry an unbounded value. - Never auto-filled from memory, client lists or files: a beat is public. Tests: config -> relay-confirmed signed beat carries the tag (e2e against the PGateRelay fixture); it is unreachable from `filterable_tags` and absent from a claim built from the same capability; multi-byte truncation keeps valid UTF-8 and keeps the beat parseable; blank/absent/legacy all read as unstated; a foreign beat's padded or oversized value is normalised by the reader. Two schema-guard tag-count assertions updated deliberately (6->7 display+ filterable, 14->15 beat tags). Scope 1 of 3 of the specialist-discovery foundation order; discovery read path and docs follow.
Core half of scope 2. `crate::discovery` reads kind-30340 announcements and reduces them to the seats a buyer could target, with the specialty text each one declared. It publishes nothing. Shape: a PURE reducer plus a thin transport. `reduce_directory` holds every rule and touches no relay, so all of it is exercised offline against drafts built by the production `heartbeat` emitters — which is why these tests need no live relay and no sats. `fetch_directory_async` is the only part with a socket and is the only part behind a feature gate. The rules, and the order they run in: - Parse first; anything `parse_heartbeat` refuses is counted, never a row. A junk event squatting the kind must not become a seat someone might hire. - Resolve by (pubkey, d) via `HeartbeatKey`, NEVER by event id: kind-30340 is addressable and superseded in place, so an id-keyed reduce lists one seat twice at two rates. Newest `created_at` wins. - THEN judge the winner: future-dated (past a 300 s skew tolerance), stale (past 900 s = the shipped 300 s cadence x 3 missed intervals), retracted. Resolve-before-filter is load-bearing — filtering first would drop a seat's newer `accepting=n` on its own merits and leave its older `accepting=y` standing as the survivor, i.e. the seat would stay advertised by the very event that retracted it. A test asserts both directions. Three outcomes, never two. `fetch_events` resolves `Ok(empty)` on TIMEOUT, so emptiness alone proves nothing: `Err(DiscoveryError)` is a failed read, `read_confirmed == false` is a read the relay never answered, and only `read_confirmed == true` makes an empty list a statement about the market. Absence is concluded from a SECOND fetch taken after the shared EOSE probe's answer, the discipline `award_presence_async` already applies (MakePrisms#291/MakePrisms#322). Single-relay `fetch_events`, not the pool, so a refused REQ surfaces as an error instead of an empty market. Unstated admission renders as "unstated", never "closed" — calling it closed would tell a buyer that every seat predating the S4.2 tags refuses it. Rows come back in pubkey order. Freshness-descending would ship a ranking policy this slice deliberately excludes, and any order the function picks is the order a caller reads first. Tests: 9, green on --all-features AND on a no-feature build (the reducer is genuinely offline). Includes a source-level check that the module references no publish/award/payment symbol — the property is "this code cannot spend", which a runtime test can only show for one path. Scope 2 of 3, part a; MCP tool + daemon route follow.
A fifth MCP tool and its daemon RPC, both reads. `discover_sellers` routes over the buyer socket for one reason only — the daemon holds the home's identity and the relay read is authenticated as the buyer — and reaches no money: no wallet, no money_lock, no reservation ledger, no published event. Bounds are refused out of range rather than clamped, at the RPC boundary before a socket opens. A silently-clamped 60s budget hands the caller a 10s empty answer it then reads as sixty seconds of evidence, which is the exact failure the read_confirmed flag exists to prevent. The timeout is now a budget for the WHOLE read rather than per leg. Connect, fetch, liveness probe and recheck draw down one deadline, because four legs of eight seconds is a thirty-two-second tool and the MCP client's window is fifteen. Running out mid-read yields an unverified directory — an unanswered read, which is what it is — never an empty market and never an error. There is no query, keyword or specialty predicate on either surface. On an MCP tool a "filter" is worse than a matcher: the caller never sees what it removed. Two tests pin the absence so it cannot be added by accident. tools_list_is_slimmed_to_the_trade_loop widened deliberately, 4 tools to 5.
The protocol's capability section counted its own tags in prose ("five tags",
"the last five", "budgets for four will be one short"), so adding a sixth
without recounting would leave the document contradicting itself in the exact
place a reader goes to learn how many there are. Recounted at all four sites.
The display class now names three tags at every one of its enumerations —
§4.5, §4.5.2, the §6.1.2 requestability rule, and the §6.2 claim rule. A
reader who filters on specialty because one list forgot to mention it would be
deciding money on a sentence an operator typed, which is the single thing the
class exists to prevent.
§4.4 gains the flow and the two traps around it. An empty directory is not an
empty market: a subscription that ends on timeout yields the same zero rows as
a market with nobody in it, so read success is decided on the connection and
the two are reported as different answers. And discovery ends at a pubkey — it
selects nobody, pays nothing, and hands the choice back to the buyer, who then
uses the unchanged targeted path.
Both quickstarts say the same thing from their own side: the buyer's says the
rows are not a match, a ranking or a credential and to read read_confirmed
before trusting an empty list; the seller's says a specialty admits no job and
wins no award, and that leaving it out hides nothing.
docs/protocol-v1.md · docs/BUYER-QUICKSTART.md · docs/SELLER-QUICKSTART.md · docs/README.md
The order asks for the discovery-to-target example, and a rendered row is not that: the flow only holds if the pubkey a row carries is a value the EXISTING targeted-post parameter accepts. So the test joins the two real ends — reduce_directory's row into OfferDraft::new, through to_event_draft and parse_offer — and asserts the parsed offer is targeted, addresses that seat, and refuses to match another. No relay, no post, no payment. It guards a quiet class of breakage no rendering test can see: a row field that looks right and is not a valid target — padded, truncated, npub-encoded, upper-cased. Any of those would leave discovery ending one string-conversion short of the flow it exists to start. It also pins the negative half: the specialty text that informed the choice is absent from the offer's tags. Discovery informs the CHOICE; it never becomes a term of the deal.
The one line I added to the dispatch was the one fmt hunk I introduced (mcp.rs had 8 at base, 9 with mine). Block form, so the file is back to exactly its base hunk count and none of the dirt in this tree is mine.
Advisor verdict specialist-discovery-aad7b2a.md, findings F1 and F2. Both were real and both were mine. F1. read_confirmed was set unconditionally at the tail of the reducer, so any read that returned at least one event was certified — including one that took a single beat and then timed out before EOSE. If that beat was stale the output was an EMPTY seller list marked confirmed: the precise lie the flag exists to prevent, told in the one shape that looks most like a fact. The empty path was no better; it leaned on a liveness probe that is a different subscription with a different filter, and a probe's EOSE is evidence about the probe. So completion is no longer inferred. The read opens its own subscription with a known id, brings the notification receiver up BEFORE the REQ (an EOSE for stored events can land immediately), and only that id's EOSE sets ConfirmedByEose. A spent deadline, a dropped socket or an ended stream leaves the read Unconfirmed with its partial rows kept and honestly flagged — rows in hand are useful; pretending they are the whole market is not. A CLOSED naming our subscription is a refusal with a reason and surfaces as an error, never as "no sellers". Cleanup unsubscribes on every path, including the timeout. fetch_events could not do this: it ends on either an EOSE or a spent timeout and returns the same Ok(events) for both, which is why no caller of it can honestly certify completion. F2. (author, created_at) is not a total order. Two signed same-address beats CAN share a timestamp, and if they disagree about `accepting` then whichever the reducer kept decided whether the seat was live or retracted — by iteration luck. The signed event id now crosses the transport-to-reducer seam and breaks the tie by lowest lexical id, per NIP-01's retention rule for addressable events. Rows stay keyed by (pubkey, d); the id is a tie-breaker, not a key. Three tests: both input permutations of a live-versus-retraction tie in both id assignments, that a newer timestamp still outranks a lower id, and that an unconfirmed read reports itself honestly however many rows it holds.
Advisor finding F3: the green gate did not exercise F1 at all. The transport tests compared CONSTRUCTORS and searched SOURCE STRINGS, the fixture began at a hand-assembled unsigned draft, and the target handoff went through OfferDraft and parse_offer rather than the daemon's own post mapping. Every one of those passes while the relay leg is wrong — which is what happened. So each of the eleven tests here drives the real fetch_directory_async against a scripted relay and asserts on what came back. The relay is the crudest thing that can script an ENDING, because an ending is the whole contract and a conforming relay always answers: events+EOSE, EOSE alone, silence with the socket up, one event then silence, one event then a dropped socket, and CLOSED naming our subscription. They must land on confirmed rows, a confirmed empty market, an unconfirmed empty read, an unconfirmed KEPT row, an unconfirmed read, and an error carrying the relay's own reason. The teeth were proven by breaking the fix: with the completion flag seeded optimistic, four of these tests fail. They were red before they were green. The announcements are signed by real keys and built from a real [seat] config through Advertisement::capability -> heartbeat_for_state -> event draft -> signature, the chain the seller daemon publishes through. One test serves an event with a byte of its signature flipped and asserts it never becomes a row while an honest beat in the same response still does. The fixture records every inbound frame, so "discovery published nothing" is read off the wire: no test may see an EVENT frame. No wallet file may appear either — open_wallet_async creates it on the first call, so its absence is machine evidence rather than a promise. Cleanup is asserted on the property, not on a race: unsubscribe hands the CLOSE to the SDK's writer while disconnect tears the socket down, so which arrives first is timing. Insisting on the CLOSE made the test intermittent (observed). Either ends the subscription; what would be a defect is neither, or a CLOSE naming somebody else's subscription, and both are asserted. map_post_job_params is lifted out of the post_job RPC handler so the daemon's own parameter boundary can be exercised without a daemon, a relay or a wallet. Same mapping, same error strings, now callable — and the discovered pubkey is asserted through it rather than around it, since a value OfferDraft accepts and the RPC rejects is a value no user can post with.
|
Someone is attempting to deploy a commit to the MakePrisms Team on Vercel. A member of the Team first needs to authorize it. |
Advisor verdict specialist-discovery-57bd843-r2.md. The completion fix passed; what it lost was the hard error the round-1 instruction told me to preserve. F1a. AuthenticationFailed is emitted on the RELAY's notification channel and is never forwarded to the pool's (relay/inner.rs:417-419), and the read consumed only pool notifications. A relay that challenges, receives our signed AUTH and rejects it with a negative OK owes the subscription nothing further: no EOSE, no CLOSED. So the read waited out its deadline and returned an unanswered directory — turning "this relay refused your identity", which has a reason and a fix, into "nobody answered", which has neither. The previous fetch_events path handled this and my rewrite dropped it. The receiver is now this relay's own, opened BEFORE connect() rather than before the REQ, because the challenge and the negative OK both land during connect and a receiver opened afterwards would miss them — the same ordering trap the EOSE receiver already avoids, one leg earlier. F1b. The REQ went out through Client::subscribe_with_id, which returns Result<Output<()>>; the pool folds per-relay send failures into output.failed and returns Ok(output) even when NOTHING succeeded (pool/mod.rs:955-973). The comment promising that immediate send failures become hard errors was therefore untrue. It now subscribes through the single relay, whose result is Result<(), Error> — the failure of this relay's REQ, propagated. One relay is the whole market here, so its disposition is the read's. Request-scoped EOSE, the CLOSED reason, partial-row retention and cleanup on every exit are unchanged. F2 and F3 are untouched. Two scripted negatives, both red-proved: removing the AuthenticationFailed arm fails a_rejected_authentication_is_an_error_not_an_unanswered_read and nothing else. The fixture gained a NIP-42 script that challenges, refuses the client's signed AUTH with a negative OK, and then stays deliberately mute — the exact shape that produced the downgrade. The failure-result test drives a port with nothing listening and pins the one outcome that must be impossible: a subscription that reached no relay may never yield a confirmed directory. Also corrected here because the verdict named them and they describe this code: comments still promising a liveness probe and a recheck, the whole-call wall-clock claim the timeout does not make (connect and cleanup sit outside it), and the fixture's claim to stop its accept task on drop when it has no Drop impl. An unused fixture helper went with them.
Contributor
Author
Residual F1 repaired —
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
Buyers cannot currently find a seat they have never met.
parse_heartbeathas no production consumer, so the kind-30340 announcements sellers already publish are read by nobody. This adds the smallest foundation that closes that gap:specialtyon the seat announcement.[seat] specialtyinconfig.toml→["specialty", text]on the kind-30340 beat. Bounded at 1024 bytes and truncated on a character boundary rather than refused — an over-long line costs an operator the tail of a sentence, never their place in the directory.crates/maxplayer-core/src/discovery.rs. Latest beat per(pubkey, d), retraction honoured, stale and future-dated dropped and counted, unparseable counted.discover_sellers, a fifth MCP tool plus its buyer-daemon RPC. Both reads: no wallet, no mint, no published event.Docs:
protocol-v1.md§4.4 (the flow and its two traps) and §4.5 (the display class, recounted), plus both quickstarts.Two properties worth a reviewer's attention
specialtyis a discovery field, not a matching field. It is display-only by protocol rule: emitted fromdisplay_tags(), neverfilterable_tags(), so it is structurally unable to reach a kind-3402 claim — not a rule a future edit can forget, a function it would have to move. There is deliberately no keyword, query or specialty predicate on either the MCP tool or the RPC; two tests pin the absence. A buyer reads the text and decides for itself, then uses the unchanged targeted-post path. Claim and award selection are untouched:claim_meets_capability_requeststill reads onlyharness_family/harness_model/capabilities.Relay failure is not an empty market.
fetch_eventsresolvesOk(empty)on timeout, so zero rows from a relay that never answered is byte-identical to zero rows from an empty market. The read therefore decides completion on the transport, never on the row count:read_confirmedis set only by the directory subscription's ownEOSE. A spent deadline, a dropped socket or an ended stream returns the rows it has, honestly flagged unconfirmed. ACLOSEDnaming our subscription is an error carrying the relay's reason.Review history — read this before reviewing
An independent review of
aad7b2areturned DENY with three blocking findings. All three were real. The repairs are appended, never rebased:3c44e9cread_confirmedwas set unconditionally in the reducer. A read that took one beat and then timed out beforeEOSEwas certified — and if that beat was stale, the output was an empty seller list marked confirmed. The empty path leaned on a liveness probe, which is a different subscription with a different filter.created_atresolution was not deterministic3c44e9c(author, created_at)is not a total order. Two signed same-address beats sharing a timestamp and disagreeing aboutacceptingresolved by iteration luck — a seat live or retracted by chance. The signed event id now crosses the transport seam; ties break by lowest lexical id per NIP-01. Rows stay keyed by(pubkey, d).57bd843F3's answer is
crates/maxplayer-core/tests/discovery_relay_behavior.rs— eleven tests driving the realfetch_directory_asyncagainst a scripted NIP-01 relay that scripts the ending: events+EOSE,EOSEalone, silence with the socket up, one event then silence, one event then a dropped socket, andCLOSED. Announcements are signed with real keys and built from a real[seat]config through the productionAdvertisement::capability→heartbeat_for_statechain. One test flips a signature byte and asserts it never becomes a row while an honest beat in the same response does. The fixture records every inbound frame, so "discovery published nothing" is read off the wire — no test may see anEVENT— and no wallet file may appear.The teeth were proven by breaking the fix. With the completion flag seeded optimistic, 4 of the 11 fail. They were red before they were green.
Verification
Base:
d55ceafe287b97481eefbd304574ad1db6e93d3a.EXIT=0— 19 targets, allok: 1,640 passed, 0 failed, 3 ignored (core lib 1432/0/2; the new relay-behavior target 11/0/0).Honest limits
credential_proxy::tests::a_declared_over_cap_body_is_refused_before_the_upstream_sees_itis a pre-existing intermittent failure, reproduced at base without these commits — a write-vs-reset race in the test itself (the proxy RSTs on the declared content-length, soreqwestsurfacesBodyWrite/ConnectionResetinstead of a 413). It did not fire in the run above. Out of scope here; worth its own issue.discovery.rsand both new test files are 0. One pre-existing hunk inbuyer/mod.rswas removed as a side effect of liftingmap_post_job_params.