docs: fix order state transitions and improve documentation accuracy - #18
Merged
Conversation
- Replace vague Re-sync user data from relays, with specific protocol action details and add confirmation dialog text explaining when to use it
- Taker cancel returns order to pending (republished), not canceled. -Add timeout behavior note referencing expiration_seconds from event 38385.
- Replace incorrect unilateral cancel with cooperative cancel flow - Add cancel transitions to FIAT_SENT table (were missing entirely) - Clarify that order status does not change until both parties cancel
- Buyer may need to provide new invoice if payment fails after retries - Separate available actions by role (seller: none, buyer: add-invoice) - Reference PAYMENT_FAILED section for full details
- No handler in mostrod, no spec in protocol docs - Mark COMPLETED_BY_ADMIN as reserved/unused status in mostro-core - Clean up DISPUTE and IN_PROGRESS transition tables
- Only applies to pending orders not taken before expires_at - No direct notification; client detects via updated replaceable event
- Add release and cooperative cancel as available actions during dispute - Clarify IN_PROGRESS is a dispute status (kind 38386), not an order status - Users can resolve disputes themselves while admin review is pending
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated ACCOUNT_SCREEN.md to replace a generic re-sync with a Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Mostro as Mostro Server
participant LocalState as Local State Manager
participant UI as UI
Client->>Mostro: send `restore-session` command
activate Mostro
Mostro->>Mostro: fetch non-finalized orders & disputes (annotate `trade_index`)
Mostro-->>Client: return orders & disputes
deactivate Mostro
Client->>LocalState: update/merge returned orders & disputes
activate LocalState
LocalState-->>Client: merged state confirmed
deactivate LocalState
Client->>UI: show success snackbar
activate UI
UI-->>Client: snackbar displayed
deactivate UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.specify/v1-reference/ACCOUNT_SCREEN.md:
- Around line 152-153: Update ACCOUNT_SCREEN.md to match the protocol: replace
the incorrect action name "restore-session" with "restore" wherever referenced
and rename the payload field "trade_index" to "last-trade-index" so the action
and payload names match the documented protocol ("restore" /
"last-trade-index"); ensure both occurrences in ACCOUNT_SCREEN.md that mention
the action and payload are updated and any example payloads or descriptive text
reflect the corrected names.
In @.specify/v1-reference/ORDER_STATES.md:
- Around line 64-67: The MD028 failures are caused by blank lines inside
consecutive blockquote sections around the "Cancel behavior depends on role in
the order" and "Timeout behavior" blocks; remove the empty lines so each
blockquote paragraph is a continuous series of lines that each start with ">"
(no blank line between them) and apply the same change to the similar blockquote
at the later occurrence (the block containing the timeout/cancel explanation
referenced in the comment).
- Around line 318-322: The spec marks the ORDER_STATE `completed-by-admin` as
reserved/unused but later appendices still document
`admin-completed`/`admin-complete` restore/action/display entries, causing a
contradiction; update the appendices to match the reserved status by removing or
marking as unavailable any restore/action/display references to
`admin-completed`/`admin-complete` and replace them with a note that
`completed-by-admin` is a terminal/reserved state with no actions, or conversely
re-enable the state consistently if intended—search for the tokens
`completed-by-admin`, `admin-completed`, and `admin-complete` and edit the
restore/display sections so the protocol documentation and enum comment are
consistent.
- Around line 122-127: The Appendix A terminal-action mapping still lists a
generic "cancel → canceled" which contradicts the cooperative-cancel semantics
introduced (where a single-party "cancel" leaves the order in `active` until the
counterparty also sends `cancel`); update the Appendix A mapping and any other
terminal-action tables that show "cancel → canceled" (including the other
occurrences noted) to reflect that a single-party `cancel` is non-terminal (maps
to `active`/no terminal transition) and only a mutual/both-party `cancel` should
map to `canceled`, and adjust any explanatory note/footnote to reference the
cooperative-cancel behavior for `cancel`, `active`, and `canceled`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba7cc379-1f12-4c10-8b72-c280a932a8b7
📒 Files selected for processing (2)
.specify/v1-reference/ACCOUNT_SCREEN.md.specify/v1-reference/ORDER_STATES.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.specify/v1-reference/ORDER_STATES.md:
- Around line 572-573: The table rows for the `fiat-sent` -> `release`
transition leave the buyer outcome as `-`, causing ambiguity; update the
`fiat-sent | release` entries (and the similar rows around the `581-582` area)
to explicitly state both buyer and seller next-states (e.g., buyer -> `settled`
or `settled-hold-invoice` and seller -> `settled` as appropriate to the release
flow) so the role-specific FSM mappings match the release flow description;
locate the entries referencing `fiat-sent`, `release`, and
`settled-hold-invoice` and replace the `-` with the correct buyer/seller
outcomes consistent with the documented release behavior.
- Line 794: Table rows containing the extra cell with the note "(reserved — not
generated by current protocol)" (e.g., the row with `completed-by-admin` /
`admin-completed`) are malformed and break the Markdown table; remove the extra
pipe-delimited cell and move the reserved note into an existing cell (for
example append it to the same cell as `admin-completed` using a <br> or
parentheses), and apply the same fix to the other rows that add that extra cell
with the reserved note so each row has the correct number of columns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7a343e4-8197-49d4-b274-b11264db1bad
📒 Files selected for processing (1)
.specify/v1-reference/ORDER_STATES.md
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
2 participants
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.
Summary by CodeRabbit