Merged
Conversation
Bumps [actions/checkout](https://github.com/actions/checkout) from 5.0.0 to 6.0.2. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@08c6903...de0fac2) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 6.0.2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Contributor
Author
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
mambax7
added a commit
that referenced
this pull request
Apr 17, 2026
* normalize profile and session indexes for 2.7.0
Add three idempotent upgrade tasks to the 2.5.11 → 2.7.0 patch that
standardise index DDL across fresh and upgraded installations:
- normalizeprofilefieldname — ensures profile_field.field_name is
varchar(64) and its UNIQUE index uses explicit prefix (64) USING BTREE.
- normalizeprofileregstepsort — recreates profile_regstep.sort as
KEY (step_order, step_name(100)) USING BTREE.
- normalizesessionpk — recreates session PRIMARY KEY as
(sess_id(200)) USING BTREE.
Each check queries information_schema.STATISTICS.SUB_PART and only
applies when the prefix differs from the target, so the tasks are safe
to re-run against installations at any intermediate state. Two private
helpers (indexExists, indexPrefixLength) back the checks.
Fresh-install SQL updated to converge on the same final state:
- install/sql/mysql.structure.sql
session PRIMARY KEY → (sess_id(200)) USING BTREE.
- modules/profile/sql/mysql.sql
profile_field UNIQUE KEY field_name → (field_name(64)) USING BTREE.
profile_regstep KEY sort → (step_order, step_name(100)) USING BTREE.
All composite index sizes stay under the 767-byte InnoDB 5.7 default
limit when evaluated against utf8mb4 (4 bytes/char), keeping the schema
portable to InnoDB without innodb_large_prefix:
profile_field.field_name : 64*4 = 256 bytes
profile_regstep.sort : 2 + 100*4 = 402 bytes
session PRIMARY KEY : sess_id is CHARACTER SET ascii, 200 bytes
Follows the conventional-commits + XOOPS extension format (feat type, upgrade scope, single-sentence title under 70 chars). No RFC: footer — let me know if this work ties to a
specific RFC and I'll add one.
* fix(upgrade): harden index-normalisation tasks for 2.7.0
Address review feedback on the profile/session index normalisation tasks
added in 030463f:
- Gate profile_field and profile_regstep tasks on table existence. If the
Profile module isn't installed, both check_ and apply_ short-circuit
to "already applied" instead of aborting the whole 2.7.0 patch on a
missing information_schema row or a failing ALTER against a missing
table.
- Refuse to silently narrow profile_field.field_name. If a site has
customised the column to a width greater than 64, apply_ now logs a
descriptive error and returns false rather than relying on server
sql_mode to raise an error — avoids silent data truncation on servers
without STRICT_ALL_TABLES.
- Tighten the profile_regstep.sort layout check. The previous check
validated only the step_name SUB_PART; a partial hand-patch that
dropped step_order from the index could pass. Now reads the full
index via information_schema.STATISTICS ordered by SEQ_IN_INDEX and
asserts the exact expected shape
[(step_order, 1, NULL), (step_name, 2, 100)].
- Split the session PRIMARY KEY rebuild into separate DROP and ADD
statements, guarded by indexExists('PRIMARY'). A session table that
somehow lost its PK (manual intervention, prior failed migration) can
now be normalised; the previous combined ALTER aborted on the DROP.
Adds private tableExists() helper alongside indexExists() and
indexPrefixLength().
* fix(upgrade): make tableExists tri-state and verify column DATA_TYPE
Address second-round review feedback on PR #23:
- tableExists() now returns ?bool. null means the information_schema
query failed (connection issue, permission denial), distinct from
false which means "table is absent". Callers previously collapsed
both into "skip the task", which could silently leave a present
table unnormalised when the DB was in a partially-broken state.
Callers now treat null as a hard failure (log + return false) and
only skip when the table is genuinely absent.
- check_/apply_normalizeprofilefieldname() now validate DATA_TYPE in
addition to CHARACTER_MAXIMUM_LENGTH. A field_name defined as
char(64) has CHARACTER_MAXIMUM_LENGTH=64, so the previous check
incorrectly treated it as already normalised. The new check requires
DATA_TYPE='varchar' before passing. apply_ additionally refuses to
touch the column when DATA_TYPE is anything outside {varchar, char}
(e.g. text, enum) — admin intervention is required for those cases
rather than issuing a MODIFY that could truncate data.
The sess_id(200) uniqueness concerns raised in r3102108842 and
r3102108855 are intentional — XOOPS generates 32-char session IDs
and the 200-byte prefix provides InnoDB/MyISAM portability under
utf8mb4. Keeping as-is.
* fix(upgrade): reorder type and length guards for clearer diagnostics
When profile_field.field_name is an unexpected wide type such as
`text` (CHARACTER_MAXIMUM_LENGTH = 65535), the old order fired the
"refusing to narrow to varchar(64)" branch first, which is misleading
— the real blocker is the type mismatch, not the length.
Swap the order: type mismatch is rejected first, so only legitimate
`varchar`/`char` columns ever reach the narrowing guard. Behaviour
is unchanged; only the logged diagnostic improves.
Addresses CodeRabbit feedback on PR #23 (r3102217357).
* docs(upgrade): clarify profile_field.field_name normalize invariants
The task header previously claimed the target was
"varchar(64) NOT NULL DEFAULT ''", but the check_ function only
validates DATA_TYPE + CHARACTER_MAXIMUM_LENGTH, so a column that is
already varchar(64) with a different nullability or default would be
reported as normalised without enforcement.
Update the header to state what is actually enforced:
- checked invariant: column is varchar(64) AND unique index uses
prefix (64)
- not checked: IS_NULLABLE, COLUMN_DEFAULT
The MODIFY ALTER still sets NOT NULL DEFAULT '' when it runs, matching
the module install SQL. Sites that have intentionally changed
nullability or default are left alone — forcing a rewrite there could
coerce NULL values to '' and surprise admins.
Addresses Copilot feedback on PR #23 (r3102290919, r3102290942).
* fix(upgrade): make indexExists tri-state for parity with tableExists
indexExists() previously returned false both when the index was genuinely
absent and when the information_schema query failed. Callers in the
three normalize tasks treated a false result as "index absent, skip the
DROP and proceed to ADD" — if the query had actually failed on a table
with the index present, the subsequent ADD would hit a "Duplicate key
name" / "Multiple primary key" error that looked like a real DDL bug
rather than an information_schema access problem.
indexExists() now returns ?bool with the same semantics as tableExists():
true → index exists
false → index is absent (safe to skip DROP)
null → information_schema query failed (log + return false)
Updated three call sites (apply_normalizeprofilefieldname,
apply_normalizeprofileregstepsort, apply_normalizesessionpk) to log a
clear diagnostic when the helper returns null and abort the task.
Addresses Copilot feedback on PR #23 (r3102290952) and CodeRabbit
feedback (r3102217365).
* fix(upgrade): strict layout checks for field_name and session PK indexes
Bring the two remaining normalize checks up to the same strictness as
check_normalizeprofileregstepsort: instead of validating a single cell
(SUB_PART on one column), query the full index layout via
information_schema.STATISTICS ordered by SEQ_IN_INDEX and assert the
exact expected shape.
- check_normalizeprofilefieldname now requires exactly one row with
COLUMN_NAME='field_name', SEQ_IN_INDEX=1, SUB_PART=64, NON_UNIQUE=0.
A non-UNIQUE or composite index named `field_name` no longer slips
through as "already normalised".
- check_normalizesessionpk now requires exactly one row with
COLUMN_NAME='sess_id', SEQ_IN_INDEX=1, SUB_PART=200. A composite
PRIMARY KEY containing sess_id(200) plus another column no longer
slips through.
All three normalize tasks are now consistent in how they verify index
shape — the task is truly idempotent only when the live schema matches
the canonical layout.
Addresses Copilot feedback on PR #23 (r3102364473, r3102364533).
* refactor(upgrade): skip-on-wide-column and drop unused indexPrefixLength
Two cleanups in the normalize task area:
- apply_normalizeprofilefieldname() no longer aborts the whole 2.7.0
patch when profile_field.field_name has been widened beyond 64 chars
locally. It now logs a clear warning and returns true so the
remaining 13 upgrade tasks can still run. The legacy UNIQUE index on
the full wider column stays in place — still functional, still
unique. The admin is told to narrow the column manually and re-run
the upgrade to normalise the index shape.
Rebuilding the UNIQUE index with prefix(64) on a wider column was
considered and rejected: it would actively weaken uniqueness from
"full column width" to "first 64 chars" and could allow previously
blocked duplicates.
- indexPrefixLength() helper is removed. After the strict-layout
refactor of the three normalize checks (commit 2a806d7), no caller
remains. Dead code drifts — delete it.
Addresses Copilot feedback on PR #23 (r3102436778, r3102436829).
* revert(upgrade): drop session PRIMARY KEY normalize task
Codex review (HIGH severity) flagged that changing the session PK
from (sess_id) to (sess_id(200)) introduces a silent session-hijack
vector:
- sess_id remains varchar(256), so two distinct session IDs sharing
the first 200 characters are treated as the same row by the
PRIMARY KEY.
- htdocs/kernel/session.php:147 writes sessions with
ON DUPLICATE KEY UPDATE, so a collision silently overwrites the
existing session rather than erroring.
- No code in core enforces session_id <= 200 chars — the length is
governed by PHP's session.sid_length (tunable up to 256) plus the
XOOPS token generator. Admin config could push IDs past 200 bytes
and trigger collisions without any code signal.
The 2.5.11 schema had no correctness issue with PRIMARY KEY (sess_id);
the prefix change was a "while we're here" consistency pass that
introduced real risk to solve a non-problem.
Changes:
- Remove task normalizesessionpk from the task list and numbering
(cleancache renumbered 14 → 13).
- Delete check_normalizesessionpk() and apply_normalizesessionpk().
- Revert htdocs/install/sql/mysql.structure.sql so fresh installs
keep PRIMARY KEY (sess_id) on the full column.
Addresses Codex finding #1 and supersedes the earlier dismissals of
Copilot r3101820136, r3101820175, r3102108842, r3102108855.
* fix(upgrade): enforce full column shape for profile_field.field_name
Codex review (MEDIUM severity) flagged that the normalize task claimed
to enforce "varchar(64) NOT NULL DEFAULT ''" but the check only looked
at DATA_TYPE + CHARACTER_MAXIMUM_LENGTH, and the apply path only ran
the MODIFY when type/length differed. A site with varchar(64) NULL or
a different default would be reported as normalised without actually
converging to the documented target.
- check_normalizeprofilefieldname() now reads IS_NULLABLE and
COLUMN_DEFAULT in addition to type/length, and only returns true
when all four match. Uses strict comparison so a NULL default is
correctly rejected (PHP (string) null == '' would collapse them).
- apply_normalizeprofilefieldname() now triggers the MODIFY whenever
any of the four attributes diverge, not just type/length. When the
rewrite includes nullability or default changes, a warning is logged
identifying the previous state so admins can audit — in particular,
NULL values in a previously nullable column are coerced to '' by the
MODIFY, which is a visible data change worth surfacing.
- Task header comment restored to claim the full four-attribute
invariant, now that it is actually enforced.
Addresses Codex finding #2 and supersedes the docblock-honesty
compromise in a9ab21f.
* fix(upgrade): assert NON_UNIQUE on profile_regstep.sort layout check
check_normalizeprofileregstepsort() previously validated columns and
prefix lengths but not NON_UNIQUE. A UNIQUE index with the same column
order and prefixes (e.g. UNIQUE KEY sort (step_order, step_name(100)))
would incorrectly pass as normalised — the canonical form is a plain
KEY, not UNIQUE.
Extend the STATISTICS query to SELECT NON_UNIQUE and assert it is 1
on both rows of the index. All three normalize checks now verify the
full expected shape of their index at identical strictness:
- field_name: (field_name, 1, 64, NON_UNIQUE=0) — UNIQUE
- sort: (step_order, 1, NULL, NON_UNIQUE=1),
(step_name, 2, 100, NON_UNIQUE=1) — plain KEY
Addresses Codex finding #3 and Copilot r3102515513.
* fix(upgrade): MariaDB default quoting, log escape, NULL pre-flight
Address three correctness/safety gaps in the profile_field normalize
task flagged by CodeRabbit and Copilot reviews of 0045e0f, and
confirmed by Codex:
- MariaDB 10.2.7+ wraps string-literal COLUMN_DEFAULT values in single
quotes — a column DEFAULT '' comes back as the 2-char string "''"
instead of an empty string. The direct `'' !== $row[3]` check would
therefore permanently mark any MariaDB 10.2.7+ install as "not
normalised" and attempt a no-op MODIFY on every upgrade run. A new
normaliseColumnDefault() helper strips surrounding quotes before
comparing, so the check works uniformly across MySQL and MariaDB.
- $currentDefault is interpolated into upgrade log messages that the
upgrader renders as HTML (joined with <br>). A crafted column default
containing HTML/JS markup would render in the upgrade UI. Wrap the
dynamic values in htmlspecialchars(ENT_QUOTES, 'UTF-8') so the log
displays them verbatim.
- When field_name is nullable and the UNIQUE index still exists, the
column can legally hold multiple NULL rows (SQL's NULL != NULL rule).
MODIFY ... NOT NULL DEFAULT '' coerces those NULLs into '', which
then violates the UNIQUE constraint and aborts the ALTER with a
low-signal duplicate-key error. A pre-flight COUNT(*) now detects
this case before attempting the MODIFY and refuses with an actionable
message naming the exact row count so the admin can resolve the
conflict and re-run.
Addresses CodeRabbit r3102684017 and Copilot r3102721783, r3102721820.
* fix(upgrade): let widened-column skip path satisfy check_
apply_normalizeprofilefieldname() returns true when profile_field.field_name
has been widened beyond 64 chars — it logs a "resolve manually" warning
and defers the normalise, so the rest of the 2.7.0 upgrade can proceed.
check_normalizeprofilefieldname() however still required
CHARACTER_MAXIMUM_LENGTH = 64, so on widened-column installs it kept
returning false. Since the XoopsUpgrade framework drives patch
applicability solely from check_* results, the patch showed as
permanently "not applied" even after apply_ ran and succeeded.
Mirror the pattern already used by check_/apply_cleancache: apply_ sets
a session flag on the skip-with-warning path; check_ reads the flag and
returns true when it's set. The widened column is then reported as
resolved-for-this-session and the upgrade queue can complete, while
admins still see the warning in the upgrade log. A new session (fresh
login) clears the flag so the advisory is re-surfaced on every
subsequent upgrade run — consistent with how the framework surfaces
operational state.
Addresses Copilot r3102874899.
* fix(upgrade): re-validate widened-skip flag against live column state
The session-flag short-circuit added in 69d75e0 was unconditional: once
apply_ had taken the skip-with-warning path, check_ would return true
for the rest of the session regardless of the actual column state.
That meant an admin who followed the logged advice and manually
narrowed profile_field.field_name back to 64 chars within the same
upgrade session would see the task keep reporting "already applied"
and never actually rebuild the unique index — they would have to log
out or wait for a new session before the upgrade would converge.
Move the column-metadata fetch above the flag check and honour the
flag only while the original trigger condition (CHARACTER_MAXIMUM_LENGTH
> 64) still holds. If the admin has narrowed the column, the flag is
stale — clear it and fall through to the normal canonical-shape check
so apply_ can rebuild the index immediately. No extra cost: the
information_schema query was already performed further down; it has
just been moved up.
Addresses Copilot r3102996202.
* fix(upgrade): fail hard when NULL pre-flight COUNT query fails
The nullable-to-NOT-NULL pre-flight added in 4f593a2 silently skipped
the collision guard when the COUNT(*) probe itself failed — if
$this->db->query() returned anything other than a valid mysqli_result,
the whole block was bypassed and control fell through to the
MODIFY ... NOT NULL DEFAULT '' statement. The MODIFY would then fail
with a duplicate-key error in exactly the case the pre-flight was
added to catch, producing the low-signal SQL error the guard was
supposed to replace.
Treat query failure as a hard error: log the DB error via the standard
_DB_QUERY_ERROR template and return false. Mirrors the pattern used in
every other information_schema probe in this file.
Addresses CodeRabbit r3103056876.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps actions/checkout from 5.0.0 to 6.0.2.
Release notes
Sourced from actions/checkout's releases.
Changelog
Sourced from actions/checkout's changelog.
... (truncated)
Commits
de0fac2Fix tag handling: preserve annotations and explicit fetch-tags (#2356)064fe7fAdd orchestration_id to git user-agent when ACTIONS_ORCHESTRATION_ID is set (...8e8c483Clarify v6 README (#2328)033fa0dAdd worktree support for persist-credentials includeIf (#2327)c2d88d3Update all references from v5 and v4 to v6 (#2314)1af3b93update readme/changelog for v6 (#2311)71cf226v6-beta (#2298)069c695Persist creds to a separate file (#2286)ff7abcdUpdate README to include Node.js 24 support details and requirements (#2248)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)