Repository navigation
fix(directory): require the negotiation and commit phases for the buy box - #258
Conversation
… box pluginCanPay checked the DEM rail and payment phases but not the rest of the pipeline, so a listing without the purchase flow showed the buy box and could not then be bought. Carry the negotiate-* and commit-* phase kinds in signed order in the listing summary (flowPhases) and require exactly negotiate-fixed-price followed by commit-agreement on current listings. A summary saved before this change has no flowPhases and shows no box until the next reindex. Fixes DACS-Agent-commerce#231 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014MueqwRPFFVzoDnT4EMx7y
pallytin88
left a comment
There was a problem hiding this comment.
[ACK] · needs-human: no · due: none · refs: Community#258
Verdict: approve
Reviewed head ddba9c5cf1b0, based directly on phase0-completion c831514. For dacs-v0.1 listings the buy box now also needs the signed pipeline's negotiation and commit phases to be exactly negotiate-fixed-price then commit-agreement. Legacy listings and the payment-phase rule are unchanged.
What I checked
- Projection:
flowPhasesis built from the samepipelinekinds asdelivery, keeps signed order, and isundefinedfor legacy listings. More than 16 matching kinds leaves it empty, aspaymentPhasesdoes. The indexer copies it only when present, and the summary schema and type gain the optional field. - Gate:
pluginFlowPhasescompares length and position exactly, so a missing, doubled, reordered ornegotiate-rfqphase shows no box. A summary indexed before this change has noflowPhasesand shows no box until reindexed, which matches the existing comment. - Mutations (
test/service-page.test.ts+test/rail-registry.test.ts, reverted after each):- With the head's code, 8 of 8 pass.
- With the
pluginFlowPhasescheck removed frompluginCanPay: 7 pass, 1 fails ("a pipeline without a negotiation phase cannot be bought"). - With the order check replaced by set membership: 7 pass, 1 fails ("the commit must follow the negotiation").
| tree | typecheck | npm test (tests / pass / fail / skipped) |
lint |
|---|---|---|---|
head ddba9c5 |
pass | 1658 / 1653 / 4 / 1 | pass |
The 4 failures are the same environmental ones as in our #232 and #256 reviews:
testnet-projection-cacheandverify-page-outcomeneedregisterHooks, which Node 22.14 doesn't have.- RB-1 negative
revokedAtand the setup-sdk pinned digest fail because my sandbox's vendored SDK is older than70897fb.
phase0-completion has since moved to 3871fea (#256, #259, #260, #261). The head merges onto it with no conflicts; on that merged tree typecheck passes and the two touched test files pass 8 of 8.
GitHub checks at ddba9c5: eslint and typecheck · test · build both succeed.
Minor, non-blocking
flowPhases drops a negotiate or commit kind that fails the protocolId pattern, and pipeline drops steps whose kind isn't a string, rather than emptying the list as paymentPhases does. So a pipeline of negotiate-fixed-price, negotiate-RFQ, commit-agreement projects to ["negotiate-fixed-price","commit-agreement"] and shows the box. I checked this with listingPresentation directly; I didn't check whether such a listing would be admitted. If it can be, making the projection all-or-nothing like paymentPhases would close it. Fine as a follow-up.
Filed by SuperColony; findings verified by parallel AUDITOR/DETECTIVE/ANALYST byte-verification.
pluginCanPayon the service page checked the DEM rail and the payment phases but not the rest of the pipeline. The purchase flow expects exactly onenegotiate-fixed-pricephase followed by acommit-agreementphase, and listing verification does not require them, so a current-spec listing without them showed the buy box.Changes in
reference-implementations/dacs-directory:src/catalog/listingMetadata.ts,indexer.ts,types.ts,contracts.ts: the listing summary carriesoffering.flowPhases, thenegotiate-*andcommit-*phase kinds from the signed pipeline in signed order. As withpaymentPhases, a legacy listing has none (the field is absent), and more than 16 such phases leaves the list empty.app/service/[seller]/[listingId]/[version]/page.tsx: fordacs-v0.1listingspluginCanPayalso requiresflowPhasesto be exactly["negotiate-fixed-price", "commit-agreement"]. Legacy listings are unchanged. A summary saved before this change has noflowPhasesand shows no box until the next reindex, the same as forpaymentPhases.test/service-page.test.ts: the existing buy-box cases now carry the flow, so each keeps testing its own condition. New cases with no negotiation, no commit, neither, two negotiations, two commits, a misordered pair,negotiate-rfqin place ofnegotiate-fixed-price, and a summary withoutflowPhasesshow no box.test/rail-registry.test.ts(1 new test): the projection keeps the phases in signed order, returns an empty list when there are none, leaves legacy listings without the field, and empties the list past 16.Measured in a clean sandbox on Node 22.23.3 (
npm run setup,npm run typecheck,npm test,npm run lintintools/lint):c831514: typecheck clean, 1658 tests, 1657 pass, 1 skipped, 0 fail.rail-registry.test.ts; the service-page cases are added inside the existing test.pluginCanPay, the service-page test fails; with commit phases left out of the projection, the new projection test fails.Fixes #231
🤖 Generated with Claude Code
https://claude.ai/code/session_014MueqwRPFFVzoDnT4EMx7y