completeLogin() re-checks isActive()/isOperator() immediately before signing
in, not only at the password step. The two-factor challenge previously only
confirmed the operator row still existed (Operator::find() + a null check),
so disabling an operator or revoking every console role in the window between
the password step and the pending code did not stop the login from
completing — the realistic case is an owner locking someone out mid-login,
not a contrived one.
One check, in the static method both OperatorLogin's direct exit and
OperatorTwoFactorChallenge::verify() already share, rather than a second copy
in the challenge. completeLogin() now returns bool instead of void: false
means refused, and neither the guard nor the session was touched. Each caller
turns that into a ValidationException in its own vocabulary — 'email' /
auth.failed for the direct exit (already unreachable in practice, since
authenticate() just checked the same two conditions synchronously; kept for
defense in depth), 'code' / auth.not_an_operator for the challenge, since the
code entered may be genuinely correct and reusing "wrong code" would mislead.
The three sites that refuse to create or rename an operator onto a
customer's email (Admin\Settings::saveAccount(), ::inviteStaff(), and
clupilot:create-operator) all checked Customer::where('email', ...) as
a proxy for "does this address already have a portal login". A users
row with no matching customers row — an email changed on one side
only, or legacy/orphaned data — passed straight through: this dev
database already had one.
Extracted the three copies into Customer::emailTaken(), which checks
both tables directly, so the three sites cannot drift from each other
again.
The setup-route exemption already went through AdminArea::routeIs(),
which matches both admin.<name> and admin.via*.<name>. The logout
exemption next to it used a bare $request->routeIs('admin.logout'),
which matches only the canonical name — on a recovery hostname the
logout route is admin.via0.logout, so the check failed there and an
unenrolled operator's logout POST bounced back to enrolment instead
of being let through, on exactly the host that exists because the
canonical one is not working.
RequireOperatorTwoFactor exempted the whole of admin.settings while
compulsory two-factor was on, on the theory that it was "the page
where two-factor is set up". That component also carries staff
management, site visibility, network restrictions, account changes,
and the two-factor policy switch itself, so an unenrolled operator got
unrestricted access to all of that, not just enrolment.
Enrolment (secret, QR, confirm, recovery codes, regenerate, disable)
moves to its own route and component, admin.two-factor-setup /
Admin\TwoFactorSetup, reachable by every operator regardless of
capability. That is now the only page the middleware exempts besides
logout, and its redirect points there instead. admin.settings goes
back behind the policy like every other console page; only the policy
switch itself stays there, since only someone who already satisfies
it should be the one changing it.
EnsureCustomerActive used session()->invalidate() to sign a suspended
or closed customer out. In shared-host mode the console and the
portal keep their login key in one session, so invalidate() (flush()
+ migrate(true)) silently signed out an operator elsewhere in that
same browser too. Same fix as the console's own /logout route:
regenerate() gets a fresh session id without flushing attributes
belonging to another guard.
confirmPassword() already checked the right account per guard, but the
session marker it stamped, auth.password_confirmed_at, was one flat
key shared by every guard. In shared-host mode the portal and console
guards keep their login in the same session, so confirming a
customer's portal password left the marker in place for an
operator-only gate too. One private method now computes the key for
all three callers so they cannot drift apart again.
PublicSiteGate let anyone through on a bare Auth::guard('operator')->check(),
which stays true for a disabled operator's live session — disabling
someone does not sign them out, so it never stopped this gate from
showing them the real site indefinitely, even though EnsureAdmin
already treats them as inactive. Now requires the same thing EnsureAdmin
does: an active operator holding a console role.
Chosen to merely ignore an invalid operator session here rather than log
it out. This gate only decides what one request sees; logging out would
ripple into whatever else that session is doing (a concurrent console
tab, mid-task), a bigger side effect than a visibility check exists to
have. EnsureAdmin sets the precedent — it also rejects a disabled
operator per request without touching their session.
Customer::ensureUser() enforced uniqueness only within users, so a
customer provisioned (or registering publicly) with an operator's email
got a second, colliding identity for the same address — the exact thing
R21 exists to prevent. Checks Operator by address now, not by session
(the deleted assertNotAdmin() checked who was signed in, which could
never be the right question).
Public registration gets the same guard via a plain unique() validation
rule, reusing Laravel's stock "already taken" message rather than a
dedicated one — naming the collision specifically would tell a public
visitor an address belongs to staff, a worse leak than the generic
refusal every other collision here already gives them.
Impersonation answers 409 instead of crashing into it. The Stripe
webhook keeps recording the paid order (same principle as
openContract()'s own comment: the order is proof money changed hands)
and withholds only the colliding login, logged for an operator to
resolve by hand.
Verified the reverse direction Codex flagged as already covered: two
Admin\Settings actions and the clupilot:create-operator command all
already refuse an operator email that collides with a customer's, and
that check is sound today because every users row is created in
lockstep with a customers row of the same email (CreateNewUser and
ensureUser are the only two creation sites, both confirmed by grep) and
nothing in this app can change a user's email afterwards (Fortify's
updateProfileInformation feature is disabled, and no other code calls
that action).
console.require_2fa exempted admin.settings from the redirect it forces on
an unconfirmed operator, but that page only ever configured the global
policy — nobody could enrol behind it. A newly invited operator stayed
permanently redirected to a page with no way to satisfy the requirement,
the opposite of what the switch was for.
Adds enable/confirm/regenerate/disable to Admin\Settings, reusing Fortify's
own actions exactly as the portal's Settings already does — they only ever
touch the model they're handed, no guard or auth() call inside them, so
pointing ConfirmsPassword's guard at 'operator' is the only change needed.
The new card is deliberately not gated behind the site.manage capability
that hides the rest of this page's Owner-only sections: the compulsory
switch applies to every operator, so enrolment has to be reachable by every
operator too. Dropped the account card's stale hint pointing at a "separate
security area" that never existed.
POST /broadcasting/auth carries only the 'web' middleware group
(Laravel's own withBroadcasting() registers it that way), so
PusherBroadcaster::auth() resolved retrieveUser() on the DEFAULT guard
before the admin.runs channel's own Auth::guard('operator') check ever
ran — found nobody, since an operator is never signed in on 'web', and
threw straight from there. The guard was correct but unreachable.
Fixed by naming both guards on the channel registration itself.
Also adds broadcasting/auth to RestrictAdminHost::SHARED: in exclusive
mode it was neither admin.* by name nor on the shared list, so it 404'd
on the console host too — degrading silently to wire:poll.4s rather
than breaking outright, which is why nothing screamed.
It checked Auth::guard('operator')->check() instead of the users row it
was handed, so with an operator's own session still attached (shared
mode carries the same cookie through both legs of impersonation) it
threw the moment a customer's email already had a users row — a 500 on
the first impersonation of anyone who had signed into the portal
themselves. Deleting it rather than fixing the check: operators live in
`operators` now, never in `users` (R21), so a users row can no longer
BE an operator account for this to guard against. Also drops the dead
`'is_admin' => false` write — the column reads from nowhere any more.
redirect()->to(AdminArea::home()) resolved against whatever host the
leave() POST itself arrived on — the portal's, once the console has a
hostname of its own — landing the operator on the portal's dashboard
instead of back at the console. route('admin.overview') is domain-bound
to the console and generates the correct cross-host URL, matching what
this line did before this branch. The test named after this behaviour
now actually asserts it.
Deleted tests/Feature/Auth/ConsoleLoginLandsInTheConsoleTest.php — it tested
the three deleted response classes. Its intent (an operator's sign-in landing
in the console, not the portal) is covered by OperatorLoginTest's "signs an
operator in on the operator guard" and OperatorTwoFactorChallengeTest's "signs
the operator in once the code actually checks out", both confirmed present
before deletion; the route-separation half is covered by the new
ConsoleHostSeparationTest cases added in this commit.
saveServer() wrote host/port/encryption without touching any mailbox, so
the console kept showing every mailbox as verified against a server
nothing had tested since. Mirrors EditMailbox::save()'s existing guard on
a single mailbox's own identity, just at server scope: only clears when a
field actually changed, so re-saving unedited values leaves a real
verification alone.
Both places now delegate the actual clear to new Mailbox::
invalidateVerification()/invalidateAllVerifications() methods rather than
touching last_verified_at directly, sharing the mutation. The "did this
change" comparison stays separate in each caller - one diffs a loaded
model's attributes, the other diffs persisted settings against incoming
scalars, and forcing them through one function would add machinery no
actual duplication justifies.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MailboxTester required a usable SECRETS_KEY unconditionally, refusing an
unauthenticated relay's test-send on a fresh install with no key yet, even
though real sending never touches the cipher for that mailbox. Gated the
check on authenticates, same as the password-presence check beside it.
Comparison pass across MailboxTester and MailboxTransport turned up two more
of the same shape. MailboxTransport's delegate-cache fingerprint decrypted
$box->password unconditionally, even when unauthenticated and never going to
use it — a mailbox that once authenticated, stored a password, and later had
authenticates unchecked would crash real sending the moment SECRETS_KEY
became unusable. And MailboxTester had no guard against a stored port <1;
Symfony silently reinterprets that as port 25 rather than refusing it, so the
test button could reach whatever happens to listen there and report success
for a configuration MailboxTransport refuses outright.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An SMTP relay with a host and a from address but no username or password
- a trusted local or private-network relay - always used to be seeded as
a placeholder, because the takeover required a username to treat an
account as real, and Mailbox::isConfigured() always required a stored
password. Laravel's own SMTP mailer has supported unauthenticated relays
all along.
Add mailboxes.authenticates (default true, so every existing row keeps
its current behaviour) to tell "does not authenticate" apart from
"authenticates, but nobody has typed the password yet" - the same empty
password column used to mean both. The takeover migration now seeds the
real account off the host/address alone and marks it authenticates=false
only when neither a username nor a password resolved; isConfigured(),
MailboxTransport and MailboxTester all read it before requiring or
sending a password; the edit modal gets a checkbox for it with the
password field's hint updated to match, in both languages.
isUsable() checked only "is SECRETS_KEY nonempty", so a set-but-malformed
key (wrong length, garbage base64) read back as usable even though
encrypter() rejects it two lines below. EditMailbox::save() and
MailboxTester::run() both gate on isUsable() specifically to avoid an
uncaught RuntimeException reaching the operator; a lying isUsable() meant
that guard did not fire in exactly the configuration it exists for.
Both methods now read resolveKey() — the base64: prefix, the raw-base64
path, the 32-byte check — so they cannot disagree about a value either
one is given. SecretVault::isUsable() and the secrets console page's
"no key" banner both delegate down to this and are covered here too, not
assumed to inherit the fix correctly.
savePurposes() only checked that each key was a nonempty string, so a
stale or tampered mailbox key (reachable by posting straight to
/livewire/update) saved successfully while MailboxResolver silently
returned null for it. Every purpose's key must now name a mailbox that
exists; system's must additionally be active, since every unmapped or
inactive OTHER purpose falls through to it.
EditMailbox::save() also cleared last_verified_at only when the
password changed, but smtpUsername() depends on address and username
too (falling back to address when username is blank) — changing
either while leaving the password untouched kept showing a successful
verification for an identity nothing had actually tested.
EsmtpTransport's autoTls (default true) and requireTls (default false)
are independent of the implicit-TLS constructor argument: 'tls' left
these at their defaults, so a server that omitted STARTTLS sent
mailbox credentials in the clear while reporting success, and 'none'
would silently upgrade the moment a server offered STARTTLS. Both
MailboxTransport and MailboxTester already duplicated the "ssl, or
port 465 ⇒ implicit" half of this decision independently; MailTlsPolicy
is now the one place both read, for both dimensions.
MailboxTester still builds through Mail::build() rather than a hand-built
transport: MailManager::createSmtpTransport() forwards the whole config
array to EsmtpTransportFactory as DSN options, which already reads
auto_tls/require_tls itself, so the same policy reaches this path
through keys the config array already supports.
I3: nothing stopped an operator unchecking Aktiv on the mailbox that
mail.purpose.system points at. MailboxResolver::active() filters an
inactive mailbox at BOTH the direct lookup and the system fallback, so
that one checkbox could take mail.purpose.system to null and every
purpose without its own mapping down with it — the existing
system_required guard on savePurposes() reads as if this were covered,
but it only blocks the empty-mapping path, not this one.
EditMailbox::save() now refuses active=false against the mailbox 'system'
currently points at, the same way savePurposes() already refuses to leave
'system' empty.
I4: Admin\Mail::saveServer() writes mail.host — the platform's outbound
relay for every purpose mailbox at once — and CloudReady::toMail() puts a
customer's Nextcloud admin password in cleartext in the message body. So a
signed-in mail.manage session was already a credential-interception
primitive, the same threat Admin\Secrets' second gate exists for. Both
saveServer() and EditMailbox::save() (only when a new password is actually
being typed) now also require passwordRecentlyConfirmed(), following the
ConfirmsPassword pattern Secrets.php and Settings.php's two-factor actions
already use — including their per-action UI shape (an inline confirm form
scoped to the gated action), not a whole-page lock, so address, display
name, username, no-reply, active and the purpose mapping stay reachable
without confirming, per the brief's capability split.
Both gated and ungated paths are covered by tests, and both guards were
mutated away and confirmed to fail before being restored.
Mail::to($x)->queue($mail) resolves the DEFAULT mailer, and
Illuminate\Mail\Mailer::queue() (vendor Mailer.php:482) does
`$view->mailer($this->name)->queue($this->queue)` — overwriting
MaintenanceAnnouncementMail/MaintenanceCancelledMail's own 'cp_maintenance'
with the default mailer's name ('array' in tests, 'log'/whatever
MAIL_MAILER is in prod) before the job is even built. Both mails kept
sending through the old .env account instead of MailboxTransport. CloudReady
was never affected — MailChannel.php:66 preserves a notification's own
mailer.
Fix: name the mailer before queueing — Mail::mailer($mail->mailer)->to(...).
$this->name on the resolved Mailer is then already $mail->mailer, so the
vendor overwrite becomes a no-op instead of a silent downgrade. Confirmed by
reading the vendor source, not assumed.
SenderAddressTest's "names the purpose mailer..." test asserted
$mail->mailer on a bare, never-queued mailable, which proves nothing about
what survives queueing — the constructor sets it right regardless of what
MaintenanceNotifier does with it. Replaced it with one that drives
MaintenanceNotifier::deliver() under Queue::fake() and inspects the pushed
SendQueuedMailable's mailable. Verified it fails against the pre-fix code
first (the pushed job carried mailer = 'array'), then verified the fix
makes it pass, then mutated the fix away and watched it fail again.
EditMailbox::save() now checks SecretCipher::isUsable() before touching a
typed password and reports it as a form error, instead of letting
SecretCipher::encrypt() throw all the way to Laravel's debug page. Also
switches the mail.manage migration to Role::findOrCreate, matching every
sibling capability migration, so a squashed replay can't hit
RoleDoesNotExist.
The vault's mail.password row is no longer deleted by this migration at
all, in either direction: deleting it unconditionally destroyed a real
password whenever MAIL_USERNAME was empty (nowhere to carry it to), and
even on a successful carry-over a later rollback had nothing left to fall
back to. Reproduced both against real MariaDB, along with a migrations-
table drift that crashed a retry on the mailboxes.key unique constraint —
fixed with Mailbox::firstOrNew() and a transaction around each direction.
down() now goes through the new Settings::forget() instead of a raw
delete, so it stops leaving the settings cache holding what it just
removed. A skipped .env password (SECRETS_KEY still unset) now prints an
operator-visible line instead of migrating clean and failing silently
later. Host/port that resolve to config/mail.php's own placeholder
defaults are treated as unset rather than as a real relay.
The seed migration also guards against SECRETS_KEY being unset at migrate
time (encrypting the .env password would otherwise throw and take the
whole migration down, on a fresh install as much as in the test suite),
and the four tests/Feature/Mail files written before this migration reset
the mailbox table in their beforeEach so they keep testing what they did
before mailboxes.key collided with the seeded rows.
A mutation the reviewer ran (if ($box !== null) -> if (true)) left every
test green: CloudReady hand-rolled the same null-mailbox guard as
SendsFromMailbox, but nothing exercised its null branch. Added the
missing test, then extracted the decision itself (null mailbox -> no
sender fields, no_reply -> no Reply-To) into one shared method so an
Envelope-shaped and a MailMessage-shaped consumer can no longer disagree.
Re-review of the previous fix round found the "cannot disagree" part of
finding 4 was still open. resolution() had unified the log/array/null/
mailbox decision, but inside the mailbox branch describe() returned
$box?->address unconditionally while delegate() additionally required
isConfigured() before using it. A mailbox with active=true, a real
address, and no password produced
'mailbox://support/support@clupilot.com' — reading as entirely healthy
— while send() threw. That is exactly the harm the original finding
named: the log says fine, the mail fails.
resolution() now decides "unconfigured" too, not just "which
non-delivering mode": it calls isConfigured() itself, once, and returns
that verdict alongside the box. describe() and delegate() both branch
on the SAME result again, the same way they already did for log/array/
null — a mailbox with no password now reads as
'support@clupilot.com [unconfigured]' rather than a plain address, and
the address stays visible so an operator can tell WHICH mailbox needs
fixing.
Also renamed a test the re-review flagged as promising more than it
checked: "registers a mailer per purpose without touching the database
at boot" only ever verified the config array's shape — the DB-touching
claim is a separate, already-passing test just below it
(defines every purpose mailer as a plain literal...). Renamed to
"registers a mailer for every purpose". No behaviour change.
Added a test creating a mailbox with no password and asserting the DSN
both names the address AND carries the marker, plus that send() still
throws — proving agreement, not just a matching label painted on
separately. Mutated describe()'s unconfigured branch to drop the
marker: the test failed exactly on the missing 'unconfigured' string
(DSN read as a plain, healthy-looking address again). Reverted, reran,
passes.
Full suite: 667 passed. vendor/bin/pint --test clean on both files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reviewer continued the mutation sweep from Task 4's own commit and
found three more unpinned lines in delegate(), plus two real behaviour
gaps, all inside the same 40-line method:
- The fingerprint refresh (the headline feature: a worker's mailer
must pick up a corrected password without a restart) had no test —
mutating its check to "delegate === null" left the suite green.
- The implicit-TLS third argument to EsmtpTransport had no test either
— mutating it to a hardcoded true left the suite green, because the
only assertion touching it was instanceof EsmtpTransport, which the
hanging variant also satisfies.
- isLogging() only ever checked for 'log', but phpunit.xml sets
MAIL_MAILER=array for the whole suite. Task 5 wires real mailables
to these mailers next; the first feature test that sends one without
Mail::fake() would have opened a real SMTP connection.
- describe() and delegate() each ran their own copy of that guard —
which is exactly how deleting it from ONLY delegate() passed the
whole suite in the first place: __toString() kept reporting "log"
from its own untouched copy.
- An unconfigured host/port (Settings::get('mail.host', '') and a
stored null port casting to 0) built a transport pointed at
smtp://:587 or silently downgraded to plaintext port 25, instead of
refusing the way a missing mailbox already does.
Fixes: replaced isLogging() with resolution(), the one place that now
decides "log, array, null-default, or a real mailbox" — describe() and
delegate() both branch on ITS result, so a guard broken in one cannot
look fine in the other. Added an explicit RuntimeException for a blank
host or non-positive port, guarded the same way as the missing-mailbox
case just above it.
Tests: added coverage for the fingerprint rebuild (reuses the same
delegate until the password actually changes, proven by object
identity), the exact DSN string for both the STARTTLS (587) and
implicit-TLS (465) cases, 'array' and unset-default both landing on a
non-delivering transport, and the blank host/bad port throwing.
Mutated each of these four in turn on the final code and confirmed:
fingerprint check removed -> the reuse test fails (same object handed
back after the password changed); TLS argument hardcoded true -> the
587 DSN test fails ('smtps://...' where 'smtp://...:587' was
expected); NON_DELIVERING narrowed back to ['log'] -> both the array
and unset-default tests fail (a real EsmtpTransport where a safe
transport was expected); host/port guards removed -> both throw
expectations fail. Reverted each, reran, all twelve tests pass again.
Replaced the two tests that reached into delegate() via Closure::bind
to check "log or SMTP" with one behavioural test: point Settings at a
closed loopback port (127.0.0.1:1, so refusal is instant and entirely
local — no DNS, no dependency on this environment's network egress
policy) and assert Log::shouldReceive('debug')->once() while sending
for real through Mail::mailer('cp_support')->raw(...). Mutated
NON_DELIVERING to drop 'log' specifically and confirmed this test
alone catches it: a real connection attempt to the closed port throws
TransportException (Connection refused) in under 300ms. Reverted,
reran, passes again.
Also: the two tests that only checked the config array's shape and a
snapshot of the resolved address promised more than they verified —
CLAUDE.md R19 names a test that recomputes the implementation as
checking nothing. Added a source-level test that the five cp_* config
entries are parenthesis-free literals (same technique
DisplayTimezoneTest/IconLayoutTest use: a property of the file, not of
one run) as the actual "no query at boot" proof, and renamed the
send-time test to what it verifies now that the fingerprint test above
covers the live-refresh claim properly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of Task 4: MailboxResolver::for() only fell back on a missing
mapping or a missing row. Deactivating a mailbox — the operator's
explicit "do not use this" — left it resolvable anyway, so
MailboxTransport reached it, found isConfigured() false, and threw
instead of quietly using the next candidate the way every other
"this mailbox is unusable" case does. Worse, __toString() still
reported the deactivated address as though it were healthy.
named() now runs through a new active() filter at BOTH lookup sites —
the purpose's own mapping and the system fallback — so an inactive
system mailbox is refused exactly like a missing one, not handed out
as the last resort. A missing password stays deliberately unfiltered
here: that is a MailboxTransport concern (loud failure naming the
mailbox), not a resolver fallback trigger — silently rerouting mail
because a password hasn't been typed yet would hide the gap instead
of surfacing it.
Added two tests and mutated active() to a no-op filter to confirm they
catch it: both failed (one expected the system mailbox, got null; the
other expected null, got the inactive mailbox's full attribute dump).
Reverted, reran, both pass again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five purpose-named mailers (cp_maintenance, cp_provisioning, cp_support,
cp_billing, cp_system) land in config/mail.php as plain static entries —
no query runs to build them. Mail::extend('mailbox', ...) in
AppServiceProvider registers a transport that looks up its mailbox
through MailboxResolver only once a mailer is actually resolved, which
for queued mail is inside the worker at send time, never at boot.
Mutation-tested the brief's own three tests by deleting the MAIL_MAILER
guard from MailboxTransport::delegate(): all three stayed green. They
only exercise __toString()/describe(), a code path separate from the one
that actually sends. Added two tests that reach into delegate() itself
(a bound closure, since it's private) and assert the transport it
actually builds: LogTransport while MAIL_MAILER=log, EsmtpTransport once
it isn't. Re-ran the same mutation against the strengthened suite: it
now fails (EsmtpTransport where LogTransport was expected), then passes
again once reverted.
Verified two things against the installed versions before writing this:
LogTransport takes a Psr\Log\LoggerInterface and Log::channel(null)
resolves to the default channel rather than throwing, and
Mail::extend('mailbox', ...) is read by
MailManager::createSymfonyTransport() via $config['transport'] before it
would fall back to createSmtpTransport() — both matched the brief
exactly, no changes needed there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mailbox hand-rolled the same creating-time UUID assignment that
App\Models\Concerns\HasUuid already provides to 17 other models under
R11 (URLs address records by UUID, not integer PK), and skipped the
route-key binding that comes with it. Swap onto the trait, and cover
both the assignment and the route key with a test matching the
convention already used for Host/Order.
Also match MailboxFactory's @extends annotation to how the other
factories in the repo write it (Pint's fully_qualified_strict_types
rule turns a fully-qualified docblock reference into an import plus a
short name anyway, so this is what it would end up as either way).
Three things reported together.
── Editing belongs in a modal (R20)
The seats table grew its input fields into the row. It worked and it looked
broken: the row grew, the columns beside it jumped, and a table half in edit
mode reads as a rendering fault rather than as a form. The project already had
the answer — EditDatacenter, whose own header comment says it avoids exactly
that row-height jump — and the seats table simply did not use it.
EditSeat is now a ModalComponent. A modal is reachable WITHOUT the page's route
middleware, so it resolves the customer itself and re-reads the record rather
than trusting a hydrated property: a forged addressEditable would otherwise
open the address of an accepted seat, and the address is the person — editing
it hands one employee's access to another with nobody told. Only an invitation
still in flight can have its address corrected, which is the case that actually
comes up.
The actions column is now always drawn. It used to disappear when the only seat
was the owner, on the reasoning that there was nothing to act on. But every
seat can be renamed, and a column that vanishes does not read as "not
applicable here" — it reads as "this product cannot do that", which is how it
was reported, three times. The owner's row says "Geschützt" rather than leaving
an empty cell.
Also caught here: the edit fields carried class-wide #[Validate] attributes, so
an empty edit form made the INVITE button fail on a field the invite form does
not have. Rules for an action belong to the action.
── The update button
"I cannot run an update, it says everything is current." `behind` is a READING
taken by the agent every five minutes, not the state of the world — push a
commit and the console insists it is up to date and refuses to act. The agent
does its own fetch before deciding, so asking against a stale reading costs one
fetch and finds nothing; being locked out costs the deployment. The button is
now offered whenever an agent is alive and no run is in flight, labelled for
what it does rather than for what the last reading said.
── The update never announced that it had finished
Because the thing being watched restarts the thing doing the watching. Mid-run
every wire:poll request fails, and what answers afterwards is a new build being
questioned by the old page's JavaScript — so the card sat on "läuft" until
somebody reloaded by hand. A small Alpine watcher now asks a plain JSON
endpoint (no Livewire, no component state, no assets), treats a failed request
as the restart rather than as a fault worth giving up over, and reloads once
the build it is looking at is no longer the build it started with.
── Support
The page was a decorated placeholder: a button that raised a toast saying the
form was "only hinted at in the prototype", three invented ticket titles living
in the translation file, and no way to see what became of anything. It looked
thin because nothing on it was real.
It now leads with the customer's own requests — what somebody arriving here
wants to know is what they asked and whether anyone answered — with contact
details moved to the side where they belong. The form attaches the plan, the
instance and who is asking automatically: making a customer describe their own
server back to the people who built it is the part of support people hate. FAQ
answers end where the thing can actually be done, and "is it me or is it you"
is answered by a link to the status page.
637 tests. R20 recorded in CLAUDE.md and enforced by EditInModalTest: no page
view may grow an input field inside a <td>.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The console announced an update for "spätestens um 15:21" while the clock on
the wall said 17:21. Not a formatting slip in one string: fourteen views and
four components printed database values straight out, so every absolute time in
the product was two hours out all summer and one hour out all winter.
Two of those views looked as though they had handled it. They called
->timezone(config('app.timezone'))
which reads like "convert to local time" and, app.timezone being the STORAGE
zone that must stay UTC, converts to UTC — a no-op wearing the costume of a
fix, which is worse than no call at all because it stops the next person
looking. That exact string is now banned by test.
Worse than the labels were two FORMS. Maintenance windows and plan versions
filled their datetime-local fields from UTC and parsed what came back as UTC. A
datetime-local field carries no offset — it is the digits a person reads off
their own clock — so an operator typing 21:00 scheduled a window for 23:00
their time, and nothing anywhere said so. App\Support\LocalTime now holds
toField() and fromField() side by side, because getting one end right is not
half a fix, it is a fresh bug.
The mechanism is one Carbon macro, ->local(). It copies before converting:
Illuminate\Support\Carbon is mutable, so without that, rendering a timestamp
would rewrite the model attribute as a side effect and anything comparing or
saving it afterwards would be an hour or two out. Both Carbon classes get the
same body — Carbon keeps macros in one global table, so the second registration
replaces the first for every Carbon class, and two different bodies meant the
immutable version, safe for itself, silently became the mutable one's
implementation too. My own test caught that.
The existing tests had not caught any of this because they built their expected
values with the same wrong call the views used — a test that recomputes the
implementation asserts nothing. They now assert the wall clock, and the queued
update additionally asserts that the UTC time is NOT shown.
Recorded as R19 in CLAUDE.md and enforced by tests/Feature/DisplayTimezoneTest:
no Blade or Livewire component may format an absolute time without ->local(),
the UTC no-op is banned, storage stays UTC, both sides of the DST boundary are
checked, ->local() must not mutate, and a form field must round-trip to the
same instant.
APP_DISPLAY_TIMEZONE, default Europe/Vienna. 615 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The panel had been built by reading the template and believing the result.
Every round of review found the same class of defect, so this round the
template was rendered in a browser and measured with getComputedStyle, and the
implementation measured the same way — two number columns instead of an
opinion.
That immediately settled a disagreement: a review of the template's SOURCE said
the metric grid used a 20px gap. Rendered, it is 14px. The measurement wins.
Brought to the template's figures: label 11.5px (it was 11px in three separate
places, which is how it drifted — it is now one .lbl rule), value tracking
-0.02em, unit weight 450, metric row 6px above and 14px between, page head
centred with a 14px gap, grid gap 14px, columns switching at 1101/561px and the
h1 at 901px to match the template's own breakpoints, ring 62px, bar 5px.
The shared button now takes its height from min-height (40px) rather than
vertical padding, at 14px/600 with 0 18px padding and a 10px radius — a button
keeps its height whatever sits inside it, icon, spinner or bare text.
Also here, from the same round of review:
- The chart's blue border was Tailwind's own `ring` utility colliding with a
component class of the same name. Renamed to `.metric-ring`. This was
dismissed once as a screenshot artefact; it was real.
- Page titles lost the `sm:text-3xl` (40px) an earlier bulk edit had appended
to 23 of them. The template's h1 is 30px.
- The users table has its actions back — edit, suspend, lock and delete — for
every seat that is not the owner, with the owner refused in the action itself
and not merely hidden in the markup.
DemoCustomerSeeder writes one complete customer as real rows: instance,
subscription, six seats across four roles, a backup, current-period traffic and
thirty days of samples with a deliberate wobble and one day at 286/288 checks.
Nothing in the panel is drawn from a fixture any more, so anything missing
shows up as missing. Removing the demo is one deletion.
Verified: 607 tests, and a Codex comparison of the two measurement sets —
"a person would call them the same design", the only difference a 1px gap.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Availability is now counted from our own monitoring answers. Uptime Kuma knows
the figure, but the REST bridge in front of it only answers "healthy, yes or
no", and waiting for the bridge to grow an endpoint would have left the panel
without the number indefinitely. The sync already asks on a schedule; counting
the answers gives a percentage that is ours and explainable.
An unanswered check is not a failed one — the counter only moves when a real
answer arrived, so our monitoring going down does not become the customer's
outage on the one figure they are asked to trust. An instance nobody has ever
checked shows no availability rather than 100 %.
The downgrade path existed nowhere. Going down is not the mirror of going up:
it can ask an instance to hold more than the target plan allows. Smaller plans
are now listed even when they cannot be taken, with the obstacle in the
customer's own numbers — "you have 31 users, this package allows 10" — because
a greyed-out button that does not name what is in the way is the thing people
ring about. Storage is checked against the last real reading rather than the
contractual allowance, or anyone who once bought a large plan could never leave
it. The limit is re-checked in the action: a rule enforced only in markup is
not enforced.
And the smaller corrections from this round:
- The cloud tab carried a hardcoded "B" as the instance's initial, and the PHP
and MariaDB versions, which tell a tenant nothing they can act on.
- "EU — Serverstandort im Angebot festgehalten" under a label already reading
"Serverstandort" was two sentences saying nothing. It is the country.
- Support promised a reply "binnen 4 Std." with nothing behind it. What is true
everywhere else is an answer on the same working day.
- The actions column appeared even when no row in the table had an action.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nexxo