Skip to content

17 August 2026

9 changes:

  • Two seams that were already half there
  • The docs shipped in vendor/ and nothing ever offered them
  • A correct header, a correct footer, and nothing between them
  • A guard for a null that could not happen
  • A stale @todo, and two classes that do not exist
  • Being told no
  • The suite is finished, and the answer to the last question changed
  • A form class that was never ported
  • Sixty-seven messages nobody could see

Two seams that were already half there

Both requested by a hybrid application, and both pointed at something the framework had already anticipated and not finished: seal() has a slot for an issued token and nothing issued one, and seal() models absence but cannot model presence-without-an-account.

SessionExchange — the browser is signed in; give it a token

The symptom, reported verbatim: "I am signed in on the site; if I leave it a while and then open the panel, it asks me to log in again." A session-authenticated site and a token-authenticated SPA on one origin, two credentials, two lifetimes. The site knows exactly who the visitor is. The panel has no way to ask.

UnifiedAuthMiddleware solves the other direction — an API endpoint accepting a cookie plus a CSRF token — and the report was right to refuse it. Adopting it means the API authenticates with cookies, which quietly invalidates every decision an application made because it does not. Their permissive CORS default was the example, and it was introduced a long way from where it would have broken.

So: an exchange. One direction, one moment, and the API still never reads a cookie.

$token = SessionExchange::issue(minimumUserType: 90, ttl: 43200);
return Response::redirect(SessionExchange::redirectUrl(sURL . 'panel/', $token));

Only a session may be exchanged

Written first without this check, and the omission is worth naming because it was invisible in review: User::getCurrentUser() prefers a sealed identity over the session, so an API request carrying a bearer token reached the minting path and received a fresh token.

That is a refresh — a credential extending itself, with rotation and revocation questions this method answers neither of and never claimed to. Every twelve-hour token good for another twelve hours on request, forever, from a method documented as exchanging a session.

issue() now refuses any request whose identity was sealed by something other than a session. Identified positively rather than by excluding token-ish via values: a blocklist would have to enumerate every credential that is not a session, which is unbounded, and that exact shape produced a separate defect elsewhere in the framework the same week.

It could not issue a token at all

Found while writing the tests that were missing, and it is the reason they were worth writing rather than backfilling for the coverage number.

issue() read its signing key from $app->authenticationKey. That property is declared on Api — which computes it in its constructor — and not on Application. This method is called from a session-authenticated MVC route by definition, because that is what a session is, so the property was absent on every real call and the exchange silently issued nothing. Its own docblock said "no signing key is configured", which reads as a deployment problem rather than as a lookup that could never succeed.

The derivation is now a single named method, Api::deriveAuthenticationKey(), used by both Api's constructor and the exchange. One value, one place: a token signed with a key the verifier does not derive fails as an authentication error arbitrarily far from its cause, and two copies of a derivation are two things to keep in step.

With one case deliberately left refusing: no declared key and no sURL. The derivation then reduces to md5('edge') — a constant every installation in that state would share, so a token minted by any of them would verify against all of them. That is not a weak key, it is no key, and refusing to mint under it is the only honest answer. A real request always has sURL.

Two smaller things the same tests found: Application::getInstance() was being called in three places, and it is a factory — given no instance it reads app.php and runs the whole constructor, database, language and session included. The framework's own docblock on currentInstance() states the rule and names the incident behind it: a CSRF fingerprint check that booted an application was a side effect in the middle of a security decision. Minting a bearer token is that same kind of code. And a session claiming user id 0 or 1 — the guest, by this framework's convention — is now refused explicitly rather than by luck.

Four decisions, three of them invisible when wrong

That framing is the reporter's and it is the reason this belongs in the framework rather than in each application:

  • The role is re-read from the database, not taken from the session. A remember-me cookie can outlive a demotion by a fortnight, and a token minted from that session is then good for its whole lifetime afterwards.
  • The token travels in the URL fragment. A fragment is never sent to a server: no access log, no Referer, no proxy in between. ?token= works, reviews identically, and writes the credential into the log of every hop for as long as logs are kept.
  • Nothing is issued for an anonymous caller — no implicit token, no partial credential.
  • Failure is null, because the caller is a route that has to redirect somewhere either way.

The claim set matches the API login's deliberately, so an exchanged token is indistinguishable to every verifier. A second shape of token would be a second thing to keep in step, and the one that is not exercised is the one that rots.

The fifth decision stays with the consumer and is documented rather than taken: an SPA that bounces to the exchange route when it has no token must record the bounce before redirecting. The route redirects back without a fragment when it cannot help, so a flag written afterwards is an infinite loop — on the one page an operator opens when something is already wrong.

RequestIdentity had two states and needed three

public static function seal(?object $user, …)

A user, or null for anonymous. Right for an API, where anonymous means no identity at all. Not enough for an application whose unauthenticated callers are people: a chat participant with a nickname and a session, present in a room, mutable, bannable, addressable, and the same person across requests for as long as they stay. They are not nobody. They are not an account either.

An application in that position keeps a second, parallel notion of who the caller is — and then every consumer asking "who is this" has to know which of two mechanisms to ask, with a convention between them instead of a type. The framework was what forced that: the seam admitted one of the two shapes and the application carried the other.

RequestIdentity::sealGuest($presenceId, 'presence');

RequestIdentity::isGuest();    // true
RequestIdentity::user();       // null
RequestIdentity::subject();    // the id, whichever kind of identity this is

subject() is the point. One question, one answer, three states — an account's id, a guest's id, or null for a request that is genuinely nobody.

The asymmetry is the security-relevant part

A guest never replaces an account; the call is refused and logged. An account does replace a guest, because that is a real login.

Symmetry would be the bug. A middleware that seals a guest unconditionally, ordered after the one that authenticates, would demote the caller — and every permission check after it would answer for the wrong person while the request looked entirely healthy. There is a test for each direction, and they are the two worth having.

An empty id is refused too: every such guest would be indistinguishable, so a mute, a ban or a rate limit keyed on it would apply to all of them at once.

user() still returns null for a guest, and that is deliberate — code asking for a user must not be handed something that merely resembles one. It is why isGuest() is a separate question rather than user() returning a shape.

What neither of these decides

Which identity model an application should use. The reporting application has chosen one and it stays theirs. Both additions are mechanism for an answer the framework already half-expressed — which is why the requests were easy to agree with: each named the place where the existing design stopped short of its own intent.

Added

  • Pramnos\Auth\SessionExchange::issue() and ::redirectUrl().
  • RequestIdentity::sealGuest(), isGuest(), guestId() and subject().

Documentation

  • Authentication guide — Handing the browser's user an API token, and Requests that are somebody without being an account.

The docs shipped in vendor/ and nothing ever offered them

docs/ is not export-ignored. Every guide travels inside the composer package and sits in vendor/pramnos/framework/docs/ of every project, and the stated reason is explicit: the documentation should be available to whoever is working there — an AI assistant included — and the vendored docs always match the vendored code, so there is no version to negotiate.

The MCP server has shipped since v1.2 with five tools and three resources. The five tools introspect the application: tables, schema, migrations, models, routes. The three resources are the application's own CLAUDE.md, README.md and app/app.php.

None of them touches docs/. So the only route from an assistant to a guide was to guess that it should look inside vendor/ — which is the failure the documentation rules were written after, not a hypothetical one. A feature was documented, present in the vendored corpus, not found, and built a second time beside the working copy.

Added

framework-docs, a sixth MCP tool

The odd one out among the six: it takes no application and needs no database, because it is the same answer in every project.

{}                                                    // the index — every guide, and the task each covers
{"query": "issue an API token for a signed-in browser"}
{"page": "Pramnos_Authentication_Guide"}              // read one in full
{"corpus": "changelog", "query": "session exchange"}  // when it changed, rather than how it works

Registered by McpServiceProvider, and by mcp:serve's fallback server — there, deliberately outside the if ($app !== null) guard the other five sit inside. There is nothing for a missing application to make unanswerable, and a server booting without one is exactly when somebody is asking how any of this is supposed to work.

Ranking follows the corpus convention. Every guide carries use_cases: frontmatter phrased as the task the reader has in hand — "Adding a column to an existing table", not "Schema builder reference". Those are the closest thing here to the question an assistant arrives with, so a hit there outweighs a heading, which outweighs the body. Body matches still count, because a question about a specific method name appears in no use case; they are worth less.

A page with no use cases is demoted, because it is not guidance. This was measured rather than assumed, and the measurement found the bug: the very first query ranked 1.2-new-features — a deliberately frozen v1.2 reference, and one of the two longest files in the corpus — above every live guide, on body volume alone. Sending a reader to a page that stopped describing current state on purpose is the one outcome this tool exists to prevent. The rule is structural rather than a list of names, so a page cannot become quietly exempt by being added later.

The changelog is a separate corpus and is never merged with the guides. There are far more posts than guides, and each post repeats the vocabulary of the change it describes. Merged, "how does this work" would be answered by three dated fragments of a feature's history — precisely what the guide/changelog split exists to prevent, arriving as a ranking accident instead of as a decision.

A page name is reduced with basename() before it is resolved. The name arrives from a model, which is a caller that can be talked into asking for anything, and app/app.php — database credentials and the authentication key — sits two directories above the guides in exactly the layout the default path produces.

Documentation

A guide now owns the MCP server

MCP was documented only in 1.2-new-features.md, which is frozen, and in a few dated posts. That is the same shape as the failure above, one level up: the mechanism built to make documentation findable was itself only findable by knowing where to look.

MCP server is now the page that owns the topic — enabling it, all six tools, the resources, how to add your own, the protocol methods handled, and the console-kernel trap that made route-list answer {"error": "No router available"} on the only path that could reach it.

The roadmap entry it closes, and the half it does not

Roadmap.md already asked for this, in three parts. Two are now built — the docs corpus and the changelog kept separate from it. The third is not, and it is the one with the evidence behind it: a pramnos_check tool that says no when a documented rule is broken. Raw SQL where the query builder belongs, unqualified authserver.* tables, flash messages passed as query params, view variables that collide with the View engine, migrations prefixed 2020_01_01_*, a hand-rolled debug panel beside the framework's own.

Every item on that list is something that happened after the guide describing it was written. Being able to look a rule up and being told when you have broken it are different mechanisms, and only the second has a track record. The entry has been narrowed to that.

Two reports from a project adopting app/themes/ for the first time. The guide was good on what a theme is — theme.html.php, [MODULE], the partials, the override hierarchy — and silent on how a page's content reaches the document. Neither of these is guessable, and both produce a page that looks like the theme is working and the content is missing.

Fixed

The two renderers read the content from two different places

// Document::render()
$content .= $this->content;          // the public property

// DocumentTypes\Html::render() — the one actually serving a page
$content .= self::_getContent();     // a static buffer

Setting $document->content is the obvious move: the property is public, it is what the parent class reads, and it looks exactly like the seam. On an HTML page it produced a correct header, a correct footer, and nothing between them — with no error anywhere.

The report framed this as Html disagreeing with its parent. Reading the other five types first turned it round: Html, Amp, Json, Png and Raw all read the buffer, and only Document::render() read the property — which in practice serves Rss. Fixing Html alone, as suggested, would have left the identical trap in four more types.

So the reconciliation is one shared resolver, Document::bodyContent(), used by all six: the buffer when it holds anything, the property otherwise. That direction is what makes it a repair rather than a behaviour change — every page that renders today renders from the buffer, so the only output that changes is output that was blank.

Worth stating since the report named _setContent() as "a static method with no mention in the guide": the instance API exists too. $document->setContent(), addContent() and getContent() are ordinary public methods, and they were equally undocumented. They are what the framework itself uses, and what the guide now points at.

A theme object that had not read its theme

public function loadtheme($theme = 'default', $path = '', $application = null)
{
    $themeobject = Theme::getTheme($theme, $path, false, $application);
    //                                            ^ $load
}

With $load = false, Theme::loadtheme() never ran, $body stayed empty, and gethead() / getfoot() split an empty string. Html::render() calls loadTheme() itself, which is exactly why this is invisible from inside the framework and obvious from outside it: the framework's own path works, and an application that assigns themeObject and renders through any other route gets an object that reports no error and produces the bare default.

The report asked for a line in the guide saying the body is loaded lazily. Making that sentence true was cheaper than writing it: gethead(), getfoot() and getheader() now read the theme file when nothing has read it yet.

Two boundaries on that, both of which an existing test or a plausible caller cares about:

  • the condition is nothing read yet — both contents and body — not merely an empty contents. A caller that assigns body itself has supplied the very thing the load would produce, and reading over it would discard a deliberate value. An existing test in this suite does exactly that, and failed immediately against the narrower condition.
  • an explicit loadtheme() still re-reads. The file it picks depends on the content type, so setting a content type and reloading is how a theme switches templates; memoising would silently ignore that.

Documentation

The Theme Guide gains How a page's content reaches [MODULE] and When the theme body is loaded — the setContent() / addContent() API, the fact that the buffer is static and therefore shared by every document in the process (which matters to a long-running worker and to a forgetful test), the property that now works and why it used to be a trap, and the two boundaries on the lazy load.

Three use_cases: entries were added with the symptom in them, not the mechanism — "Diagnosing a theme that renders a header and footer with an empty page" — because that is what somebody hitting this actually types.

A guard for a null that could not happen

Application::getInstance() is a factory. Given no instance for the key it reads app.php, defines constants and runs the whole constructor — database, language, session. currentInstance() is the lookup, returning null instead.

The rule was already written down twice: in the Framework Guide, and in currentInstance()'s own docblock together with the incident behind it — Session::getFingerprint() began asking for the trusted-proxy list, and a reference application's login tests started failing on valid tokens because a second application was being constructed underneath them.

Nine call sites were using the factory anyway, including the two places where it does the most damage: the identity lookup, and the database layer's own error reporting.

Fixed

The nine, now looking up rather than building:

File What it wanted
User/User.php — getCurrentUser() to answer who is signed in
User/User.php — getUser() a namespace, to pick a class name
User/User.php — legacyMd5Allowed() one boolean setting
Database/Database.php — displayError() somewhere to report a database error
Http/Middleware/ApiAuthMiddleware.php to record the authenticated user
Http/Middleware/UnifiedAuthMiddleware.php the same
Auth/Drivers/DatabaseAuthDriver.php two boolean settings
Auth/Controllers/ApiAccount.php (×2) to record a login, and clear it on logout

The two in bold are the ones worth naming individually.

User::getCurrentUser() is the worst placement in the framework: asking who is signed in constructed an entire application — database, language, session — which is precisely the shape of the incident quoted in currentInstance()'s own docblock.

Database::displayError() was self-defeating. Building an application builds Settings, which queries the database — the connection that just failed. It also made the method's else branch unreachable, so the error_log() fallback written for "no application" could never run and a database error outside a request went nowhere at all. Same class of cycle ConnectionPathPurityTest guards on the connect path, one step further along: not while opening the connection, but while complaining about it.

Every one of them was already written as if ($app) — a guard for a null the factory cannot return. So the guard was dead and the construction was live, and the source had been saying so the whole time. Nothing here is a behaviour change in a real request, where the application exists and the two calls give the identical answer; what goes away is an application, a database connection and a session being built as a side effect of writing one property, in the middle of an authentication decision.

The first of these was found and fixed in SessionExchange earlier the same day. This is the rest of the pattern, from reading for it rather than waiting for it.

Added

Two tests, because they catch different things and only one of them is durable.

NoApplicationIsConstructedTest is behavioural: with the application registry emptied, each path does its work and currentInstance() is still null afterwards. It also asserts the converse — that an application which does exist is still found and used — because otherwise deleting the reads entirely would pass.

ApplicationFactoryPurityTest is structural, and it is the one that matters: no file under src/Pramnos/Auth/, src/Pramnos/Http/Middleware/, src/Pramnos/User/ or src/Pramnos/Database/ may call Application::getInstance(). The behavioural test proves the known sites; this is what stops the next one, which is the real failure mode, because getInstance() is the name one remembers. Same technique as ConnectionPathPurityTest, which guards the connection path — this is the wider rule that path is one instance of.

Three details it inherits from mistakes already made in this repository:

  • it asserts that it scanned a non-trivial number of files, and names two of them. A wrong dirname() depth produces an empty scan, and an empty scan satisfies "nothing calls the factory" perfectly. A structural guard here once did exactly that and passed.
  • it strips comments with token_get_all() before matching, so a file may explain the rule without violating it — this post's own subject matter would otherwise be unwritable in a docblock.
  • exemptions are enumerated and currently empty. A file cannot become exempt by being added; somebody has to write its name and the reason, which is the conversation the exemption is for.

Worth knowing if you convert a call site yourself

currentInstance() declares ?Application; getInstance() declares no return type and returned whatever the registry held. Three integration tests installed a plain stdClass as a fake application, which had always worked, and started failing with a TypeError the moment the authentication code moved to the lookup — eight tests across two drivers.

That is the correct type being enforced rather than a regression: the registry is meant to hold applications. But it is the first thing a conversion surfaces, and the fix is a real subclass with an empty constructor rather than a loosened signature.

Not changed

Api::deriveAuthenticationKey() still derives md5('edge') when sURL is undefined — a constant every installation in that state would share. Confirmed by the framework's author as a non-issue: sURL is required for the system to function at all, so no such setup exists. SessionExchange refuses to mint under it regardless, since refusing costs nothing.

Twenty-seven Application::getInstance() calls remain outside the guarded directories, and most are legitimate: console commands, the scaffolder's generated templates, and the document types, all of which run where wanting an application is the point.

Not audited one by one. The guard covers the directories where the factory is a hazard rather than a choice; anywhere else, a call that builds an application is doing what its caller asked.

A stale @todo, and two classes that do not exist

A consumer adopting app/themes/ reported an empty <body > tag and a @todo Use bodyclasses sitting next to a method that collects body classes — reasonably concluding the framework gathered them and never printed them.

It prints them. Html::render() and Amp::render() have both emitted the list all along; the @todo was stale. That is the defect: a note describing finished work is read as a statement about the present, and this one sent somebody looking for the missing half of a complete feature. It is gone.

Checking the surrounding lines found three things that were real.

Fixed

Body classes were not escaped

Every value in <head> has been escaped since a consumer reported station names and administrator text ending attributes early. The body class list was missed in that pass, because it looked only at head values — and it is the same defect with a wider blast radius: addBodyClass() is reasonably fed a slug, a content type, or a user's chosen theme name, and a " in any of those closes the class attribute and adds an event handler to <body>. A body class is usually set once for a whole layout, so that is every page.

extraBodyTag stays raw, deliberately — it is documented as carrying markup — and now has a test whose job is to stop a well-meaning future change from "fixing" it.

Two smaller things in the same lines: the tag is now <body> rather than <body > when there is nothing to add, and the variable that joined the classes was named $comma while holding a space, which is enough to make a reader check whether the framework was emitting class="a,b".

Amp::render() could not build a canonical

\pramnos_request::$originalRequestNoChange

A legacy CMS class name, which from a namespaced file resolves to nothing. It sits in the branch that builds a canonical when the document has none — so every AMP page that did not set one explicitly died on Class "pramnos_request" not found, which is precisely the case the branch exists to handle.

Theme::saveSettings() could never have run

pramnos_settings::setSetting(…)

The same shape. A theme's settings form could be rendered and never stored. Its @return string was wrong too — it returns nothing, and never did.

Both had a modern equivalent with the identical member name, one namespace away.

Added

LegacyClassReferenceTest guards the class of error rather than the two instances: no file under src/ may name a pramnos_* class. Three have now been found — this pair plus pramnos_theme::getTheme() in Theme::getThemeObjects(), fixed on 14 August — and each one survived because it sat on a branch nothing exercised, which is exactly what behavioural tests do not reach.

It matches a class reference (pramnos_x::, new pramnos_x, instanceof pramnos_x) rather than the string pramnos_, which appears legitimately in table prefixes and cache namespaces throughout; it strips comments so the framework can document these names; and it asserts that its own matcher detects all three historical forms, because on a clean tree "no offenders" is indistinguishable from a pattern that matches nothing.

LegacyFatalsFixedTest is the behavioural companion: it executes the two branches that used to fatal. Both were verified by reverting the fix and confirming the exact original errors — Class "pramnos_request" not found and Class "Pramnos\Theme\pramnos_settings" not found.

Reported, not changed

Nothing in Theme assigns $_form. The whole theme-settings API — addSetting(), hasSettings(), getSetting(), renderSettingsForm(), saveSettings() — calls into that property, and a base Theme has it as null, so all five fatal. It is the residue of the same retirement: pramnos_html_form stopped existing, loadSettings() was hollowed out, and the methods that used the form were left calling into nothing.

Deciding what they should do instead — go inert, or be wired to something real — is a design question rather than a typo, and the framework has no form builder to wire them to. Recorded here rather than answered.

Documentation

The Document Output guide's escaping section said escaping applied to <head>. It now says body classes are covered too, and the "Body Classes and Styling" section states what the renderers actually emit, names the stale @todo as a documentation failure, and points at extraBodyTag for the raw case.

Being told no

framework-docs made the framework's rules findable. Findable is not the same as followed: every rule in the list below is something that happened after the guide describing it was written. Being able to look a rule up and being told when you have broken it are different mechanisms, and only the second has a track record.

pramnos-check is the second one. A seventh MCP tool, no application or database required.

Added

{}                                           // the whole project
{"path": "src/Models"}                       // one subtree, or a single file
{"rules": ["raw-sql", "flash-query-params"]} // a subset

Seven rules — six defects, and one that polices the escape hatch. Each is chosen for the same property: it fails silently. A table name that matches nothing, a message that reappears on reload, a view variable that is simply absent in the template, a migration an installation skips.

The authserver table list is read from the framework's own migrations at runtime, so it cannot drift out of step with the schema the framework creates.

Suppression requires a reason:

// pramnos-check: ignore raw-sql — recursive CTE the builder cannot express

A bare ignore raw-sql suppresses nothing and is reported as its own finding. The value of rule 12's "leave a one-line comment saying why" is that the next reader can tell a considered exception from an oversight, and a check tool that lets you delete findings silently is worse than no check tool.

Precision was the hard part, and it was measured

A check that cries wolf gets muted, and then the real finding it makes next month is muted with it. So every rule matches a construction, not a name — the lesson from a check in this framework's own history that flagged var rows in six unrelated functions because it matched an identifier rather than a redeclaration, and was deleted for it.

The first run against the framework's own src/ reported 29 raw-SQL findings, and sixteen were noise: SELECT version(), SELECT NOW(), select @@global.long_query_time, TimescaleDB catalogs — and one example inside a docblock, because the first version did not strip comments. A tool that reports the guide teaching the right thing has already lost.

Tightened to nine defensible findings, and each exclusion has a negative test:

  • rule 12 exempts introspection and driver-specific features in its own text;
  • a statement with no table to address cannot be expressed by a builder at all;
  • migrations must emit exact SQL and fixtures are clearer as literals;
  • reading ?message= is legitimate — an application does not control every link pointing at it;
  • authserver.user_activity_log contains the unqualified form as a substring;
  • $config->path = … is not a view variable;
  • _debug in a project with no shipped lib/debug.js has nothing to duplicate.

The second-largest source of noise was self-inflicted and caught by its own tests: blanking comments before looking for suppression comments silences nothing, because a suppression is a comment. The tool now reads code and comments as two views of the same file.

The framework does not pass its own check

Against src/: 9 raw-SQL findings and 67 flash-query-parameter findings. All reviewed, all real.

That is this tool's argument turned on its author. The rules were written down, and the framework drifted from them in seventy-six places — a controller redirecting to ?error=not_found instead of using the flash API it ships, a SELECT COUNT(*) FROM ' . $table where ->table($table)->count() exists, and three raw statements the scaffolder writes into every new project.

Recorded rather than quietly fixed. Rewriting seventy-six call sites is a decision about priorities, and presenting it as a tidy-up in the same change that introduced the tool would hide how much there is.

The suite is finished, and the answer to the last question changed

Two items were standing open on the test-suite performance study: finish the remaining DatabaseTestCase conversions, then re-ask whether paratest is the next step. Both are now answered by measurement, and the first one is answered no.

Where it ended up

Two consecutive --nocoverage runs on the same tree: 3:44 and 3:46 for 9750 tests, 211.9 s and 214.1 s of measured test time. The study opened at 14:58 for 9364 tests.

Directory Per test now Per test then
tests/Unit 7.9 ms 60 ms
tests/Integration 103.6 ms 303 ms
tests/Characterization 35.2 ms 84 ms

The remaining conversions are declined

Fifteen classes still do DDL in setUp() rather than setUpBeforeClass() — the shape that took 81 s out of ten classes earlier in the study. Seven of the fifteen are convertible at all. Together they are 10.26 s of 211.9 s: 4.8%. At the reduction actually achieved before (8.38 s → 1.38 s, ~83%), converting all seven perfectly saves about 8.5 s.

The suite's run-to-run spread is ±15 s. So the entire remaining programme is at or under the noise it would have to be measured in — there is no experiment that could show it worked. That is a different thing from "small win": it is a win that cannot be observed.

Two further reasons, both already in the study: three of the seven are PostgreSQL, where this exact conversion was measured making a class slower (5.71 s → 7.34 s, reverted); and four are migration tests, where the DDL is the subject rather than the cost.

Two other candidates were examined and left. InitSpinnerTest spends 3 s in sleep — the shape of the study's first item — but the spinner escalates on an integer-second threshold, so a command must genuinely exceed a second for the path under test to exist. The compressible part is 1.4 s, 0.6% of the run, in exchange for tightening a timing-dependent test. A flake in CI costs more than that the first time.

paratest: the previous answer expired

It said "not yet — finish the cheaper work first." The cheaper work has now been measured, and it is worth 8.5 s. So parallelism is the only remaining lever of any size, and the question is no longer whether something cheaper exists but whether the prize justifies the cost.

At 8 workers, bin-packing by class gives a 26.5 s makespan against 211.9 s of test time — ideal scaling, ceiling 10.7×, set by FrameworkMigrationsPostgreSQLTest. With ~12 s of non-test wall clock, 3:45 becomes roughly 50 s: three minutes a run.

The cost is unchanged and permanent: a database per worker on three engines, and the 38 test files that name pramnos_test themselves routed through a helper.

The recommendation is now decide it on CI, not on a developer's patience. Locally, 3:45 is inside the span where you read the diff rather than leave the desk, and --filter already answers fast feedback in under a second; buying three minutes with permanent cross-worker isolation is a poor trade when that is not the binding constraint. On CI it is minutes × runs × people, it is money rather than patience, and a worker-split flake is caught by a rerun instead of by somebody's afternoon.

Either way, step 3 — routing every connection through one helper — is worth having on its own.

What the shape says

When the study opened, 203 tests were 46% of the run and the work was to find them. Now the largest band is 525 tests averaging 190 ms, and the 19 tests over a second are 13.6% of the time — mostly the migration classes that are meant to be slow.

There is no target left. That is the same conclusion as the declined conversions, arrived at from the other direction, and it is why the study is closed rather than paused.

A form class that was never ported

Theme::addSetting() and Addon::addSetting() have declared the same API since the day those files were transferred from the legacy framework. Both fatalled. The collaborator they called into — pramnos_html_form — was never ported, and the line that built it arrived already commented out:

#$this->_form = new pramnos_html_form($this->name, false);

So $_form was null from the first commit, and ten public methods across two subsystems have never worked. The practical consequence: no addon could have settings at all.

What was built, and what deliberately was not

Pramnos\Html\Form\SettingsForm, with Field and a shared FieldStyles preset table. It is settings, not CRUD, and the boundary is the design:

  • Laravel removed its form builder from core — it survives as an unmaintained package — because markup in PHP objects fights every front-end toolchain. Symfony keeps a full Form component, and it is the single most common complaint about its learning curve.
  • This framework already generates CRUD forms, from real column introspection, with foreign keys becoming a <select> (Select2 remote for large tables) and three theme presets. A runtime builder would be a third way to render a field, after that and an SPA's components.
  • Validation, error surfacing and old input already belong to FormRequest and View::$errors.

What was genuinely missing is the shape Django's Form and WordPress's Settings API both describe: the caller declares fields, the framework renders, reads and persists them. Two subsystems had already declared exactly that.

The legacy class was not ported, and would have been the wrong thing to port. It needed four classes, not one — pramnos_html_form_field, pramnos_html_select and pramnos_html_input are all absent too. It carried a real XSS hole: value="' . $this->value . '", unescaped, on a page whose values are administrator-supplied and re-rendered after every save. It had a getInstance() singleton on a form, and its CSRF token was the field's name, so nothing else could verify it.

What was kept from it: the eight-argument signature, the three option shapes, multilanguage fields — Addon uses them — and the method name addField().

Details that are behaviour, not decoration

  • Everything is escaped: values, labels, descriptions, option labels and option values.
  • A checkbox can be turned off. Browsers submit nothing for an unchecked box, so each one renders with a hidden 0 companion the checkbox overwrites with 1. Without it a setting could be switched on and never off.
  • A rejected submit writes nothing. getData() returns [] when the CSRF token does not check out, and Theme::saveSettings() refuses to store an empty result — so an expired session cannot blank every setting.
  • An empty string is a value. Only null falls back to the default, so a setting deliberately cleared stays cleared instead of reverting on every render.
  • The CSRF field is the framework's own, Session::getTokenField(), not a token of the form's invention.
  • Style presets are shared with the scaffolder's generated forms, so a settings page and the CRUD form beside it agree. They cannot share a renderer — the scaffolder emits template source containing <?php echo …, this emits markup — but the class names are the part that drifts, and there is now one list.

One bug caught by its own test: ['1' => 'Enabled', '0' => 'Disabled'] — the obvious way to declare a boolean setting — arrives with integer keys, because PHP coerces numeric string keys. The first version read that as a flat list and rendered value="Enabled". array_is_list() decides it correctly. The residual ambiguity is documented rather than hidden: [0 => 'No', 1 => 'Yes'] is indistinguishable from ['No', 'Yes'], so use the [label, value] pair form when your keys are 0 and 1 in that order.

Documentation

The Theme Guide's settings section documented $this->addField(...) on a theme. addField() is real, but it belongs to the form — the chain was Theme::addSetting() → pramnos_html_form::addField(), and the guide had written the inner call as if it were the theme's own. It also showed a 'value,Label|value,Label' option format the code never parsed. Ten occurrences, in a section describing a feature that could not run.

The new class keeps the name addField() for exactly that reason: anyone carrying the legacy knowledge now lands on the right object.

The section is rewritten to current state — the real signature, the option shapes with their one undecidable case, escaping, the checkbox companion, where values are stored, and how addons declare the same thing.

Tests

33 for the form, at 96% line coverage; Field and FieldStyles at 100%. The escaping cases are the ones that matter, and there is a test for the converse of every refusal — a valid CSRF token being accepted — because otherwise a class that rejected everything would pass.

Two existing ThemeTest cases injected an anonymous stub implementing addField() and a public $_fields, because the real collaborator did not exist. They now use the real form and need no stub. That is worth naming as a signal: a test that has to invent its subject's collaborator is describing something that cannot run in production.

Sixty-seven messages nobody could see

pramnos-check shipped this morning and reported 76 findings against the framework's own src/: 9 raw-SQL and 67 flash-query-parameter. All reviewed, all real. They are now all fixed, and the flash half turned out to be a bigger job than "change 67 redirects".

The flash had no display side inside the framework

Base::addMessage() and addError() write $_SESSION['_messages'] and $_SESSION['_errors'], and nothing in src/ read them back: _getMessages() and _getErrors() are protected and are called from nowhere in this repository.

They are read by consuming applications, though — which is the entire point of their living on Base. A reference application calls _getErrors() in three API controllers to put the errors into a JSON response. So the mechanism did have a display side; it was outside this repository and invisible to a grep of it. An earlier draft of this post said "nothing ever read them", which was wrong, and the correction mattered — see the regression below.

What was missing here is a path a framework view can use, which is why 67 controllers passed ?error=… in the redirect URL instead. Nothing read that either: there are no views in the framework's own view directories at all. So converting the redirects without building the reading half first would have been a lateral move — from nobody showing it to nobody showing it.

So the reading half was built first

Into the seam that already existed rather than a new one. Request already owns this exact pattern for validation errors: capture once per request, clear the session entry immediately, keep the values available for the rest of the request. That is what makes a flash a flash — shown on the page the redirect lands on, and not again on a reload.

  • Request::messages() and Request::flashErrors(), captured and cleared by the same one-shot method as errors() and old().
  • View::$messages and View::$flashErrors, read once in the constructor beside the existing $errors.

$errors keeps its meaning — the per-field output of a validator — because a template that iterates one expecting the other gets field names where it wanted a sentence.

The 67 conversions

Each redirect now flashes a sentence instead of a code:

// before
$this->redirect(sURL . 'organizations?error=not_found');

// after
$this->addError('That record no longer exists.');
$this->redirect(sURL . 'organizations');

Thirty-three distinct codes across ten controllers became thirty-three sentences. ?error= and ?message= no longer appear in any controller.

Sixty existing assertions pinned the old shape — assertStringContainsString('error=not_found', $redirect) — and they were the specification of what changed. They now assert the user-visible outcome instead: that the sentence is in the flash bag. That is a better test than the one it replaced; the URL parameter was never the point.

The 9 raw-SQL findings

Where What
ApiCrudController SELECT 1 FROM $table LIMIT 1 → ->table($table)->exists()
ApiAdmin SELECT COUNT(*) AS total FROM $table → ->count()
DatabaseQueueDriver ×2 an interpolated row id, and a full-table delete → bound and built
Init.php ×3 generated test teardown: the scaffolder was writing raw SQL into every new project, with a hand-rolled comment explaining how it avoided backticks. The builder does that properly
PolicyEngine ×2 INSERT … SELECT, which the builder cannot express — suppressed with the reason, which is what rule 12 asks for

pramnos-check now reports no findings across 463 files and 7 rules.

The regression this shipped past the first fix

Twice, in fact. The first fix covered the destructive readers and missed the gates in front of them, and the second miss was worse than the first.

Round one: _getErrors() returned false

Base::_getErrors() reads $_SESSION['_errors'] and, finding nothing, returns false — it does not fall back to the instance bag. The new capture unsets that key, and View::__construct() triggers the capture on essentially every request. So an application reading its flash through Base would have received false for errors that were flashed perfectly well: an API response that used to carry errors would have carried false, and nothing about it would have looked wrong.

The claim that made this possible was mine: "nothing ever read them", from a grep of src/. But _getErrors() is protected on Base, which every application extends — so its callers are outside this repository and a grep here cannot see them. A protected member on a universally inherited base class is a public API.

Round two: the gates, which is where the real damage was

Fixing the destructive readers was not enough, and the search that found them was aimed wrong: it looked for _getErrors, and a reference application's actual readers are hasErrors() and _printErrors(). It gates every flash it displays:

if ($this->hasErrors()) { echo $this->_printErrors(); }

in its theme header and in five views. hasErrors() had no fallback, so it answered false, the printer was never reached, and the entire flash UI went silent — invalid-login messages, lockout notices, CSRF errors, and around sixty addError()/addMessage() calls across its admin controllers. Nothing failed anywhere.

How it was actually found, because this is the part worth copying: the application's vendor/ copy was swapped for a symlink to this repository, and three real HTTP requests were driven through both versions with one cookie jar — GET login, POST with a bad CSRF token, GET login — while counting how many times the message appeared.

Framework Appearances of "Security token invalid or expired"
vendor/ (1108 commits behind) 1
this branch 0

Its own 5497-test suite passed identically on both, with the same 15624 assertions. So did this framework's 9795. The regression was invisible to both suites and visible in one byte count.

All four members now consult the capture. The gates stay non-destructive — a gate that consumed would leave the printer with nothing, the same silence by the opposite route — and the printers consume, so a message is shown exactly once. Verified by driving the same three requests again: 1 appearance, not 2.

And two loose ends the same investigation surfaced

ApplicationsController read $_GET['message'] into a view, and the scaffolder generated an account-profile view that mapped ?message=profile_saved to a sentence — with nothing anywhere emitting those parameters. The same half-wired shape as the 67 redirects, generated into every new project. Both now read the flash. No $_GET['error'], ['message'] or ['msg'] reader is left in src/.

What was originally written up as the near-miss

Draining the session for the new path nearly broke the old one, and it would have been silent.

Base::_getErrors() reads $_SESSION['_errors'] and, finding nothing, returns false — it does not fall back to the instance bag. The new capture unsets that key, and View::__construct() triggers the capture on essentially every request. So an application reading its flash through Base would have received false for errors that were flashed perfectly well: an API response that used to carry errors would have carried false, and nothing about it would have looked wrong.

Three lines were enough to reproduce it once the possibility was pointed out — flash an error, let something read the request as the view does, read through Base, get false.

_getErrors() and _getMessages() now fall back to the same per-request capture, so both paths work and the session is drained once. One behavioural difference, documented rather than hidden: within a single request they can now return the same errors twice, where the second call previously answered false.

The lesson is not "add a fallback". It is that a protected method on a base class every application extends is a public API, and a grep of this repository cannot see its callers.

Three mistakes made along the way

Recorded because each was caught by something specific, and the something is the point.

The queue would have claimed nothing. Converting DELETE FROM … WHERE id = … to the builder, the atomic claim became (int) $deleted !== 1 — but delete() returns a Result, not a row count, so that comparison would have been false for every job and the queue would have processed nothing at all. Caught by reading the builder's return type rather than assuming it.

A suppression that suppressed nothing. The PolicyEngine comment was four lines above the statement; the check accepts the same line or the one immediately above. The tool reported its own suppression as unsuppressed, which is the correct behaviour and exactly what the feature is for.

A regex that was far too greedy. The last pass over the test assertions matched a bare code string anywhere and changed 74 assertions across 17 files when 11 needed changing — including files that had never failed, where assertStringContainsString('deleted', …) was about a response body rather than a redirect. The suite went green, which is worse than red: passing tests that assert the wrong thing. Twelve files were reverted and the remaining conversions checked one by one, confirming every one referenced redirectedTo, lastRedirect or the echoed redirect.

Also fixed

ViewTest::testGetTplWithActualFile asserted the debug comment in a rendered template and passed only because some other test in the suite set APP_DEBUG first. Gating that comment behind debug mode — a path-disclosure fix from earlier today — is what made it order-dependent. It now sets the variable itself and restores it.

Documentation

The Framework Guide's error-handling section documented addError() without saying how anything displays it, because nothing did. It is now a flash-messages section: how to write one, how to read it in a view, why not to use a query parameter, and the three things about the mechanism worth knowing — that messages is not errors, that reading consumes, and that it needs a session.