fix: surface network errors from user actions (#56)

[k5s-bot]

Closes #56.

Problem

The issue requires that all network request errors from user actions be displayed to the user. Glyph already has a solid error-feedback system (the notificationsStore toast store rendered by ToastContainer, plus per-page inline error state), and most user actions already use it. But an audit turned up a number of user-initiated actions that await a network write without any catch, so a failed request produced only an unhandled promise rejection: the user got no feedback, and for actions that navigate or close a dialog on success, the UI silently did nothing.

Fix

Each of these actions now surfaces its failure through the mechanism already used elsewhere in that file — an error toast (notificationsStore.error(...), using apiErrorMessage to prefer the API's own 4xx message), or the page's existing inline error state where that's the local convention:

Area	Actions fixed
Sidebar	New page, new page from template, new folder, drag-node-to-root move
Note view	Title, tags, and priority edits
Tasks board	Add lane
Task detail	Visibility change, delete task
Organizations	Delete org, rename org, remove member
OAuth clients	Rotate secret, rotate & revoke, revoke client, remove org, revoke token, revoke all tokens, load tokens on client select
Template manager	Save (create/update) — the edit form now stays open on failure so input isn't lost

Actions that were already handled (task status cycling, lane rename/reorder, deletes in the sidebar tree, share dialog, member add/role updates, etc.) are unchanged. Background/unload flushes (e.g. pagehide autosave) are intentionally left as console-only since they aren't foreground user actions with a place to show a toast.

Tests

• pnpm check (svelte-check): 0 errors, 0 warnings
• pnpm build: succeeds
• Existing pnpm test unit suite: passes
• Added e2e/network-errors.spec.ts (API-mode only): forces the page/folder create request to fail and asserts an error toast appears.

Note: The new E2E spec needs API mode (Postgres + the Go API), which isn't available in the automation sandbox, so it was validated with playwright test --list (compiles and is discovered) but not executed here — CI should run it. All other checks above were run.

---

Maintainer: apply the ai-review label to this PR to request an automated code review, or the ai-triage label to have review feedback addressed and the branch brought back in sync with its base. (The automation can't label its own PR, so this needs a human.)

🤖 Generated with Claude Code

[k5s-bot]

🤖 Starting work on this now.

[k5s-bot]

🤖 Watch live or take over the session here: https://claude.ai/code/session_01RUFkA36Rm1mPXB5S81y3fT

[k5s-bot]

Review: looks good — no changes requested. (Posting as a comment since GitHub won't let this account approve its own PR.)

This is a clean, well-scoped application of the codebase's existing error-feedback pattern to the previously unhandled user actions, with no unrelated changes.

I verified the fix is actually effective rather than dead code: every store method now wrapped in try/catch genuinely rejects on API failure and doesn't swallow internally — createPage/createFolder (repo.create propagates), updateNode/moveNode via optimisticWriter.update (re-throws after reverting), createLane (re-throws after rollback), and the orgs/oauth store methods (no internal catch). So the toast / inline-error paths will fire. apiErrorMessage returns the fallback for the 5xx the E2E test injects (matching its assertion) and prefers the API's own message on 4xx. The new spec's selectors (button[title="New page (default template)"], .toast.toast-error) match the real DOM.

Test coverage is light relative to the ~25 actions touched (the new spec exercises page/folder create only), but the pattern is uniform and low-risk. Notably, CI is fully green here — including e2e-api, so network-errors.spec.ts did run and pass, not just compile.

One non-blocking observation for the record: in settings/orgs/+page.svelte, a failed removeMember sets membersError, which the {#if membersLoading}{:else if membersError} block renders in place of the whole members list until reload. That follows the file's existing convention (updateMemberRole already behaves this way), so it's consistent — just flagging it as a known UX quirk, not something to fix in this PR.

[k5s-bot]

Review posted successfully to PR #64.

Summary

I reviewed PR #64 ("fix: surface network errors from user actions"). This is a clean approval — no changes requested.

What I verified:

• The fix is effective, not dead code. I traced every store method that the diff newly wraps in try/catch and confirmed each genuinely rejects on API failure rather than swallowing errors internally: createPage/createFolder (propagate repo.create), updateNode/moveNode via optimisticWriter.update (re-throws after reverting), createLane (re-throws after rollback), and the orgs/oauth store methods (no internal catch). So the added toasts and inline-error paths will actually fire.
• apiErrorMessage matches the test. It returns the generic fallback for the 5xx the E2E spec injects (which the test asserts) and prefers the API's own message on 4xx.
• E2E selectors match the real DOM (button[title="New page (default template)"], .toast.toast-error).
• CI is fully green, including e2e-api — so the new network-errors.spec.ts actually ran and passed in CI, not just compiled.
• Scope is respected — no unrelated refactors.

Non-blocking note left on the PR: a failed removeMember in settings/orgs/+page.svelte renders membersError in place of the whole members list until reload — but that follows the file's existing convention (updateMemberRole already does this), so it's consistent and not something to change here.

**Review U…

---

✅ Opened pull request: imaustink/glyph#64

[k5s-bot]

🤖 Starting work on this now.

[k5s-bot]

🤖 Watch live or take over the session here: https://claude.ai/code/session_011qeHvVv3EdLAfMoRqUJKjq

[k5s-bot]

ai-triage pass — synced with base, re-verified, nothing else to change

Feedback reviewed: the only review on this PR is @k5s-bot's COMMENTED review — an explicit clean approval, no changes requested. It traced each newly-wrapped store method to confirm the toasts/inline-error paths actually fire, checked apiErrorMessage, and confirmed the E2E selectors match the DOM. There were no inline review comments and no requested changes, so there was nothing to resolve or reply to in code.

The one item the reviewer flagged — a failed removeMember in settings/orgs/+page.svelte rendering membersError in place of the members list until reload — was explicitly called out as non-blocking and consistent with the file's existing updateMemberRole convention, "not something to fix in this PR." I agree and deliberately left it unchanged to keep this PR scoped to surfacing the errors (matching each file's local convention), rather than reworking that page's error-display UX.

What I did this run:

• Synced with base (main): merged origin/main into the branch (repo convention is merge commits, not rebase). The merge was clean — zero conflicts; the 7 upstream commits touched editor/search/pmmd files with no overlap with this PR's frontend files.
• Re-ran all checks locally after the merge, since main had moved:
  • pnpm check (svelte-check): 0 errors, 0 warnings
  • pnpm build: succeeds
  • pnpm test: 897 passed (66 files)
  • playwright test --list: e2e/network-errors.spec.ts still compiles and is discovered (runs in API mode, skipped in local via test.skip — this is what the description means by "API-mode only").
• Pushed the merge to the same branch (b2a3368..88305ad).

Deliberately not done: no code changes — the review requested none and the fix is verified green in CI (including e2e-api, which actually executes the new spec). I did not touch the removeMember UX quirk (out of scope per above), and left the PR description as-is since the branch's behavior is unchanged.

CI is re-running on the new merge commit; the prior run was fully green.

[k5s-bot]

The PR is updated and MERGEABLE. UNSTABLE just reflects that CI is re-running on the new merge commit (the prior run was fully green). Everything is in place.

Summary

I picked up PR #64 ("fix: surface network errors from user actions") and got it merge-ready.

What the feedback was: The only review is @k5s-bot's — an explicit clean approval with no changes requested and no inline comments. It verified the newly-wrapped store methods actually reject on failure (so the toasts/inline-error paths fire), that apiErrorMessage behaves correctly, and that the E2E selectors match the DOM. All 9 CI checks were passing. The single flagged item (a removeMember UX quirk in settings/orgs/+page.svelte) was explicitly marked non-blocking and consistent with the file's existing convention — "not something to fix in this PR."

What I did:

• Synced with base main by merging origin/main into the branch (repo convention is merge commits, and feature branches bring main in via merge). The branch was 7 commits behind; the merge was clean with zero conflicts — upstream changes were in editor/search/pmmd files with no overlap with this PR's frontend files.
• Re-verified everything locally after the merge (since main had moved): pnpm check → 0 errors/0 warnings; pnpm build → succeeds; pnpm test → 897 tests passed; playwright test --list → the new e2e/network-errors.spec.ts still compiles and is discovered.
• **Push…

---

✅ Opened pull request: imaustink/glyph#64