Skip to content

fix(schema): register steps 361-367 so an upgrade can leave the schema page - #1340

Merged
mastacontrola merged 1 commit into
working-1.6from
fix-schema-steps-not-registered
Aug 24, 2026
Merged

fix(schema): register steps 361-367 so an upgrade can leave the schema page#1340
mastacontrola merged 1 commit into
working-1.6from
fix-schema-steps-not-registered

Conversation

@mastacontrola

Copy link
Copy Markdown
Member

Regression from #1338, reported from a live deploy. An upgraded server cannot leave ?node=schema.

What I got wrong

commons/schema.php is not one large array — it is ~326 separate $this->schema[] = [...] statements. I appended #1338's seven UPDATEs before the file's final ];, so they joined the previous step's array and the step count never moved, while FOG_SCHEMA went 360 → 367.

count($this->schema) == mySchema == 360
  -> $hasIndexed false, updater applies nothing
  -> version written from the loop variable, so never advanced past 360
  -> mySchema < FOG_SCHEMA stays true forever
  -> DatabaseManager::establish() redirects EVERY request to ?node=schema

The schema page then reports nothing to do and answers the POST 204, which jQuery reports as statusText nocontent and $.notifyFromAPI renders as a red "Generic Error" toast, looping.

A fresh install was fine — the statements still ran inside that last step. That is exactly why every schema job in CI stayed green: they build from scratch and never replay an upgrade from an existing version.

The fix

Seven real $this->schema[] = [...] entries, labelled 361-367, so array_slice($this->schema, 360) yields them and the last sets the version to 367. FOG_SCHEMA is unchanged — 367 was always the right number (325 literal appends + the 35 the keySequences loop writes = 360 before this).

The part that matters more

tests/schema-gate.test.php already existed for this exact bug class and did not catch it. It compared $schema < $highest, enforcing only the case where FOG_SCHEMA is too low. #1338 made it too high, and with no new label and no new append there was nothing for its other two checks to see either.

Its own docblock already said the high side is "worse than stranding a step" — a server cannot leave the schema page to fix itself. The comparison is now equality, and the message says which direction failed and names the usual cause.

Mutation-verified both ways:

Full suite: 144 passed, 0 failed.

…a page

#1338 raised FOG_SCHEMA 360 -> 367 and added no steps. Its seven UPDATE
statements were appended before the file's final `];` on the assumption that
commons/schema.php is one large array. It is not -- it is ~326 separate
`$this->schema[] = [...]` statements -- so they landed INSIDE the previous
step's array and the step count never moved.

The consequence is total, and it is not "the icons stay wrong":

  count($this->schema) == mySchema == 360
    -> $hasIndexed false, so the updater applies nothing
    -> the stored version is written from the loop variable, so it is never
       advanced past 360
    -> mySchema < FOG_SCHEMA stays true forever
    -> DatabaseManager::establish() redirects EVERY request to ?node=schema

So an upgraded server cannot leave the schema page, while that page reports
there is nothing to do and answers the POST with 204 -- which jQuery reports
as statusText "nocontent" and $.notifyFromAPI renders as a red "Generic Error"
toast on a two-second loop. Reported from a live deploy.

A FRESH install was unaffected, because the seven statements still ran as part
of that last step. That is why every schema job in CI stayed green: they build
from scratch and never replay an upgrade from an existing version.

The statements are now seven real `$this->schema[] = [...]` entries, labelled
361-367, so array_slice($this->schema, 360) yields them and the last one sets
the version to 367. FOG_SCHEMA is unchanged at 367, which was always the right
number -- 325 literal appends plus the 35 the keySequences loop writes was 360
before this.

tests/schema-gate.test.php already existed for this bug class and did not
catch it, which is the more useful half of this commit. It compared
`$schema < $highest`, so it enforced only FOG_SCHEMA being too LOW. #1338 made
it too HIGH -- with no new label and no new append, there was nothing for its
other two checks to see either. Its own docblock already said the high side is
"worse than stranding a step". It now compares for equality and explains which
direction failed. Verified by reproducing #1338's exact shape, which fails it
with the cause named, and by the low side, which still fails as before.

Co-Authored-By: Claude <noreply@anthropic.com>
@mastacontrola
mastacontrola merged commit 4099254 into working-1.6 Aug 24, 2026
7 checks passed
@mastacontrola
mastacontrola deleted the fix-schema-steps-not-registered branch August 24, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant