Repository navigation
Conversation
Regression tests for #882, failing on main: an export that fails for a reason without a message of its own (InvalidPayload here) reads as a pasted invalid attestation, and an answer signed by another key than the node announced has no message of its own, in Rust or in Dart. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126k49pgKzD8NA7SzMHepcJ
On the Reputation screen, an export that failed for a reason without a message of its own said "This is not a valid reputation. Request it again from its source." The user is on the source and pasted nothing. The export handler now falls back to "This Mostro could not export your reputation. Try again later." An answer signed by another key than the reputation_issuer the app read for the node gets its own marker, ReputationIssuerKeyMismatch, and a text that says to refresh the node's info by switching Mostro and back. StorageUnavailable already had a text of its own; InvalidPayload, a destination mismatch and unmapped node refusals were the cases that showed the wrong text. Closes #882 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126k49pgKzD8NA7SzMHepcJ
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughBackend export validation now identifies issuer-key mismatches separately from destination mismatches. The reputation screen uses dedicated text for issuer-key errors and a generic export-failure fallback for other errors. Localization strings, tests, and contract documentation were added or updated. ChangesReputation export errors
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the main change: export failures no longer display the invalid-reputation message. |
| Description check | |
The description is detailed and covers the change, affected flow, blast radius, testing, localization, specification updates, and known limitations. It also clearly states that manual testing was not … |
| Linked Issues check | |
Issue #882 requires an export-specific fallback and a distinct error for a response signed by an unexpected issuer key. _export now passes l10n.reputationExportFailed as the fallback. `check_expor… |
| Out of Scope Changes check | |
The changes stay within issue #882. Rust marker handling, export-screen fallback handling, localization, generated localization code, tests, and the reputation transfer specification directly support … |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (20 skipped: 2… |
Full details: Description check
-
Explanation
The description is detailed and covers the change, affected flow, blast radius, testing, localization, specification updates, and known limitations. It also clearly states that manual testing was not run and that screenshots are pending for the changed error text.
Full details: Linked Issues check
-
Explanation
Issue
#882requires an export-specific fallback and a distinct error for a response signed by an unexpected issuer key._exportnow passesl10n.reputationExportFailedas the fallback.check_exportednow emitsReputationIssuerKeyMismatchfor an issuer-key mismatch and keeps destination mismatches asInvalidReputationAttestation.localizedReputationErrormaps the new marker. The PR also adds localization, spec, Rust, and widget tests. The parent issue#674supplies context only.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#882. Rust marker handling, export-screen fallback handling, localization, generated localization code, tests, and the reputation transfer specification directly support the requested behavior. The described import path, bridge, persisted data, other platforms, wire behavior, and order or dispute status remain unchanged.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (20 skipped: 20 unsupported.)
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks the export trail,
A key mismatch gets a message, not a tale.
If other errors block the way,
A clear retry note has its say.
Then hops away with carrots in its pail.
Comment @coderabbitai help to get the list of available commands.
Linked issue
Closes #882. Part of #674.
Type
fix
What changes and why
On the Reputation screen, an export that failed for a reason without a message of its own read "This is not a valid reputation. Request it again from its source." The user is on the source and pasted nothing, so the message pointed them nowhere. That text is the default fallback of
localizedReputationError, which fits the import screen, and the export handler passed no fallback of its own.The export handler now falls back to "This Mostro could not export your reputation. Try again later." An answer signed by another key than the
reputation_issuerthe app read for the node (for example after the operator rotated it while the cached info event is stale) gets a marker of its own in Rust,ReputationIssuerKeyMismatch, and its own text: "This Mostro signed the answer with a key other than the one it announced. Switch to another Mostro and back to refresh its info, then try again." That is the remedy the app offers: the capability fetch runs when the relay pool comes online and after a node switch.One correction to the issue:
StorageUnavailablealready had a text of its own throughlocalizedDaemonError("…local database is unavailable. Restart the app…"), so it never showed the invalid-reputation text. The failures that did areInvalidPayload, a destination mismatch, and node refusals with no marker in the map (invalid_action,invalid_payload).Affected flow
Settings → Reputation (
/reputation), the Export action, on failure only. Rust:check_exportedinrust/src/api/reputation_transfer.rsnow distinguishes the signing key from the destination (no bridge signature changes, no regenerated code). Dart:localizedReputationErrorinlib/features/reputation/reputation_errors.dartmaps the new marker;_exportinlib/features/reputation/screens/reputation_screen.dartpasses the fallback. No order or dispute status is touched.Blast radius
InvalidReputationAttestationfromcheck_exportednow means only "names another identity"; the wrong-key case moved toReputationIssuerKeyMismatch. The import path is untouched: its checks (check_importable, the parse) keep their markers, and the import screen keeps its default fallback.reputationExportFailed,reputationExportUnexpectedIssuerin all seven ARBs;app_localizations*.dartregenerated withflutter gen-l10n, untranslated report empty.export_reputationerrors list inspecs/004-mostro-p2p-client/contracts/reputation_transfer.mdnames the new marker and what the screen shows.Platforms tested
None by hand. See Manual testing.
Manual testing
Not run: this branch was verified with
cargo test,flutter analyzeandflutter testonly; I did not run the app against a node. The steps a person should run:Setup
App built from this branch with
flutter run -d linux(debug), identity in Reputation Mode, active node advertisingreputation_issuer.Steps
reputation_issuerfrom its info event) after the app read it, Settings → Reputation → Export → confirm.Expected: "This Mostro could not export your reputation. Try again later." On
main: "This is not a valid reputation. Request it again from its source." (fails onmain).REPUTATION_ISSUER_SKafter the app read its info event, Export → confirm.Expected: "This Mostro signed the answer with a key other than the one it announced. Switch to another Mostro and back to refresh its info, then try again." After switching nodes and back, Export succeeds.
NotEligibleForReputationExport, Export → confirm.Expected: the "at least 10 completed trades" message, as before.
Expected: "This is not a valid reputation. Request it again from its source.", as before.
Not covered
Nothing was run by hand. iOS, Android, web: not run.
Screenshots
No visible change apart from the error text on failure; pending an app run.
Automated tests
First commit (
test:), failing onmain:an_export_answer_must_come_from_the_advertised_issuer_for_the_asked_identity: the wrong-key case now expectsReputationIssuerKeyMismatch(fails onmain:InvalidReputationAttestation).test/features/reputation/reputation_screen_test.dart:an export that fails for a reason without a message of its own says this Mostro could not export(fails onmainwith the invalid-reputation text) andan answer signed by another key than the one the node announced says to refresh its info.test/features/reputation/reputation_errors_test.dart(new): a known marker wins over the fallback; a failure without a marker reads as the fallback; without a fallback it reads as an invalid attestation (the import screen's default, unchanged); the new marker has its own text (fails onmain).Checklist
cd rust && cargo fmt --check && cargo clippy -- -D warnings && cargo testpass locally —cargo testpasses; with my local toolchaincargo fmt --checkandcargo clippy --all-targets -- -D warningsreport the same pre-existing findings onmain(505 fmt diffs, 27 clippy lints), none in the file this PR touches;rustfmt --checkon it is cleandart format --set-exit-if-changed .,flutter analyzeandflutter testpass locally (flutter testsummary line pasted under Automated tests)rust/src/api/or an.arb: regenerated code committed in the same commit (CONTRIBUTING.md § Generated code) — no bridge signature changed; l10n regeneratedspecs/or.specify/updatedAI assistance
Written with Claude Code (Claude Fable 5.1) from the issue; the maintainer reviews the diff and runs the manual steps.
🤖 Generated with Claude Code
https://claude.ai/code/session_0126k49pgKzD8NA7SzMHepcJ
Summary by CodeRabbit