Back to blog
laravel permissions ci github-actions migrations architecture testing

A whole sidebar group vanished, and it took a permission cascade that never fired to explain it

The Starting Point

Big day. The plan was to pull two concepts out of the delivery module into packages of their own, get the split repositories live, then build finance/rates/effort and status reporting.

What actually happened is I spent a chunk of it chasing a sidebar group that wasn't there.

What Actually Happened

Two modules moved out, and one of them was worse than the source

The rule I landed on, stated once and now applied twice: when a concept is needed by more than one domain, it becomes its own package depending only on core, and every module that wants to attach it owns its own pivot. The shared package never learns who uses it.

Work requests first — a job sent to someone outside the platform with a checklist and a decision. Nothing about that is a delivery concept, and the tell was that the source's own template controller for it already lived outside its delivery module. It was filed under the first module that used it, not designed as part of it.

Then approvals, one module later. And here's the bit I'm least proud of: my first port of approvals was worse than the source. The source had a polymorphic owner morph. I "fixed" the polymorphism by narrowing it to a single foreign key column pointing at a project. Which removed the morph and kept the coupling — the worst of both. The approvals package now carries no owner column at all, and there's a test asserting that four specific columns stay absent, so the independence is enforced rather than intended.

The cost is recorded rather than hidden: raising an approval is now two steps where it was one. A shortcut action would restore it, and I'm explicitly not building that until a second linking module exists to prove the shape.

Migration ordering bit me here. The pivot was first timestamped to match the work-requests package's own first migration. Laravel sorts every pending migration by filename across all paths — it's global, not per package. So it ran before the table it points at:

SQLSTATE[HY000]: General error: 1824 Failed to open the referenced table 'work_requests'
SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'project_work_requests' already exists

The second one is the fun part: the create succeeded, the foreign key failed, and the orphan table stayed behind to block the retry. Neither package suite could see this — each migrates its own package alone. Only the assembled app catches it. Retimestamped with the reason written into the migration file.

The missing sidebar group

Both new modules: installed, enabled, migrated, routed. And completely invisible in the navigation. To every user. Including the vendor administrator, who holds every permission by construction. No error anywhere.

The cause is a genuinely nasty piece of wiring:

EnableModule created the permission rows itself, via PermissionRegistry::sync(). That call reports which permissions are new. And the role catalogue treats that report as the sole trigger for granting role defaults.

So creating the permissions anywhere else consumed their newness. By the time the catalogue ran, it found nothing to cascade. The permissions existed, belonged to no role, and never would. Navigation then filtered every item away, correctly, because nobody could see them.

The decision record had promised the exact opposite six weeks earlier — "installing a module cascades its defaults into roles that already exist". The intent was documented and the wiring wasn't there.

Fixed with a narrow RoleCatalogue::grantNewPermissions() — creation and cascade in one step. Deliberately not the full ensure(), which also creates the baseline roles: enabling a module must not conjure roles a deployment chose not to have. Routing through ensure() broke a round-trip test that holds disable/enable to leaving no trace, and the narrow method keeps it.

The regression test was checked the only way worth trusting: it fails with the old call restored and passes with the fix. Its first version asserted something the design deliberately refuses — re-granting a permission an administrator had revoked — so I rewrote it to the real case.

There's a note for the first real deployment too: any environment where a module was enabled before this fix carries the same orphaned permissions, and the repair is to delete the ungranted rows and re-run the admin sync.

Three packages had never run in CI

While reading the check list on a PR I noticed which legs were missing. The test and PHPStan matrices still listed the four packages that existed when they were written. The delivery module was never added when it was built, and both new modules inherited the gap.

Three packages' suites and static analysis had only ever run on my machine.

Not caught by anything failing. Caught by reading a list. A package absent from a matrix is green by omission, which looks exactly like green.

GitHub returns 404 for two completely different things

The split workflow has a guard that refuses to run if a target repository is missing, redirects, or resolves to the monorepo. The three new package repos got created, the split ran, and the guard reported them as redirecting to a JSON error object — which sends you hunting for a rename that never happened.

The actual situation: a 404 from the repository API means either "no such repository" or "this token cannot see it". GitHub returns the same code for both, deliberately, so a token can't enumerate private repos. The guard was reading only the response body.

And that couldn't have been checked in advance. A fine-grained token lists its repositories, and adding a package to the CI matrix does not add the repository to the token. The three legs failing first was the only way the question could be settled. I added workflow_dispatch so it's provable on demand rather than discovered at the next merge.

Two bugs only the tests could see

Both in status reporting, and neither visible by reading the code:

Ties in sort_order left a report's lines unordered. So a report could read differently between two page loads, and differently again once carried forward to the next month. Ordered by id as a tiebreak.

Milestone seeding walked $project->descendants(), which reads in-memory relations. So a plan edited after the project was fetched reported its old dates — precisely what that section of the report exists to show. Now queried from the database, iteratively so it can't recurse into a cycle either.

Also hit this one repeatedly while building finance lines:

SQLSTATE[23000]: NOT NULL constraint failed: project_finance_entries.total_excl_vat

That's from my first attempt at read-only derived fields. I removed the field from the payload alone — but the form's _fields[] list still named the column, and the writer reads presence-in-that-list as a deliberate clearing. Removing an input from a page only moves the handle. readOnly() is now enforced in the writer.

What Worked

Modules declaring their own settings. A setting I'd shipped an hour earlier could only be changed from tinker — the same failure this phase keeps finding: mechanism built and tested, surface missing.

The fix is not a per-module admin tab. The legacy platform grew a settings controller per module and that's documented in the survey as a thing that went badly. So a module declares SettingDefinitions and core renders, validates and stores — same shape as permissions, navigation and record types. Validation rules are named by the module, because only the module knows what would break. The delivery module requires its working-day length between 0.5 and 24 hours: a zero divides every day rate by nothing and makes the entire system free.

Two things found by tests rather than review, both of which the next module with a namespaced key would rediscover:

  • PHP mangles a dot in a form field name into an underscore. So inputs are settings[the.key] — the mangling only applies before the bracket.
  • Laravel's validator reads a dot as nested-array access, so the dot has to be escaped in the rule key.

Then I folded define() into register(), so a module describes a setting in exactly one place. Strengthening a test to actually prove its own name is what found the cost of two calls: a module that registered a panel without defining the setting rendered a control and then threw on save.

That test deserves its own paragraph. It claimed "only the open panel is rendered" — while only one module had settings. It would have passed either way. It now registers a second panel and proves both halves. A test that can't fail is documentation with a green tick.

Rate snapshots. A rate is copied onto the effort record at the moment it's recorded — name, unit, amount, chargeable — so a card corrected in March cannot reprice work recorded in January, and a rate deleted outright leaves old effort perfectly readable. Same rule as a plan version's snapshot and a work request's copied checklist. Three different features, one principle.

One deliberate divergence from the source: its 8-hour day is a constant. A client on 7.5 hours would have had every normalised figure and every cost quietly wrong. It's a setting here, and a zero falls back rather than dividing by it.

Totals moved from the service to a model saving hook. They were computed in a costing service until it became obvious the record framework writes that table too — through the generic form and through an import. A line created either way had no totals at all. Now it's derived where it can't be skipped.

Splitting "reporting" into two things. The name invited the wrong decision. The source's generic report builder and what the delivery module calls reporting — a monthly highlight report for a steering group — are different things. The source keeps them apart on purpose; its own migration comments say the table prefix exists to avoid colliding with the builder's tables.

So: the delivery module owns the capture (the stored report, frozen figures, authored narrative). The reporting module will own the view (templates, widgets, layout, export). Core's existing datasource contract already assumed that arrangement, so the seam was anticipated rather than invented.

Three rules do the work:

  • The first report seeds from live data.
  • Later months carry the previous report forward for editing rather than re-extracting, because a steering group reads the same lines month to month with the changes marked.
  • The narrative sections are never seeded and never overwritten. They're somebody's words. refreshSection() refuses them outright rather than quietly doing nothing — which from the outside would look like a feature that had failed.

Also collapsed six near-identical section tables into one with a section column. Same finding as the seven-types-sharing-seventeen-columns thing from a couple of days ago. But sections stay structural rather than client-editable, because the seeding logic branches on them — and "nothing may branch on it" is the exact condition for whether a list belongs to the client.

What I Learned

  • A function that reports "what's new" as a side effect is a terrible trigger for anything important. Anyone who creates those rows elsewhere silently consumes the trigger. The permissions existed, belonged to no role, and would never have been granted.
  • A green CI run tells you nothing about packages that aren't in the matrix. Green by omission looks identical to green. Read the check list, not the badge.
  • GitHub returns 404 for "doesn't exist" and "your token can't see it" alike. If your tooling distinguishes those, it's guessing. And you can't verify token coverage in advance — you can only try it.
  • Laravel sorts pending migrations by filename globally, not per package path. A cross-package pivot needs a timestamp that orders after the table it references, and no single-package suite can catch it.
  • A failed migration can leave a table behind that blocks the retry. The create succeeded; only the FK failed.
  • PHP turns dots in form field names into underscores — but only before the first bracket. settings[a.b] survives; settings.a.b does not.
  • Strengthen a test until it can actually fail. Mine passed because only one module had settings. It was asserting nothing.
  • Removing an input from a form doesn't make a field read-only, it just moves where the payload comes from. Enforce it in the writer.
  • In-memory relations are stale by definition. $model->descendants() after an edit reports what you fetched, not what's true.
  • Snapshot-at-the-moment-of-record shows up everywhere once you start looking: rates, plan baselines, checklists, report figures. Same rule every time.

Tech Notes

  • Stack: PHP 8.4, Laravel, MySQL (Docker), SQLite in-memory (suites), GitHub Actions, PHPStan L6
  • New packages: work-requests and approvals, both depending on core alone, both with a test asserting no owner/foreign-key columns exist
  • Settings API: modules declare SettingDefinitions (key, type, validation rules, default) in the same call that registers the panel; core renders, validates and stores. Panels are tabs via ?module=, so deep links and history work with no JavaScript, and only the open panel is built.
  • CI: added three missing packages to the test and PHPStan matrices; added workflow_dispatch to the split workflow so token coverage is provable on demand
  • Ended at seven packages split to their own read-only repos, ~584+ tests, nothing blocking
# repair for any environment enabled before the cascade fix:
# delete the ungranted permission rows, then re-run the admin sync command
Zero Focus Dev

Zero Focus Dev

Breaking things, learning fast, sharing it all.