refactor(scorm): adopt the rewritten SCORM 1.2 runtime, and read what it writes - #105
Draft
erseco wants to merge 17 commits into
Draft
refactor(scorm): adopt the rewritten SCORM 1.2 runtime, and read what it writes#105erseco wants to merge 17 commits into
erseco wants to merge 17 commits into
Conversation
…M 1.2 rewrite Coordinated update with exelearning/exelearning#2209: - assets/scorm/SCORM_API_wrapper.js is now the unmodified upstream pipwerks wrapper (MIT, v1.1.20180906, pipwerks/scorm-api-wrapper@82e455b, SHA-256 f2a558ba284edbc6842edf51678df1f7e3e05cbf09ec00bc6dd5988b6caa2e78). - assets/scorm/SCOFunctions.js is now the eXeLearning-owned SCORM 1.2 runtime (GPL-compatible AGPL-3.0-or-later), assembled from the four layers in the eXeLearning repo (public/app/common/scorm/scorm12/). It keeps the full injected contract (loadPage/unloadPage globals, the scorm facade incl. SetScoreMax/SetScoreMin/GetLearnerName/GetScoreRaw, additive pipwerks.SCORM extensions), so scorm_injector.php needs no change. - thirdpartylibs.xml: bump the wrapper version and drop the SCOFunctions entry (no longer third-party); eslint-ignore the assembled artifact. Runtime contract: doc/development/scorm12-runtime-contract.md and doc/architecture/adr/ADR-0001-scorm12-runtime-rewrite.md in the eXeLearning repository.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
============================================
+ Coverage 93.82% 93.98% +0.16%
- Complexity 738 773 +35
============================================
Files 47 47
Lines 3350 3473 +123
============================================
+ Hits 3143 3264 +121
- Misses 207 209 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Document the provenance, build process, and AGPL/GPLv3 licensing rationale for the bundled eXeLearning editor and SCORM runtime.
erseco
changed the title
The policy comment cited ADR-0001 in exelearning/exelearning, which was renamed to ADR-2209-01 when that repository replaced its global ADR counter with tracking-number identifiers. That stale reference is what fails the decision-record check this branch inherits from main: the validator rejects the retired four-digit form anywhere in the tree. Naming the repository too, since it is a cross-repository pointer and not a record of this plugin.
erseco
added a commit
that referenced
this pull request
Aug 19, 2026
…pi scope note Documents the CURRENT state, not the desired one: the channel is exclusive per package (emitter present -> xAPI, otherwise SCORM with the runtime inert under xAPI-primary), today's real convergence point is track::apply_item_scores()/attempts::record_item(), and the OVERALL is deliberately drawn as two channel-specific computations until the PR #105 follow-up makes a single normalized per-item state literal (added as a second, smaller target figure). Also records why core_xapi is out of scope — it provides statement routing and validation but replaces none of the module-specific responsibilities, and its actor-validation model conflicts with the session-attributed anonymous emitter — linking DEC-85-01, and states the underlying security property explicitly: the learner's identity belongs to the Moodle session, never to the statement the content sends.
This was referenced Aug 21, 2026
erseco
force-pushed
the
refactor/scorm12-runtime-clean-provenance
branch
from
August 21, 2026 11:03
a9503fb to
d7f40f2
Compare
erseco
changed the title
…ng #2209 The branch vendored an older revision of the SCORM 1.2 rewrite than core ships today. This brings it up to the local exelearning#2209 state, which adds: - an explicit session state machine (idle -> active -> finish_attempted -> finished | finish_failed), so a termination that failed stays observably failed and no call is forwarded to the LMS afterwards; - local refusal of the calls SCORM 1.2 forbids — reading a write-only element, writing a read-only one — with legacy compatibility getters served from a local write cache instead of an invalid LMSGetValue; - a session clock that survives being paused by the back/forward cache without double counting. Provenance re-verified rather than assumed. Diffing the file against a plain concatenation of the four source layers from public/app/common/scorm/scorm12/ in core#2209 leaves exactly four blocks: the 13-line generated header and one 2-line section marker per layer. Every other line is byte-identical upstream. The fifth layer, exe-scorm12-activities.js, is still deliberately NOT shipped, and readme_moodle.txt now explains why rather than leaving it to be rediscovered. That layer installs the activity registry that makes upstream's common.js write the versioned `exe12/` cmi.suspend_data payload; this plugin's parsers — both js/scorm_tracker.js and track::parse_suspend_data() — read the legacy line format, so shipping it would silently cost every per-iDevice grade. Verified absent: zero occurrences of `exe12/` in the assembled file. Upstream's adapter treats the registry as optional in its install guard precisely so this subset composes, so the four layers are copied verbatim with no local edits. SCORM_API_wrapper.js (pipwerks) is untouched — still verbatim upstream. vitest 23/23 · phpunit 340/1260 exit 0 · phpcs 0
…time writes
The vendored runtime this PR replaces is only half the story. The plugin
supplies its copy ONLY to packages that do not carry one —
package_manager.php:254, `if ($present) { continue; }` — and eXeLearning's
SCORM exporter DOES carry one: Scorm12Exporter.spec.ts asserts the
assembled file ships all five layers, client, activities, policy,
lifecycle, adapter. The activities layer is the one that writes
cmi.suspend_data as `exe12/1|{record}|{record}…`.
So the moment core#2209 ships, every SCORM export uploaded here arrives
with a runtime writing a payload neither of this plugin's parsers can
read — `grep -c exe12` was 0 in both classes/local/track.php and
js/scorm_tracker.js — and every per-iDevice grade in those packages is
lost silently. That is caused by this change, not independent of it, so
it belongs in this PR rather than in one of its own.
Both parsers now dispatch on the header and nothing downstream re-sniffs
the payload. One representation serves both formats: every entry carries
title, scorepct and weighted, plus an `objectid` key on — and only on —
an entry that knows its own identity. A versioned record needs no DOM
resolution and cannot collide, so captureItemScores() and the server's
legacy itemnumber fallback both step aside for it; everything else there
exists only because the legacy format cannot name its owner and reuses
the page-local slot N across pages (DEC-5-01).
Version handling is deliberately strict. An unreadable version tag, or
one newer than the parser understands, yields an EMPTY map rather than a
best-effort parse: a future revision may reorder fields, and a silently
misparsed one would publish a WRONG grade — worse than publishing none,
which merely leaves the item visibly ungraded.
Three record shapes are skipped on purpose, each documented where it is
skipped: three-field records (migrated-but-unclaimed legacy entries that
name a page position, not an iDevice), records without the evaluable flag
(the producer excludes them from cmi.core.score.raw, so they must not
reach a gradebook column either), and records whose score field is empty
(no result yet, which is not the same as scoring 0).
Ported from the trace-replay branch rather than copied: that branch
predates #124 and #125, so the exe12 half was lifted onto the current
files and the legacy attribution work it also carries was left there.
filter_registered_scores() is extracted from the inline filter in
ingest(), because the objectid map recovered from suspend_data needs the
same protection against injected objectids.
Verified red without the parser: disabling the dispatch fails all four
new PHP tests and nothing else.
phpunit 344/1287 exit 0 · vitest 33/33 · phpcs 0
erseco
changed the title
assets/scorm/SCOFunctions.js was a four-layer subset of the upstream runtime, assembled by hand and patched locally, deliberately leaving out the activity registry so that upstream's common.js would keep writing the legacy cmi.suspend_data format this plugin parses. It matched no eXeLearning release, and every update meant reassembling and re-patching it. That defence is unnecessary. Measured over 25 recorded scenarios and two package vintages, serving the complete runtime produces LMS traffic identical to the subset, in every scenario: what this plugin serves is a web export, which does not carry the exe-scorm body class, so it never calls loadPage(), the entry policy never runs, and common.js keeps its legacy writer whether the registry is installed or not. So both files are now byte-identical copies of what eXeLearning ships inside its own SCORM packages — five layers, no local edits, carrying the version stamp that says which release they came from. Updating them is now "copy the two files out of a package", and tests/local/scorm/scorm_runtime_test.php fails if the copy is ever incomplete, unstamped or patched.
…kage brought The runtime files were installed only when the extracted package lacked them. For a web export that is every time, so nothing changed there — but a package uploaded as a SCORM export brings its own runtime of unknown vintage, and the plugin deferred to it. That runtime decides the marks this plugin then has to read back, so an activity could be graded by a runtime older or newer than the one the plugin was written against, with nothing to say which. Extraction now installs both files unconditionally, replacing whatever was there. One runtime per eXeLearning version, and the plugin's copy is the one that runs. The pair is also installed together or not at all. The previous loop worked per file name, so a package shipping only one of the two received the plugin's other half — pairing files written against different wrapper versions, a combination neither project tests.
erseco
added a commit
that referenced
this pull request
Aug 22, 2026
Merging #105 into this branch appended a block of four test methods that were already present, byte for byte. PHP refuses to redeclare them, so the whole mod_exelearning PHPUnit suite failed to load — not one test red, the suite not starting at all, which is why nothing on this branch had been verified since. Cannot redeclare mod_exelearning\track_test:: test_parse_suspend_data_reads_the_versioned_exe12_payload() Removes the second copy of each: reads_the_versioned_exe12_payload, skips_unusable_exe12_records, ignores_an_unsupported_exe12_version and ingest_routes_a_versioned_suspend_payload_by_objectid.
erseco
added a commit
that referenced
this pull request
Aug 22, 2026
package_manager already replaces the two runtime files in every extracted package, so the bytes were never in doubt. The script tags were: an eXeLearning SCORM 1.2 export references libs/SCORM_API_wrapper.js and libs/SCOFunctions.js itself, and this plugin accepts such a package — content.xml at the root is the only thing it validates — so the injector's pair was added next to the package's own and the whole runtime was parsed and executed twice. Measured in a live Moodle before this change: an activity created from a SCORM export served a page carrying two wrapper tags and two SCOFunctions tags. The LMS-visible traffic survived it — one LMSInitialize, and one commit plus one finish at pagehide — but "it happens to be idempotent" is not a contract, and nothing was testing it. An ELPX, which references neither, was already fine. The injector now strips any script tag that loads either file before adding its own, so the page ends with exactly one of each, at the plugin's own relative depth. Same rule as the files themselves: the plugin's runtime wins, and there is only one of it.
The provenance tests checked the SHAPE of the vendored runtime: five layers, in order, stamped, with no local-change marker. None of that catches a copy with the wrong CONTENT. An edit inside a layer keeps every banner in place, keeps the stamp, and passes — which is exactly the drift the folder is supposed to be protected from. And the stamp cannot close the gap on its own: it names a release, and a release is built many times. assets/scorm/SOURCE now records the core repository, ref and full commit the copy was exported from, the version stamped into it, and the SHA-256 of both files. Two new tests read it: the files must match those digests, and the commit must be a full commit id rather than a branch name or a release string. Verified for the copy in the tree: re-exporting from the declared commit with `make export-scorm12` reproduces SCOFunctions.js byte for byte (sha256 567469a1…), and the wrapper is still the unmodified pipwerks file (f2a558ba…) recorded in THIRD-PARTY-NOTICES.md upstream.
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
2 participants
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.
Summary
The plugin side of exelearning/exelearning#2209: replace the legacy SCORM 1.2 runtime bundled by the Moodle plugin with the clean-provenance runtime introduced there, document the origin, build process, and licensing of the bundled third-party and eXeLearning code — and harden the SCORM grading channel itself, fixing two defects surfaced while aligning the xAPI contract (#121 / exelearning#2302) that were deliberately kept out of those xAPI-only PRs.
This change also addresses the documentation requested by external validation finding DIR-05.
Runtime changes
Replace
assets/scorm/SCORM_API_wrapper.jswith the unmodified upstream pipwerks SCORM API wrapper:v1.1.2018090682e455b4032ee08febf64d2fa2bf1aacaebaa446f2a558ba284edbc6842edf51678df1f7e3e05cbf09ec00bc6dd5988b6caa2e78Replace
assets/scorm/SCOFunctions.jswith the eXeLearning-owned SCORM 1.2 runtime, complete and byte-identical to what eXeLearning exports — all five layers in upstream's order, nothing dropped, nothing patched:exe-scorm12-client.jsexe-scorm12-activities.jsexe-scorm12-policy.jsexe-scorm12-lifecycle.jsexe-scorm12-adapter.jsThe copy carries upstream's version stamp, both as a header line and as
exeScorm12.runtimeVersion, so "which eXeLearning release is this?" has an answer:Always serve the plugin's runtime.
classes/local/package_manager.phpnow installs both files into every extracted package, deleting whatever the package brought with it. An activity in this plugin grades with the runtime the plugin ships and no other — including when what was uploaded is a SCORM export that carries its ownlibs/. The pair is installed together or not at all: the plugin's wrapper next to a package'sSCOFunctions.jswould pair files written against different wrapper versions, which nobody tests.Record pipwerks in
thirdpartylibs.xmlas the plugin's only third-party library.New
tests/local/scorm/scorm_runtime_test.php(5 tests) fails if the vendored copy is ever a subset, a locally patched file, an unstamped file, or a wrapper that is not upstream pipwerks. That test is what stops this folder drifting again.Exclude the generated
assets/scorm/SCOFunctions.jsfile from the repository ESLint scope.The new runtime is implemented from the SCORM 1.2 Run-Time Environment specification and is licensed under AGPL-3.0-or-later. It no longer contains ADL/CTC-derived code or Rustici Creative Commons fragments.
Documentation and licensing
assets/scorm/readme_moodle.txtwith:SCOFunctions.jsis first-party eXeLearning code rather than a third-party library.dist/readme_moodle.txtwith:dist/rather thandist/static/;Third-party codesection toREADME.mdexplaining:Why the four-layer subset is gone
The first version of this PR deliberately dropped
exe-scorm12-activities.jsto keepwindow.exeScorm12.activitiesabsent. Measured, that defence was unnecessary and the subset was a liability:LMSSetValuesequence, same final CMI;exe-scormbody class, soloadPage()never runs, the entry policy never runs, andcommon.jskeeps its legacycmi.suspend_datawriter whether the activity registry is installed or not;59c84359c, which landed 54 minutes after the vendoring commit).Complete file, stamped, plus a test that fails on drift: same behaviour, no hand assembly, and a copy whose provenance is a single answer.
Compatibility
No injector change is required.
classes/local/scorm/scorm_injector.phpcontinues to inject:libs/SCORM_API_wrapper.jslibs/SCOFunctions.jsThe replacement keeps the existing public contract, including the global lifecycle functions and the
scormfacade used by exported content.SCORM grading channel hardening
Two defects in the channel this PR owns, both verified against the code and both now fixed with regression tests. They matter more after exelearning#2302: with the xAPI emitter scoped to web exports, SCORM uploads grade permanently through this channel instead of through xAPI with their native runtime silenced.
track::ingest()ignored the master grading switch. Withgradeenabledoff the instance has no grade items, so the registered-objectid filter emptiesitemscores, the server-side recompute never runs — and ingest fell through to trusting the client's corruptiblecmi.core.score.raw, writing attempt rows, gradebook updates and lifecycle events for an activity its teacher configured as ungraded. The xAPI channel has had this guard (xapi\ingestor::ingest()) since its first version; the SCORM channel now mirrors it: acknowledge, write nothing. Preview keeps its own contract.captureItemScores()could silently drop a cross-page answer. Its change-detection baseline was keyed by the page-local slotN, which the legacysuspend_dataformat reuses across pages (the root cause exelearning#2209 removes for newly exported SCORM 1.2 packages by keying its versioned payload by stable id — this plugin-side fix is what covers SCORM 2004 and every already-published package). A page-2 iDevice landing on a slot whose page-1 occupant had the same score and weight compared equal against the stale baseline and was dropped: its gradebook column was simply never written, with nothing logged anywhere. The baseline is now keyed by objectid — two different iDevices are never "unchanged" relative to each other — and carries forward entries for unloaded pages, so returning to a page re-emits nothing until a score actually changes and the write-avoidance optimisation survives.Known limitation, deliberately not fixed here
track::recompute_overall_pct()still normalises the overall over the reported item map. A partial attempt on a multipage package is therefore inflated (weights 60/40, 50% on only the 60-item → published 50, while counting every gradable iDevice of the package would give 30); single-page packages are unaffected because eXeLearning zero-seeds every gradable iDevice of a loaded page intosuspend_data. If an exact package-wide denominator is ever wanted, the weights must be parsed server-side from the teacher's content.xml at sync time — never learned from learner statements (#121 was closed on that principle). With SCORM as the grading channel, the visited-pages denominator matches eXeLearning's own native semantics and the mod_scorm/mod_h5pactivity precedent. Tracked in this PR's follow-up list.Behavior changes to verify
incompletestatus is no longer reset tonot attemptedwhen a page is reopened.pagehideandvisibilitychangeinstead of relying only onunload.cmi.core.exitis empty after a terminal status andsuspendotherwise.The legacy Moodle-specific window flags, such as
mod_scorm_is_window_closing, were intentionally not copied. Any compatibility issue requiring them should be reported to the main eXeLearning repository and evaluated against ADR-0001.Verification
readme_moodle.txtfiles are included in the generated release ZIP next to the code they document.buildScorm12RuntimeFiles()emits upstream.tests/lib_extract_test.phpcovers the new rule end to end: a package that brings its ownlibs/SCOFunctions.jscomes out of extraction carrying the plugin's.Follow-up
"weighted"sits beside"isScorm"), and count unreported-but-weighted iDevices as 0. Never learn weights from learner statements — Reconstruct xAPI weighted scores across pages #121 was closed for exactly that reason (see its closing note); with SCORM as the grading channel (xapiprimaryenabled=0) the current visited-pages denominator is the accepted semantic.Moodle Playground Preview
The changes in this pull request can be previewed and tested using a Moodle Playground instance.
ℹ️ The eXeLearning editor is fetched from the shared release and unpacked into the plugin when the playground boots, so the first load may take a few extra seconds. ELPX upload, viewer and preview work normally.