Skip to content

[ENHANCEMENT] [MER-5672] Adaptive Lesson: RC I Exploration — Decoding the Mystery of Greenhouse Molecules (Playwright)#6731

Merged
nicocirio merged 18 commits into
masterfrom
enhancement/MER-5672-adaptive-lesson-rc-1-exploration-playwright
Jul 17, 2026
Merged

[ENHANCEMENT] [MER-5672] Adaptive Lesson: RC I Exploration — Decoding the Mystery of Greenhouse Molecules (Playwright)#6731
nicocirio merged 18 commits into
masterfrom
enhancement/MER-5672-adaptive-lesson-rc-1-exploration-playwright

Conversation

@simonchoxx

@simonchoxx simonchoxx commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

MER-5672

Summary

Adds the Playwright test for the adaptive lesson "Exploration: Decoding the Mystery of Greenhouse Molecules" (Unit 5, REAL CHEM I), which drives the lesson's 33+ screens through the happy path (correct answers only) against a real imported course. Since the course and its correct answers are private content (course IP), this PR also adds new infrastructure so the test can fetch them from a private bucket instead of requiring local files or presigned URLs.

Context

The project is a full course; the test needs to navigate to the correct page within it. The lesson has 33 screens total (the student may not hit every one).

All course content — the lesson title, search term, completion text, and every correct answer — lives in a private answers JSON that must never be committed to the repo (it's public). Both that JSON and the course zip are stored in a private bucket (MinIO in dev, S3 in other environments), and the server serves them through an authenticated endpoint, so no developer needs to copy files around by hand.

What changed

Private asset infrastructure for Playwright:

  • New GET /test/assets/*path endpoint, serving files from a dedicated bucket (Oli.Scenarios.PlaywrightAssetStorage), authenticated with the same scenario token already used by the Playwright scenario runner.
  • Extracted the token-based authorization into a shared module (OliWeb.PlaywrightAuth) with a constant-time comparison, reused by both endpoints.
  • The test downloads the course and answers from that bucket instead of relying on local paths or presigned URLs — zero file configuration to run the test.

Security hardening of that infrastructure:

  • Neither the bucket name (PLAYWRIGHT_ASSETS_BUCKET) nor the automation API key (PLAYWRIGHT_AUTOMATION_API_KEY) has a default value in code — both must be configured explicitly. This avoids, respectively, an S3 bucket-squatting risk (bucket names are globally unique) and having a public, guessable credential in the repo for an endpoint (/api/v1/automation_setup) that is reachable in every environment.
  • CHANGELOG.md updated to document both new environment variables.

Test stability, found by running it repeatedly:

  • The adaptive deck's clicks are now bounded by a timeout (previously they could hang indefinitely on some screens).
  • The matching widget now reports a clear error when it fails to link a pair, instead of silently continuing.
  • The course archive download happens outside Playwright's traced request context, avoiding an intermittent trace corruption issue with large buffers.

Verification

  • The spec was run multiple times end-to-end against a real local server (MinIO + Postgres), completing all 33+ screens and reaching the completion screen.
  • Full Elixir test suite: 7977 tests + 26 doctests, no failures related to this change.
  • mix format, tsc, eslint, and prettier all clean on the touched files.
  • Verified the branch merges cleanly against master (no conflicts).

Running the test locally

1. Required environment variables (add to your oli.env — no defaults, the app won't start this feature without them):

PLAYWRIGHT_SCENARIO_TOKEN=<a_value_you_choose>
PLAYWRIGHT_ASSETS_BUCKET=<your_bucket_name>
PLAYWRIGHT_AUTOMATION_API_KEY=<key_generated_at_/admin/api_keys>
  • PLAYWRIGHT_ASSETS_BUCKET: the name of the bucket in your local MinIO (or S3) where you'll upload the private assets. No default — pick any name, as long as it matches the bucket you actually create.
  • PLAYWRIGHT_AUTOMATION_API_KEY: generate this once, logged in as admin, at /admin/api_keys — fill in a hint, click "create" (generates a random value), toggle automation_setup_enabled on for it. Never reuse a value that appears in the repo or in this PR — the endpoint it protects is reachable in every environment.

2. Seed the bucket once. Inside the bucket, create a mer-5672/ folder with these two files:

<your-bucket>/
└── mer-5672/
    ├── real-chem-course.zip
    └── answers.json

(ask the team for both files — they're private content, not in the repo).
Only the bucket name is configurable (via PLAYWRIGHT_ASSETS_BUCKET) — the mer-5672/ folder and both file names are hardcoded in this test's spec, so they must match exactly. The answers JSON's structure is also specific to this test; a future test for a different lesson would define its own schema.

3. Start the server with those env vars loaded, then run:

cd assets/automation
npx playwright test real-chem-greenhouse-molecules

The test takes 5-7 minutes (33+ screens with server-side evaluation on each). If any of the env vars above are missing, the test is skipped with an explicit message indicating which one.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️ PR is large (1497 LOC changed). Consider splitting.

Risk score: 12 → risk/high

Generated by 🚫 dangerJS against d9ede05

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

AI Review — performance

Stream private assets instead of buffering them

file: lib/oli_web/controllers/playwright_support_asset_controller.ex
line: 55
Description: send_resp/3 serves an S3 object already loaded as one binary, so large course archives consume their full size in memory per concurrent request and can cause memory pressure.
Suggestion: Stream the S3 response through send_chunked/2 with bounded chunks, or enforce a strict maximum object size before loading it.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

AI Review — security

Unbounded asset downloads can exhaust server resources

file: lib/oli/scenarios/playwright_asset_storage.ex
line: 29
Description: S3.get_object/2 buffers the entire object in memory without enforcing a maximum size. Repeated or unexpectedly large asset requests could exhaust application memory and bandwidth.
Suggestion: Enforce a strict object-size limit using S3 metadata before downloading, and stream approved objects to the response instead of buffering them fully.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

AI Review — elixir

No issues found

@simonchoxx

Copy link
Copy Markdown
Contributor Author

AI Review — performance

Stream private assets instead of buffering them

file: lib/oli_web/controllers/playwright_support_asset_controller.ex line: 55 Description: send_resp/3 serves an S3 object already loaded as one binary, so large course archives consume their full size in memory per concurrent request and can cause memory pressure. Suggestion: Stream the S3 response through send_chunked/2 with bounded chunks, or enforce a strict maximum object size before loading it.

Not addressing this — this endpoint only serves a single local Playwright run at a time (gated behind enable_playwright_scenarios, off in prod, plus the scenario token) — there's no realistic concurrent-download scenario to cause memory pressure. The objects are tens-of-MB course archives, well within what a single BEAM process buffers comfortably. Would reconsider if this endpoint ever gained real concurrent traffic.

@simonchoxx

Copy link
Copy Markdown
Contributor Author

AI Review — security

Unbounded asset downloads can exhaust server resources

file: lib/oli/scenarios/playwright_asset_storage.ex line: 29 Description: S3.get_object/2 buffers the entire object in memory without enforcing a maximum size. Repeated or unexpectedly large asset requests could exhaust application memory and bandwidth. Suggestion: Enforce a strict object-size limit using S3 metadata before downloading, and stream approved objects to the response instead of buffering them fully.

Not adding a size cap for now, for the same reason: this endpoint is unreachable without the scenario token and compiled out of prod, and the objects served are known course archives that developers seed into the bucket themselves — not arbitrary or attacker-controlled uploads. A cap makes sense if this endpoint ever serves less-controlled content.

@nicocirio nicocirio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I ran the test locally after following the env var setup described here, and it completed successfully.

Image

Suggestion: I’m fine with approving this as-is, but I want to call out the tradeoff. We’re reusing the existing automation_setup flow for the full test seed path, which is pragmatic, but it also means every environment needs an admin-created API key exposed to the test runner as PLAYWRIGHT_AUTOMATION_API_KEY (created from /admin/api_keys, with automation_setup_enabled enabled).
It looks like there was another possible direction here: extending the existing Playwright scenario path with a custom hook to handle the archive-ingest/setup flow. I don’t think that exists as a native scenario directive today, so it likely would have required extra scenario infrastructure or at least a purpose-built hook. Reusing automation_setup is simpler in the short term, but it does add a per-environment bootstrap step compared to a dedicated server-side secret or a scenario-based setup path.

@nicocirio
nicocirio merged commit f1be066 into master Jul 17, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants