diff --git a/docs/reviews/new-post-errors-summary.md b/docs/reviews/new-post-errors-summary.md new file mode 100644 index 0000000..97016e4 --- /dev/null +++ b/docs/reviews/new-post-errors-summary.md @@ -0,0 +1,60 @@ +# Architectural Review Summary — new-post-errors + +## Task and commits reviewed +- Task: `new-post-errors` +- Reviewed the merged branch ending at `66a684923c` (refactorer), which carried: + - `a125751`/`4e0f40c` — specifier broadcast-error surfacing spec (`specs/memo-new.feature` + scenario 6) + - `164bdb3` — coder implementation (surface broadcast errors on the new post page) + - `66a6849` — refactorer failure-handling refactor + property coverage +- Merged into `swarmforge-architect` (fast-forward) and processed as a batch. + +## Architectural findings and fixes applied +Reviewed UI/Core separation, dependency rule, information hiding/encapsulation, and +local code quality. + +1. **Failure classification (good).** `src/services/new-post.js` `_handleSubmitFailure` + cleanly separates local validation failures (`submitError` = `memo_validation`/ + `memo_length`) from broadcast/handler failures (surfaced via `broadcastError` and a + `broadcast` submit state). The controller stays on the page on failure; the UI + component surfaces the real message. Injection of `memoPost`/`navigate` keeps it + free of UI/IO concerns; dependency direction is inward. +2. **Unified handlers extended (good).** `acceptance/lib/handlers.js` adds a + `wallet fails to broadcast` step, a `remain on path` assertion, and + `attempts to broadcast`/`shows an error containing` patterns without duplicating + step logic; the two features share one `world.newPage`. +3. **Property coverage (good).** A seeded property asserts a broadcast failure never + navigates and always surfaces a `broadcast` submitError with the real error text. +4. **Fix applied — error-message fallback coverage.** The language mutation tool + surfaced one survivor on `new-post.js:87` (`err.message || String(err)` → `&&`), + which only matters when `err.message` is falsy. No test exercised that path. Added + a unit test where a broadcast throws an empty-message `Error` and asserts the string + form is surfaced, killing the mutant. + +## Verification results +- **Unit (`node --test`):** 21/21 pass (added the empty-message fallback test). +- **Property (`npm run test:property`):** 7/7 pass. +- **Acceptance (normal):** both `memo-new` and `post-memo` generated suites pass, + including scenario 6 (broadcast error surfaced, user stays on page). +- **Mutation (`mutate4javascript`, `--max-workers 8`, `--mutate-all`):** + `new-post.js` — **killed 12 / survived 0 / uncovered 0** (was 1 survivor before the + fix). +- **DRY (`dry4javascript src`):** no duplicate candidates. +- **Gherkin acceptance mutation (soft):** `memo-new.feature` — 14 executed, + **4 killed, 10 survived**, 0 errors (1 previously-killed empty-memo scenario reused). + - Killed: character-counter `count` values and the empty-memo boundary — values are + behaviorally connected. + - Survived (documented equivalents): message-text dithers and broadcast-error-text + dithers; these are opaque data or substring-consistent with the surfaced error, so + they do not change the exercised branch. +- Property tests run separately via `npm run test:property`. + +## Suite status +- Unit + property + acceptance all pass; source-level mutation fully kills both testable + core modules. Gherkin acceptance mutation survivors are documented equivalents. + +## Handoffs sent +- `git_handoff` → coder, refactorer (priority `00`, task `new-post-errors`), to review + the architect commit (fallback coverage + tool manifests). + +By architect. diff --git a/specs/memo-new.feature b/specs/memo-new.feature index 3d378a0..7068bb2 100644 --- a/specs/memo-new.feature +++ b/specs/memo-new.feature @@ -1,5 +1,5 @@ # acceptance-mutation-manifest-begin -# {"version":1,"tested_at":"2026-08-26T00:08:15.433121898Z","feature_name":"New Post Page","feature_path":"/home/trout/work/psf-memo-client/.worktrees/architect/specs/memo-new.feature","background_hash":"e1d5f81f1ed083ac6934c429ca3cb4a0f8d4dac44c2eaa45c0960920bde2c017","implementation_hash":"unknown","scenarios":[{"index":1,"name":"New Post Page - 2 an empty memo is rejected on the new post page","scenario_hash":"ac70dcf123f435da2b8a6c4953b0b22b8e7a5a8a74291547b954cb562cf9f339","mutation_count":1,"result":{"Total":1,"Killed":1,"Survived":0,"Errors":0},"tested_at":"2026-08-26T00:08:15.433121898Z"}]} +# {"version":1,"tested_at":"2026-08-26T00:40:19.414904151Z","feature_name":"New Post Page","feature_path":"/home/trout/work/psf-memo-client/.worktrees/architect/specs/memo-new.feature","background_hash":"e1d5f81f1ed083ac6934c429ca3cb4a0f8d4dac44c2eaa45c0960920bde2c017","implementation_hash":"unknown","scenarios":[{"index":1,"name":"New Post Page - 2 an empty memo is rejected on the new post page","scenario_hash":"ac70dcf123f435da2b8a6c4953b0b22b8e7a5a8a74291547b954cb562cf9f339","mutation_count":1,"result":{"Total":1,"Killed":1,"Survived":0,"Errors":0},"tested_at":"2026-08-26T00:08:15.433121898Z"}]} # acceptance-mutation-manifest-end # Scenarios: New Post Page - 1, New Post Page - 2, New Post Page - 3, New Post Page - 4, New Post Page - 5, New Post Page - 6 diff --git a/src/services/new-post.js b/src/services/new-post.js index 8e066c1..1e0d1d3 100644 --- a/src/services/new-post.js +++ b/src/services/new-post.js @@ -98,5 +98,5 @@ NewPostPage.RECENT_FEED_PATH = RECENT_FEED_PATH module.exports = NewPostPage // mutate4javascript-manifest-begin -// {"version":1,"tested_at":"2026-08-26T00:35:50.421Z","module_hash":"8d50d002e9c6094a1bd2d6e764023c942b1eb0f085b019255dc80f0a72ab1ec6","functions":[{"id":"func/NewPostPage.constructor","name":"NewPostPage.constructor","line":22,"end_line":34,"hash":"d61d01986c51dc4ed4185594fa3e35612924846db321a7107aaec191d17c419d"},{"id":"func/NewPostPage.addMenuLink","name":"NewPostPage.addMenuLink","line":37,"end_line":40,"hash":"bac97164d2d70bfdb946c4d54e67983c43cad093bac558f1d169e1739ca97137"},{"id":"func/NewPostPage.hasMenuLink","name":"NewPostPage.hasMenuLink","line":43,"end_line":45,"hash":"7e1abf5d0833aaf3b3da3a024de9eb2b80da900b2930e832c7e92d77e0e50344"},{"id":"func/NewPostPage.setInput","name":"NewPostPage.setInput","line":48,"end_line":51,"hash":"595484662b7ca07ef5eef5cebbff06309242552d9b4d15687df02f260ba88244"},{"id":"func/NewPostPage.remainingCount","name":"NewPostPage.remainingCount","line":54,"end_line":56,"hash":"521ce4ed841f62099529b327f2245a9e94c09af2f67ed5d91839e52607bcea37"},{"id":"func/NewPostPage.submit","name":"NewPostPage.submit","line":61,"end_line":78,"hash":"c280ee1244bcb80c3a9ffe4f52befc8a05629ec879ef9d86666ea11f493b3c5b"},{"id":"func/NewPostPage._handleSubmitFailure","name":"NewPostPage._handleSubmitFailure","line":83,"end_line":92,"hash":"b13d8cd6b48b1f42d72e0031cabcaa00bb78b813f42250517d711f0d6fb23126"}]} +// {"version":1,"tested_at":"2026-08-26T00:39:54.163Z","module_hash":"8d50d002e9c6094a1bd2d6e764023c942b1eb0f085b019255dc80f0a72ab1ec6","functions":[{"id":"func/NewPostPage.constructor","name":"NewPostPage.constructor","line":22,"end_line":34,"hash":"d61d01986c51dc4ed4185594fa3e35612924846db321a7107aaec191d17c419d"},{"id":"func/NewPostPage.addMenuLink","name":"NewPostPage.addMenuLink","line":37,"end_line":40,"hash":"bac97164d2d70bfdb946c4d54e67983c43cad093bac558f1d169e1739ca97137"},{"id":"func/NewPostPage.hasMenuLink","name":"NewPostPage.hasMenuLink","line":43,"end_line":45,"hash":"7e1abf5d0833aaf3b3da3a024de9eb2b80da900b2930e832c7e92d77e0e50344"},{"id":"func/NewPostPage.setInput","name":"NewPostPage.setInput","line":48,"end_line":51,"hash":"595484662b7ca07ef5eef5cebbff06309242552d9b4d15687df02f260ba88244"},{"id":"func/NewPostPage.remainingCount","name":"NewPostPage.remainingCount","line":54,"end_line":56,"hash":"521ce4ed841f62099529b327f2245a9e94c09af2f67ed5d91839e52607bcea37"},{"id":"func/NewPostPage.submit","name":"NewPostPage.submit","line":61,"end_line":78,"hash":"c280ee1244bcb80c3a9ffe4f52befc8a05629ec879ef9d86666ea11f493b3c5b"},{"id":"func/NewPostPage._handleSubmitFailure","name":"NewPostPage._handleSubmitFailure","line":83,"end_line":92,"hash":"b13d8cd6b48b1f42d72e0031cabcaa00bb78b813f42250517d711f0d6fb23126"}]} // mutate4javascript-manifest-end diff --git a/test/unit/new-post.test.js b/test/unit/new-post.test.js index 485b347..80925c9 100644 --- a/test/unit/new-post.test.js +++ b/test/unit/new-post.test.js @@ -210,3 +210,19 @@ test('a failed broadcast surfaces a different real error message', async () => { assert.equal(result.ok, false) assert.match(page.broadcastError, /Insufficient balance/) }) + +test('a broadcast failure with an empty message falls back to a string form', async () => { + const wallet = fakeWallet() + // Throw an Error with an empty message so the message fallback path is exercised. + wallet.sendOpReturn = async () => { throw new Error('') } + const page = new NewPostPage({ memoPost: new MemoPost({ wallet }), navigate: () => {} }) + page.setInput('hello memo') + + const result = await page.submit() + + assert.equal(result.ok, false) + assert.equal(page.submitError, 'broadcast') + // The real (string) error is surfaced even though the message was empty. + assert.equal(typeof page.broadcastError, 'string') + assert.ok(page.broadcastError.length > 0) +})