Answering Y at the schema prompt no longer takes the manual path

[darksidemilk]

Draft. Reported from a live upgrade — see the transcript below. phpstan could
not be run locally (no vendor/); see Not verified locally.

The bug

From the reporting server's own console. The operator typed Y:

 * Backing up database.........................................Done

 * Install/update the FOG database schema now? (Y/n) Y

 * You still need to install/update your database schema.
 * This can be done by opening a web browser and going to:

   https://fog.example.com/fog/management/index.php?node=schema
 ...
 * Press [Enter] key when database is updated/installed.

 * Verifying database schema...................................Done

Note what is absent: there is no * Updating Database.... step. The [Yy]
case never matched, so the automatic deploy never ran. The install then reported
Verifying database schema...Done — truthfully, because that server's schema
was already current.

read returns exactly what the terminal sent. It strips the newline and nothing
else, so a client ending its lines with CRLF delivers $'Y\r', which matches
none of the patterns:

Y            -> AUTOMATIC (deploys)
y            -> AUTOMATIC
yes / YES    -> AUTOMATIC
$'Y\r'       -> MANUAL branch (prints token URL, no deploy)   <-- here
'Y '         -> MANUAL branch (prints token URL, no deploy)
' Y'         -> MANUAL branch (prints token URL, no deploy)

The manual branch is not a harmless fallback. updateDB()'s own comment says it
"verifies nothing and hands the install token out on stdout", and this is how
it gets reached by accident. On a server that genuinely needed migrating, the
same slip leaves it unmigrated under a Done two lines later.

The fix

normalizeAnswer() strips the carriage return and trims. It is applied before
the case, not inside the read branch, so every source of the value goes
through it — including bin/installfog.sh's -y. The read gains -r.

The direction of the fallthrough is deliberately unchanged. An answer still
unrecognised after normalisation opts out, exactly as today. Only the characters
a terminal adds by itself stop deciding it. (Inverting it was considered and
rejected: it would make nope deploy.)

Second, smaller thing, from the same report

"it made me click through to the schema update rather than erroring but there
was no schema update to apply, this would likely confuse the average user"

The question implies pending work. Asking it on a database with every indexed
step applied — and then migrating nothing — reads as the answer having been
ignored. It is now skipped in that case, and the state stated instead:

 * The database schema is already at version 244 --
   no migration to apply. Checking required rows.

The deploy still runs there, and that is the load-bearing part. The POST is
not only migrations: it seeds required rows, and Schema::seedRequiredRows()
exists for precisely the state where the indexed steps are applied and rows are
missing. Its docblock records finding that by deleting a seeded row on a server
at vValue == FOG_SCHEMA and watching the updater redirect instead of restoring
it. Skipping the step would make that repair unreachable on the servers that
need it — so the new predicate gates the question, never the deploy.

schemaStepCount() is factored out of verifySchemaDeploy() rather than
copied. Two copies of that grep would be two things to keep in step, and the
cost of disagreement is an installer that asks about a migration it then reports
as unnecessary.

Unknown on either side keeps today's behaviour exactly — ask, and deploy:

fresh install (no schemaVersion table)	unknown → ask
DB_external=yes (refuses to look)	unknown → ask
schema.php whose formatting stopped matching	unknown → ask, never "zero steps, all done"

Testing

tests/schema-prompt-answers.test.sh — 37 assertions on generated fixtures with
schemaVersionInDB() stubbed. No install, no database, no network, no root.

Three groups, because they fail differently: what normalisation returns; where
each answer routes; and the wiring — normalizeAnswer() can be correct and
unreached, so the call's existence and its position before the case are both
pinned.

Mutation-tested:

mutation	result
no normalisation (upstream)	19 of 37 fail
normalizeAnswer called after the case	exactly the two wiring assertions fail
a zero step count reads as "done"	exactly the two assertions that refuse it fail

Neighbouring suites, all unchanged: installer-tls-verification 20/20,
revert-offer 46/46, install-settings-resolution 50/50,
web-chain-external-leaf 17/17, web-chain-feedback 9/9,
installer-schema-deploy ok.

selfcall-verification reports 14/4 identically before and after on this
machine — empty-CN -subj fixture failures specific to it, not this change.

Not verified locally

• phpstan (both passes) did not run — no vendor/. This adds no PHP, so
  the risk is low, but the tests/ pass baselines occurrence counts.
• Not run against a real database. schemaVersionInDB() is stubbed, so the
  predicate's inputs are exercised but not its live query. The end-to-end path
  is what the reporter will re-test.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ps28eGTALgieR6TBUSafVg