setup-php installs PHP 8.3+ from apt on these arm64 runners: a ~145s floor
against ~35s for 8.1/8.2, with a tail that twice ran past the step timeout
and failed the run (#178). Caching the .debs softened it without removing
the apt step, and 8.5 has the same problem.
Add a per-version CI image built on php:<version>-cli-alpine and a workflow
that publishes it to git.unsupervised.ca/unsupervised/ci-php:<version>. The
org is public, so the packages pull anonymously.
The image carries bash and nodejs because act_runner runs JavaScript actions
inside the job container, GNU coreutils/grep/sed because the workflow scripts
use `tac` and `grep --include`, and curl/jq/git/zip for release.yml and
bin/build-zip.sh. Composer 2 and the intl and zip extensions round it out.
Nothing consumes the images yet — ci.yml and release.yml switch over in a
follow-up, because a job cannot run in an image that has not been published.
Part of #187
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01D9acV1mHktGAb1uyvNmrR2
The Book a lesson for a student panel built its picker from the us_student
role but vetted the submission with the book_lesson capability. ChildLoginGate
and RegistrationLoginGate withhold that capability from accounts that keep the
role, so the panel offered every guardian-managed child and every unapproved
signup and then refused them — with a message claiming no student had been
chosen, and a form cleared of all five fields.
Withholding book_lesson stops those accounts registering in their own name. It
was never meant to stop the studio acting for them, which is what the panel is
for, and for a child is the only route to a lesson besides their guardian.
Guard the student role instead, via a new RoleManager::isStudent() shared with
every picker and guard on the staff side so the two cannot drift apart again.
Group enrolment gets the same predicate: addDirect() and grantAccess() vetted
their posted ids not at all, and would enrol an instructor, an administrator,
or an account deleted since the page was drawn — raising a real payment against
them for a priced class.
Keep a refused booking's fields as submitted, reading the form through one
LessonController::submittedBooking() so what gets booked and what is shown
again cannot disagree about a field name. A booking that succeeds still leaves
an empty form, so the next one does not inherit it.
Closes#185
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01XunBYk2sFEc1oL14sUiuBU
Two related gaps, closed together because the second is created by the first.
A private lesson could only be booked by the student or their guardian, so a
booking taken over the phone had no way in — where group classes have had "Add
students directly" all along. "Book a lesson for a student" is now a panel on
Scheduler and My Lessons: student, open time, lesson type, with weekly term
reservations and a no-charge option for make-up lessons. The booking core is
extracted to Booking\LessonBooker and shared with POST /bookings, so the two
paths cannot drift on offering rules, slot claiming, or billing.
That leaves a registration with no intake answers and no policy acceptances,
because nobody was at a keyboard to give them — already true of every directly
added group-class student. Ticking the boxes on a student's behalf would be an
audit trail that says something untrue, so instead the answers are collected
another way and recorded afterwards, from a lesson's or an enrolment's detail
page. Every recording must say how it was collected, which is stamped on each
row along with who typed it and shown in a new "How it was given" column: a
policy ticked online and one transcribed from paper must never look alike.
Only staff-made registrations qualify (us_lessons.booked_by,
us_group_enrollments.enrolled_by) — one the student made already holds their
own answers. Only what is still missing can be recorded, re-checked at write
time, so a stale or double-posted form cannot duplicate or overwrite. No IP is
stored for a transcription, and accepted_by stays the student while recorded_by
names the staff member.
Intake is now generic over Registration\IntakeSubject, which Lesson and
Enrollment both implement; LessonDetail became Registration\IntakeAudit and is
shared by both detail views rather than duplicated.
Closes#182
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01QfHt6CyJHz6KkA4RuaS7WK
I misread the instruction to hold 8.5 as "take it out until the cache
lands" and removed it in f6481d4, so #179 merged without it. It was meant
to stay where it was. Restoring it.
8.5 takes the same php-builder path as 8.3, so the apt cache in this
branch is exactly what it needs, and the per-version key means it caches
its own ~70 -dev packages rather than sharing with the ondrej-path jobs.
composer test passes on 8.5.9 locally: 915 tests, 2592 assertions.
Refs #178.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
Run 536 proved the cache works -- ACTIONS_CACHE_URL is now populated, no
GHES warning, "Cache saved with key: apt-php-builder-deps-ARM64-v1" -- and
in doing so showed the key was wrong. The jobs that saved it were 8.1 and
8.2.
They take the ondrej PPA path, a handful of runtime packages, while 8.3
takes php-builder and its ~70 -dev packages. One shared key therefore lets
whichever job finishes first decide what every other job restores, and 8.1
is always first, at ~30s against 8.3's several minutes. 8.3 would have
restored a few runtime .debs it has no use for and then downloaded all 70
anyway. My "the dep list is the same for every PHP version" comment was
true only among the builder versions.
Keyed per version now, so each path caches what it actually installs. That
also picks up a small win on 8.1 and 8.2 rather than only avoiding harm.
Both temporary steps are gone. Report restored .debs was there to show the
cache URL arriving and the restore landing; both are established, so it
goes. Keep downloaded .debs stays -- it is not instrumentation, it disables
Ubuntu's docker-clean, without which there are no .debs left to cache.
Refs #178.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
thatguygriff/infra#1 turned the runner cache server on, so actions/cache
has an endpoint for the first time. v4 rather than v3 because v4.2+ can
speak the cache service v2 API, which is what the runner serves.
The instrumentation step now also prints ACTIONS_CACHE_URL, so a single
run shows the endpoint arriving and the cache being read in one log
rather than needing a separate probe.
Refs #178.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
Experiment for #178. On self-hosted runners setup-php installs PHP 8.3+
through php-builder, whose install.sh apt-installs ~70 -dev packages
before unpacking the build. The build tarball itself is only 19MB, so
that apt work is the whole cost, not the download.
Ubuntu's image drops the .debs after install, so every job fetches them
from the archive again. This keeps them and restores them through the
cache server, which lives in the cluster, so a WAN download becomes a
local one. The dependency list does not vary by PHP version, so a single
key serves 8.3 and 8.5 and the quality and build jobs alike.
Only the .debs are cached. /var/lib/apt/lists is deliberately left alone,
since a stale index is how apt starts 404ing mid-install, and reducing
flakiness is the entire point.
The Report restored .debs step is temporary instrumentation to show
whether the cache is actually being read. Baseline to beat, from run 526:
8.1 30s, 8.2 53s, 8.3 454s, quality 151s, 8.5 733s and a hard failure.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
8.5 lands on setup-php's php-builder path, same as 8.3, so it pays for
~70 apt -dev packages on every run. It timed out in run 526 and passed in
527 on identical code, which is a coin toss, and merging it would mean
intermittent red for a version nothing ships on yet.
The code is fine on 8.5 — verified locally on 8.5.9: 915 tests, PHPStan
and PHPCS clean, check-platform-reqs satisfied. This is purely about the
runners, so 8.5 comes back once #178 is fixed and the matrix is cheap
again. This PR is now just the PHPCS/PHPStan consolidation.
Refs #178.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
Tested and the hypothesis is dead. actions/cache@v4 resolved properly
(SHA 0057852) and printed the same GHES warning as v3, so the action
version was not what was closing the gate.
That points at the runner rather than the action. Gitea's docs say the
runner patches the GHES check out of the action's bundle only when it
recognises it, and that a bundle it does not recognise "is left alone".
No patching for either v3 or v4, and no ACTIONS_CACHE_URL to patch it
towards, is what you get when cache is simply off in the runner config.
So the fix is cache.enabled in act_runner's config.yaml, with host set to
an address job containers can reach. Nothing in this repository unblocks
it, and inert steps are what let the Composer cache rot unnoticed, so the
experiment comes out again. Both halves — the apt cache and the move to
v4 — should land together once the runner serves a cache.
Refs #178.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
My last conclusion was wrong. The GHES warning is not evidence that the
runner has no cache server; it is actions/cache v3 disabling itself.
v3's isGhes() reads GITHUB_SERVER_URL, and anything that is not
github.com reads as GitHub Enterprise, so on Gitea it always trips and
the action returns before touching the cache. That explains the 0.2s
save perfectly well without any runner setting being off.
Gitea's runner 3.0.0 release notes say every runner starts its own cache
server and that the runner "patches action bundles at load time to open
the GHES gate and read the cache endpoint from ACTIONS_CACHE_URL", with
actions/cache supported unforked. Our runners report v3.0.0, so the
server should be there and the gate should be open — but the patching
evidently does not reach a v3 bundle.
So this reapplies the apt cache and moves every actions/cache to v4. If
the hypothesis holds the GHES warning disappears, the save step actually
takes time, and a second run restores the .debs. The Composer cache gets
the bump too, since it has been silently doing nothing for the same
reason.
Refs #178.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
Measured on run 527 and it cannot work. Every actions/cache step on these
runners prints
Cache action is only supported on GHES version >= 3.5 ... check with
GHES admin if Actions cache service is enabled or not
and then no-ops. The save step finishes in 0.19-0.31s, which is not a
few hundred megabytes of .debs going anywhere. setup-php timings were
unchanged against the run 526 baseline, within the usual variance:
8.3 454s then 148s, 8.5 733s then 446s, both noise rather than signal.
The same warning appears on the Composer cache this workflow has carried
all along, including run 523 and earlier, so that step has never cached
anything either. Worth fixing, but in the runner config rather than here.
Leaving dead steps in the workflow is how the Composer cache went years
without anyone noticing it did nothing, so the experiment comes out until
act_runner has its cache service enabled. It is in the history when that
happens. Detail in #178.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
Experiment for #178. On self-hosted runners setup-php installs PHP 8.3+
through php-builder, whose install.sh apt-installs ~70 -dev packages
before unpacking the build. The build tarball itself is only 19MB, so
that apt work is the whole cost, not the download.
Ubuntu's image drops the .debs after install, so every job fetches them
from the archive again. This keeps them and restores them through the
cache server, which lives in the cluster, so a WAN download becomes a
local one. The dependency list does not vary by PHP version, so a single
key serves 8.3 and 8.5 and the quality and build jobs alike.
Only the .debs are cached. /var/lib/apt/lists is deliberately left alone,
since a stale index is how apt starts 404ing mid-install, and reducing
flakiness is the entire point.
The Report restored .debs step is temporary instrumentation to show
whether the cache is actually being read. Baseline to beat, from run 526:
8.1 30s, 8.2 53s, 8.3 454s, quality 151s, 8.5 733s and a hard failure.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
The two jobs were identical up to their final step, each paying for its own
Setup PHP. That step is the flaky one (#178), so running it twice to reach
two short commands was two chances for a run to fall over instead of one.
They are now steps in a single Coding Standards & Static Analysis job.
The one thing given up is that PHPCS failing now stops the job before
PHPStan reports, where before the two ran in parallel and both spoke. That
seemed a fair trade for halving the exposure, and the fix for a PHPCS
failure rarely depends on knowing PHPStan's verdict at the same time.
PHP 8.5 joins the test matrix. composer.json already allows it at >=8.1
and the suite passes on 8.5.9 locally: 915 tests, PHPStan and PHPCS clean,
check-platform-reqs satisfied. 8.4 is deliberately not added, only 8.5.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
The GitHub API rate limit was the wrong diagnosis, so this reverts the
1Password-backed token added in #175 along with the mirrored composite
action, leaving the workflows as they were.
Timing every Setup PHP step across runs 454-523 rules the rate limit out.
PHP 8.1 and 8.2 install in 26-41 seconds, 12 for 12, never once failing.
PHP 8.3 has never finished in under 143 seconds and ranges up to 1273,
with two outright failures. Those jobs share a fan-out, and so an egress
address and a rate limit bucket, with the 8.1 and 8.2 jobs that are never
touched. A throttle could not sort itself by PHP version that way.
Run 523, the first to carry the token, is the direct refutation: the
token resolved and verified, and Setup PHP still took 749 seconds on
kallone and 408 on eris. The 8.3 penalty also predates the whole story,
sitting at ~145 seconds back on 30 July.
What is left is a slow path specific to 8.3 on these arm64 runners, whose
long tail sometimes crosses the step timeout and reports the unhelpful
"Could not setup PHP 8.3". That is worth fixing on its own terms rather
than behind a token that was never in the path.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Uw545F1vveNJKjzLxdi2ks
setup-php resolves its tools through the GitHub API, unauthenticated at 60
requests an hour per source address. A CI fan-out across the fleet exhausts
that bucket, and the step then retries for several minutes before reporting
only "Could not setup PHP 8.3". It reads as a hang rather than a throttle,
and it took out both a main CI run and a release build.
Each cluster has its own egress address and so its own bucket, which is why
the same job passed on one runner and failed on another in the same minute.
The token comes from 1Password through the Connect instance in whichever
cluster picked up the job, matching the pattern in thatguygriff/infra. That
repository's composite action is not reachable from here, so it is mirrored
locally. It stays a step output rather than being exported to the job
environment, to keep it away from the package scripts composer install runs.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0133tYSQoZhoKebKZV8o2GPs
Stripe configuration was a one-way door. Keys could be entered but never
removed, and entering them moved every student onto card billing at once,
so there was no way to have Stripe live and satisfy yourself that card
payments worked before committing the studio to them.
Two settings-page changes open both directions:
Default payment method (`us_default_payment_method`) is now an explicit
choice between card and e-transfer for students with no per-student
override, rather than something inferred from whether keys exist. Card
remains the default, so a site that adds keys and changes nothing else
behaves as before. BillingMethodResolver still degrades a card default to
e-transfer while Stripe is unconfigured — there is nothing to charge a
card with — and `comp` is deliberately not offerable studio-wide, since
it would silently stop billing everybody; an unrecognised stored value
reads back as card. Holding the default on e-transfer with Stripe live is
the staged-rollout path: move individual students to card on their detail
page, watch real charges land, then flip the studio over.
Clear Stripe configuration deletes the publishable key, secret key and
webhook signing secret and returns the mode to test, so a re-configuration
later cannot inherit live. Currency, HST, e-transfer and registration
settings are untouched, as are recorded payments. The button only appears
when some Stripe value is stored, and reuses the page's existing nonce and
`manage_billing` check.
`composer test` (915), `composer lint` and `composer cs` all pass. Options
only — no schema change, so no version bump.
Closes#173
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01WyktWmwNRgMYuwe5eBuPZm
The group-class page matched enrolments to the account instead of to the
student: the lookup it built from GET /enrollments was keyed by offering id
alone, so the first household enrolment marked the class as "yours" and took
the Enrol button away from everyone else on the account. A parent could enrol
one child and was then offered nothing but Withdraw.
The server was never the constraint — hasActiveEnrollment() checks the
(offering, student) pair and GET /enrollments deliberately returns the whole
household — so the fix is to stop discarding student_id on the way in. Active
enrolments are now grouped per class as a list, each student gets their own
"… is enrolled in this class." line and their own named Withdraw button, and
the Enrol button stays (as "Enrol another student") while anyone the account
may enrol is still out.
The enrolment form offers only the students not yet enrolled. When exactly one
of them is left the picker collapses, and that case needed care: an omitted
student_id reads as "enrol the account holder" server-side, so a hidden field
carries the id rather than posting nothing and signing up the parent instead of
the last child.
Closes#170
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Jy9UPhmpLUAfimsyecHN2Z
A weekly booking reserves a series of lessons, but the student answers
the intake and ticks the policy boxes once — so BookingEndpoint records
both against the anchor lesson alone. The admin detail view looked them
up by whichever lesson id was being viewed, so every occurrence after
the first showed no answers and no acceptances at all.
LessonDetail now takes the Lesson rather than a bare id and resolves the
registration to `series_id ?? id`, so each occurrence reads the anchor's
records. This is the same seam PaymentService already uses to find a
series lesson's payment on the anchor.
Nothing was ever missing from the database, so existing bookings read
correctly with no migration and no schema change.
Closes#167
Co-Authored-By: Claude Opus 5 <[email protected]>
The Profile block is headed "Your profile", but the one person on it you
could not change was yourself: your name, your birth year, and whether you
take lessons yourself were fixed at whatever signup recorded, and correcting
any of them meant asking a studio admin.
A "Your details" section now opens the page, saved through the same
nonce-checked template_redirect post/redirect/get path the child rows use:
- Your name, written to display_name and nickname together, for the reason
updateChild() does — UserName reads the nickname first, and leaving it
behind would put the account's email address back on every screen that
names a person.
- "I take lessons myself", the positive of us_guardian_only. This makes good
on the claim already in bookableStudents() and the feature doc that a
guardian-only account can put itself right from the profile page.
- Your birth year, held to the same normaliseBirthYear() rule as every other
student.
The email is shown but not editable: it is the account's user_login as well
as its address, so changing it stays a studio-side job.
The birth-year field deliberately carries no `required` attribute. It is
asked of a student only, and this page loads no JavaScript, so a
browser-enforced `required` would leave a guardian who books solely for
other people unable to submit the form at all; handleSelf() enforces it
against the checkbox instead. Unticking the box does not clear a stored
birth year — it says who books, not "forget what is on file".
Closes#165
Co-Authored-By: Claude Opus 5 <[email protected]>
Every account-signup question was asked of everybody who registered, on the
same terms: "school and grade" had to be put to an adult signing themselves
up, and a question a studio needed answered for each student could only be
made required by demanding it of everyone.
A question now carries an audience — everyone, or only the students someone
registers on behalf of — and its own required flag for each side, so optional
for you and required for every student you enrol is expressible. Both settings
are account-scope only: an offering asks its questions once, about the student
being booked, so there is no second audience to differ from, and an offering
question mirrors its single "required" into both columns.
Every caller reads askedOfSelf()/isRequiredForSelf()/isRequiredForChild()
rather than the raw flags, so a students-only question can neither block the
account holder nor have an answer filed against them by a crafted post. The
family screen, which only ever adds a student, is held to the students' rule.
is_required_child arrives from dbDelta defaulting to 0, which would quietly
stop every existing required question being required of the students a
guardian registers — the case it most likely existed for. A one-time backfill
copies is_required across, guarded by its own option so a question later made
optional for students stays that way.
Closes#163
Co-Authored-By: Claude Opus 5 <[email protected]>
Anywhere the plugin named a person it could show their email instead —
"Managed by [email protected]" in the students table, the same under
Booked by, instructor names on the class pages.
WordPress defaults a new account's `nickname` to its `user_login`, and
signup uses the email address as the login. So every self-registered
account carried its own address as its nickname, and UserName::format()
fell straight through to it. The name they typed was in `display_name`
all along. Accounts created by a guardian were never affected —
GuardianService::createChild() sets `nickname` outright, which is exactly
why children read correctly and their parents did not.
UserName::format() now walks nickname then display name, skipping either
when it is really the login or the email, so existing accounts read
correctly with nothing to migrate. An identifier still never reaches the
screen: an account with nothing but its address on file falls back to the
id, as before. Signup also sets `nickname` at insert, so new accounts are
right at the source rather than relying on the fallback.
Tests: composer test (866), composer lint, composer cs all pass.
Co-Authored-By: Claude Opus 5 <[email protected]>
The parent/guardian was only named further down under Profile, where it
reads as background rather than as an account fact, and only when there
was one — so a page with no such line was ambiguous between "books for
themselves" and "the lookup found nothing".
It now sits in the Account table beside display name and email, as the
guardian's name linked to their own detail page, and always renders: a
student who books for themselves says so outright. No email address —
theirs is one click away on their own page, and repeating it here only
makes the row harder to scan. The Profile section keeps only the note
explaining the placeholder email, which is a different point.
Tests: composer test (863), composer lint, composer cs all pass.
Co-Authored-By: Claude Opus 5 <[email protected]>
Three fixes from testing the branch.
A group class was only listed when its schedule resolved to exact
datetimes, which needs a class time *and* a duration — both optional on
the offering form, and the schedule note exists precisely so a studio can
write "Tuesdays 4:00pm" instead. A class configured that way vanished
from the list, which is the one thing this feature must never do. So
Offering::sessionStarts() splits "when does it meet" from "how long does
it run" (sessionWindows() is that plus the duration, unchanged), and
SessionSchedule degrades instead of disappearing: dated rows with an open
end when there is no duration, and a single row carrying
Offering::scheduleLabel() when there is no time to derive dates from.
Only a class whose last day has passed drops out.
Deleting a guardian now deletes the children linked to them, releasing
each one's lessons and enrolments first. A child account is login-less
and exists only so the guardian has somebody to book for; without the
guardian nobody can reach it, book for it, or be billed for it, so it was
left stranded on the roster still holding slots. A `handled` set makes
the re-entrant delete_user each child deletion fires a no-op, and stops a
circular link recursing.
The upcoming panel never stated its own line-height, so a theme setting
line-height: 0 above it — the usual icon-font reset — was inherited
straight through. Below 1 that produces both reported symptoms at once:
stacked lines overlap, and the status pill's background is shorter than
the text in it. Pinned at the same id-level specificity as the rest.
Tests: composer test (863), composer lint, composer cs all pass.
Co-Authored-By: Claude Opus 5 <[email protected]>
Five items from the latest demo pass:
- A policy's title can be edited from the Policies screen. Only the title
moves; the slug is what the gates resolve policies by, so a rename can
never detach a policy from acceptances already recorded against it.
- Signup is one page again. The studio's registration questions move from
a second step behind "Next" onto the main form, in an "About you" panel
above the students being added, and that panel also asks an adult
student for their birth year (the same us_birth_year meta a child's
uses). register.js disables and hides the whole panel for a pure
guardian, since the questions describe a student.
- The password is re-scored on submit, not only as it is typed. zxcvbn's
dictionary arrives after page load, so a password typed straight away
was never scored at all and the first the student heard of it was the
server rejecting the whole form.
- Group-class sessions appear alongside lessons wherever upcoming lessons
are listed: the [us_scheduler] panel (students and instructors) and the
admin student detail page. GroupClass\SessionSchedule derives them from
Offering::sessionWindows(), the same derivation the billing scan uses.
They carry kind = 'group_class' and no Cancel action - a session is one
date in a term, not a booked slot.
- Deleting a user releases what the account was holding: each upcoming
lesson is cancelled, its slot freed for rebooking, its pending payment
voided, and active class enrolments cancelled. Past lessons and paid
history are left alone.
Tests: composer test (851), composer lint, composer cs all pass.
Co-Authored-By: Claude Opus 5 <[email protected]>
thatguygriffandClaude Opus 5
Release Bot