Skip to content

Owner section in Settings, and the email confirmation landing - #47

Open
ClaydeCode wants to merge 9 commits into
mainfrom
feature/clayde/owner-section-settings
Open

Owner section in Settings, and the email confirmation landing#47
ClaydeCode wants to merge 9 commits into
mainfrom
feature/clayde/owner-section-settings

Conversation

@ClaydeCode

@ClaydeCode ClaydeCode commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Closes #44 (#44).

The owner's email had two homes in this UI showing two different values. FreeshardBase/freeshard#220 settled where it lives — the owner's user row, users.email, verified by definition, with users.pending_email as the unverified candidate — and freeshard#221 shipped the API. This is the terminal side of that: one Owner card in Settings that owns both fields, and the screen the confirmation link lands on.

The first half of the issue (removing the published address from the Public page and Welcome.vue) already shipped as #45 / #46 and is not in here.

What is in it

An Owner card at the top of Settings, bound to the owner's user row — not to the controller's profile, not to identity.

  • Name writes through immediately.
  • Email does not: PATCH /protected/users/me makes it a candidate, and the live value only changes when the owner opens the mailed link. The card renders the three states the model allows (no address, pending, set) plus the self-hosted variant where GET /protected/settings reports email_enabled: false, the address is taken directly, and no resend control is offered.
  • A 502 from PATCH is not a failed save. The candidate is stored and only the mail did not go out, so the card says exactly that and offers the resend. Per the issue this is the live path on every shard today.
  • The About list loses Owner and Owner Email. They were the controller's copy of the same two values under the same labels — the duplication the issue opens with.

A confirmation screen at /?confirm_email=<token>.

  • The token sits in the document query string, not in the hash route, so it is read from window.location.search. $route.query never sees it.
  • It is reachable before the pairing check, because the link is opened in whatever reads mail — often a browser this shard has never seen.
  • It confirms only on a click. There is deliberately no GET route on shard_core; a request the browser makes on its own would let a mail scanner or prefetcher spend the single-use token.

One correction to the issue

§4 says POST /public/users/confirm-email answers "204 on success, 400 otherwise". The merged endpoint answers 204 either way — anything else would let a caller probe for pending addresses — so the screen cannot report success or failure, only that the request went through. Its copy is hedged accordingly ("If the link was still valid…"), with the expiry and the resend named so an owner whose link had died knows what to do. Two reviewers checked this against shard_core on main independently. Worth a look, since it is a real difference from what the issue specifies.

Recommended reading order

  1. src/lib/owner-email.js, src/lib/http-error.js — the token, the three states, the 502 discriminator
  2. src/components/OwnerCard.vue — the card
  3. src/views/ConfirmEmail.vue, src/App.vue, src/router/index.js — the landing
  4. src/views/Settings.vue, src/components/EditableText.vue — wiring and the new hint slot
  5. tests/unit/*.spec.js, agents.md

Verification

npm run lint, npm run test:unit (8 suites, 71 tests) and npm run build all green. Every new test was run against a deliberate mutation of the code beneath it; the ones that did not fail were rewritten until they did.

@vue/test-utils is added as a devDependency: the jest preset already compiled .vue files, but the mounting library was missing, so no component in this repo could be tested at all.

Review panel

Four adversarial reviewers ran against the diff and the issue, each without my reasoning: general correctness, test adversary, DevEx/readability, security. No BLOCKING findings from correctness, DevEx or security. The test adversary raised four, all proved by mutation and all fixed.

Blocking — test adversary (all fixed in 72c49d6)

Finding Resolution
does not confirm on its own asserted synchronously, so an auto-confirm one tick after mount passed Drains the microtask queue before asserting. Verified: fails against $nextTick(() => this.confirm()).
The headline 502 test's stubbed load already carried the candidate, so it passed on mount data rather than on the recovery The first read has no candidate, later reads have the stored one. Verified: fails when the recovery is deleted.
deliveryFailed = false in the resend path was entirely unpinned — a successful resend could leave "could not be sent" up forever Added the end-to-end test. Verified: fails when the line is removed.
The $route.name !== 'ConfirmEmail' guard in App.vue was untested; without it a reload on the confirm screen rejects with NavigationDuplicated outside beforeMount's try and strands the loading splash Added the reload test. Verified: fails when the guard is removed.

Advisory, fixed

  • Correctness + security, independently: isDeliveryFailure keyed on status 502 alone. Traefik answers 502 too whenever shard_core is down — where nothing was stored — and the card would then have claimed the address was saved and invented a pending one. It now requires the JSON detail only the API sends. (72c49d6)
  • Correctness: after a reload the pending hint asserted a confirmation mail had been sent; the card cannot know that, and today it usually had not. Reworded. (72c49d6)
  • Correctness: the no-address prompt promised storage, billing and service mail on a self-hosted shard that has no controller to send any. Now conditional. (72c49d6)
  • Correctness: Promise.all meant an unrelated /protected/settings failure blanked the whole card, including the name field. Now independent. (72c49d6)
  • Security: the token was only stripped from the URL on success, so an unclicked or failed link kept a live credential in the address bar, the history entry and any Referer. Stripped when the screen opens instead. (72c49d6)
  • Security: console.error(e) on the axios error expanded e.config.data — the token — into devtools. Logs the status now. (72c49d6)
  • Security + correctness: a stale token parked the browser on the confirm screen with no in-app way back. There is a continue button in every state. (72c49d6)
  • Correctness: the owner reload behind the Settings refresh overlay was fired and forgotten. (72c49d6)
  • DevEx: OwnerSection → OwnerCard; errorDetail moved out of the owner-email module; saveEmail's nested try/catch lifted into showStoredCandidate; EditableText gained a hint slot so callers stop rebuilding its grid; load() clears a stale delivery warning; confirmation copy no longer says "Done." and retracts it; one name for the resend action everywhere; agents.md counts and the 429 sibling case. (cbbc2fa)

Advisory, not taken

  • Two editable fields called "Name" (this card edits users.display_name, the Public page edits identities.name). Both stay, since the issue names the field "Name" and they are genuinely different values; the card now carries a line saying so and pointing at Public View.
  • Settings.vue has no test file, so the removal of the two About fields and the refresh wiring are unverified there. Standing up a harness for that view means registering the global filters, a router and the PayPal SDK — a bigger change than this issue warrants. The $ref call it flagged is now guarded and awaited.
  • A 400 test for the confirmation screen — the endpoint never sends one; see the correction above.

🤖 Generated with Claude Code

@ClaydeCode
deps: add @vue/test-utils for component tests
34aefa7 
The jest preset already compiles .vue files but the mounting library was
missing, so components could not be tested at all. The lockfile's version
field also catches up with package.json, which just set-version rewrites.
@ClaydeCode
lib: add helpers for the owner's email address
bb07eae 
Reading the confirmation token, naming the three address states and telling
a 502 that stored the candidate apart from a real save failure are the parts
of the coming Owner section that are easy to get subtly wrong, so they live
where a test can reach them directly.
@ClaydeCode
settings: put the owner's name and email on an Owner card
c3ec115 
Both fields now read and write the owner's user row, which is where the
address lives since freeshard#220. The email does not write through: it
becomes a candidate the owner confirms from a mailed link, so the card also
carries the pending state, resend and cancel.

A 502 from PATCH /protected/users/me stores the candidate and only fails to
send the mail, so it is reported as such rather than as a failed save. On a
shard that cannot send mail at all the address is taken directly, and no
resend control is offered.

The About list drops Owner and Owner Email: they were the controller's copy
of the same two values under the same labels, which is the duplication this
card removes.
@ClaydeCode
router, ui: land the email confirmation link on its own screen
4d302af 
The controller mails the link as https://<shard-domain>/?confirm_email=<token>,
so it arrives here and the last step is the UI's. The screen confirms only on
a click, because a request the browser makes on its own would let a mail
scanner spend the single-use token — the same reason there is no GET route.

It is reached before the pairing check: the link is typically opened in
whatever reads mail, often a browser this shard has never seen, which the
redirect to /welcome would otherwise swallow. The token sits in the document
query string rather than in the hash route, so it is read from
window.location and dropped from the URL once it has been spent.
@ClaydeCode
meta: document where the owner's address lives and how it is confirmed
e455951 
The two traps a future agent walks into are the 502 that stored the address
anyway and the confirmation token sitting outside the hash route, so both are
written down next to the auth flow they belong to.
@ClaydeCode
settings, ui: refresh the Owner card, and send unpaired browsers some…
b929e84 
…where useful

The Settings refresh button reloaded only the controller profile, so a
confirmation opened on a phone never showed up on the desktop that was
waiting for it. After confirming, an unpaired browser is sent to the public
page instead of the app grid, which needs a pairing it does not have.
@ClaydeCode
settings: report a cancelled address change honestly
bfce4c9 
Cancelling re-read the owner row afterwards, so a failed read reported the
cancellation itself as failed when it had already gone through. The 204 says
exactly what changed, so the card applies that instead.
@ClaydeCode
settings, ui: address the review panel
72c49d6 
- Tell the API's 502 from the proxy's. Traefik answers 502 whenever shard_core
  is down, where nothing was stored; only the API's own 502 carries a JSON
  detail, and only it means the candidate is on file.
- Stop the pending hint from claiming a confirmation mail went out. After a
  reload the card cannot know whether one did, and today it usually did not.
- Stop promising mail on a shard that cannot send any: the no-address prompt
  is the first thing a self-hosted owner reads.
- Keep the name editable when only the settings read fails.
- Drop the confirmation token from the URL when the screen opens rather than
  when it succeeds, so a failed or ignored link stops riding in the address
  bar, the history entry and any Referer. Log its status, not the request.
- Offer a way off the confirmation screen in every state; without it a stale
  token parked the browser there with no in-app way back.
- Await the owner reload behind the Settings refresh overlay.

Tests: the 502 recovery, the resend that clears the warning, the reload on the
confirm screen and the third address state were all passing against mutations
of the code under them.
@ClaydeCode
ui, lib, meta: readability pass over the owner section
cbbc2fa 
- OwnerSection becomes OwnerCard: it renders a b-card, and every sibling that
  does is named for one.
- EditableText gets a hint slot, so callers stop rebuilding its grid from the
  outside to line text up under a field.
- errorDetail moves to lib/http-error.js. It formats any axios failure and had
  nothing to do with the owner's address.
- saveEmail hands its 502 branch to showStoredCandidate, which drops a
  try/catch nested inside a catch.
- load() clears the delivery warning: after a resend from another device, a
  refresh was leaving a false "could not be sent" on screen.
- The confirmation screen leads with what it can honestly say instead of
  saying "Done." and taking it back in the next clause, and all four places
  that talk about resending now call the button by its name.
- agents.md: the 429 that stores nothing, and two counts the diff made stale.

Tests read through the DOM the owner uses — pencil, type, submit — rather than
EditableText's internals, and the route table is no longer reached through the
shared router instance.
@ClaydeCode
ClaydeCode requested a review from max-tet August 31, 2026 16:02
@ClaydeCode ClaydeCode mentioned this pull request Aug 31, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@max-tet max-tet Awaiting requested review from max-tet

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Owner section in Settings; stop showing and editing the email on the Public page

1 participant

@ClaydeCode