docs(architecture): adopt tracking-number identifiers, and add agent skills

.agents/skills/architecture-records/SKILL.md

@@ -0,0 +1,145 @@
+---
+name: architecture-records
+description: "Use when writing, renaming or reviewing an ADR or change document in this repository: how the tracking-number identifier works when issues are disabled, what the validator enforces, and why the index is never committed."
+compatibility: "One shared TypeScript file, run with the Node that ships on the CI image. No build step, no dependencies. Run with `make architecture-check`."
+---
+
+# Architecture records in this repository
+
+## When to use
+
+Before creating a file under `docs/architecture/`, before renaming one, and when
+`make architecture-check` fails and the message is not self-explanatory.
+
+## The identifier, and the one thing people get wrong
+
+A record is named after the **GitHub tracking number** of the change it belongs
+to:
+
+```text
+docs/architecture/adr/ADR-<number>-<NN>-<decision-slug>.md
+docs/architecture/changes/<number>-<change-slug>/
+```
+
+**There is no global counter.** Do not look at the existing records and pick the
+next integer. That rule is what this convention replaces: every branch evaluates
+it against its own working tree, two branches pick the same number, and because
+the collision is in the *filename* Git merges both files cleanly and reports
+nothing.
+
+**Issues are disabled on this repository:**
+
+```console
+$ gh api repos/exelearning/nextcloud-exelearning -q .has_issues
+false
+```
+
+So the tracking number is always a **pull request** number. Do not try to open an
+issue to get one — you cannot, and you should not want to.
+
+### The chicken-and-egg case
+
+A record written before its PR exists has no number yet. The sequence is:
+
+1. Write the record with a placeholder name.
+2. Push the branch and open the PR.
+3. Rename the file to the PR's number, set `id` and `tracking_issue`, fix the H1.
+4. Run `make architecture-check`.
+
+One rename, before review. That is the cost of not having a counter, and it is
+cheaper than a silent collision.
+
+### The two-digit sequence
+
+`<NN>` is scoped to that tracking number **only**, starts at `01`, and is present
+even when a change has a single ADR. That last part matters: it means adding a
+second decision later never renames the first one, so inbound links stay valid.
+
+`ADR-42-01` and `ADR-43-01` coexist happily. Two `ADR-42-01` do not.
+
+## What the validator enforces
+
+`make architecture-check` runs `tools/architecture-records.mts`. It fails on:
+
+| Category | Examples |
+|---|---|
+| Grammar | filename not `ADR-<n>-<NN>-<slug>.md`; slug not kebab-case; leading zeros; retired `ADR-NNNN` form |
+| Agreement | `id` ≠ filename; `tracking_issue` ≠ filename; H1 ≠ `# <id>: <title>` |
+| Uniqueness | duplicate `id`; duplicate sequence within one number |
+| Vocabulary | ADR status outside `Proposed/Accepted/Rejected/Superseded`; change status outside the lowercase set |
+| Shape | non-calendar dates; non-integer issue/PR references |
+| References | `related.adrs`, `related.changes`, `related_adrs` that resolve to nothing |
+| Supersession | one-sided `supersedes`/`superseded_by`; a superseded record not set to `Superseded` |
+| Hygiene | a retired identifier anywhere in the tree; a committed `records.md` |
+
+`tools/architecture-records.mts` is a **verbatim copy** of the canonical file in
+`exelearning/exelearning`, where it is covered by `bun test`. Do not edit it
+here: fix it in core and re-copy, or the copies drift apart — which is the whole
+failure mode this convention exists to prevent. Running `check` in CI exercises
+the file under Node on every pull request, so a runtime-specific breakage still
+surfaces here.
+
+## The index is not a file
+
+There is no `records.md`. Print it:
+
+```bash
+make architecture-records
+```
+
+It is derived entirely from frontmatter. Committing it would guarantee a merge
+conflict on every concurrent branch — the exact problem this convention removes —
+so the validator treats its presence as an error.
+
+If you want a rendered index somewhere, generate it at that moment. Do not
+reintroduce the file.
+
+## Status is frontmatter-only
+
+Never add a `## Status` section. It duplicates `status:` and drifts from it. One
+canonical source per mutable field; the validator has no way to catch a stale
+prose heading, which is precisely why the heading is banned rather than checked.
+
+## ADR or change document?
+
+- **ADR** — a durable *decision*, append-only. "We will serve published content
+  from an opaque iframe."
+- **Change document** — the *design* for a unit of work, historical once shipped.
+  "Here is how the opaque viewer is built, tested and rolled out."
+
+A change usually contains several ADRs. Extract the decisions that will outlive
+the feature; link them with `related_adrs`. Do **not** create one ADR per section
+of a design — that is the most common failure mode, and it produces records
+nobody can cite.
+
+## What deserves an ADR here
+
+Decisions about the Service Worker scope and the iframe sandbox; the editor
+`postMessage` contract; whether `.elpx` packages stay opaque; entry-path
+normalization as the security boundary; the Nextcloud integration surface
+(controllers, preview provider, OCP usage); release packaging and what ships.
+
+Not: bug fixes restoring intended behaviour, routine refactors, dependency bumps,
+or anything with no cross-cutting consequence.
+
+## Superseding
+
+Accepted records are append-only. To change one:
+
+1. Write a new ADR under the number that motivates the change.
+2. New record: `supersedes: [ADR-<old>]`.
+3. Old record: `status: Superseded` and `superseded_by: [ADR-<new>]`.
+
+Both directions are required — CI rejects a one-sided relationship, and it also
+rejects a superseded record left at `Accepted`.
+
+## Common failures and their fix
+
+| Message | Fix |
+|---|---|
+| `uses the retired global numbering` | Rename to the tracking-number form. |
+| `frontmatter id "X" does not match filename` | The filename is authoritative; fix `id`. |
+| `H1 is "…" but should be "…"` | The H1 mirrors `id` and `title` exactly, including punctuation. |
+| `references unknown ADR` | The target does not exist on this branch. Fix or drop it. |
+| `the record index must not be committed` | `git rm` the `records.md`. |
+| `references retired identifier` | Use the current identifier. |

.agents/skills/elpx-package-safety/SKILL.md

@@ -0,0 +1,99 @@
+---
+name: elpx-package-safety
+description: "Use when touching ZIP entry handling, entry-path normalization, the Service Worker, or the sandboxed iframe. Entry-path normalization is this app's security boundary and it currently has three implementations that do NOT agree."
+compatibility: "PHP ZipEntryService, TypeScript src/elpx/paths.ts, and the hand-written Service Worker src/sw/exelearning-sw.js."
+---
+
+# `.elpx` package handling and its security boundary
+
+## When to use
+
+Before changing anything that turns an untrusted string into a ZIP entry or a
+runtime URL: `lib/Service/ZipEntryService.php`, `src/elpx/*`,
+`src/sw/exelearning-sw.js`, or any new helper that handles entry paths.
+
+## The boundary
+
+A `.elpx` package is a ZIP whose entry names are **attacker-controlled**. Anyone
+who can upload a file to Nextcloud can craft one. The only thing standing between
+a crafted entry name and reading outside the package is path normalization.
+
+`AGENTS.md` states the rule: any new helper that handles entries must call
+`normalizeEntryPath` (TS) or `ZipEntryService::normalizeEntry` (PHP). Follow it.
+Never inline your own check, never "just" `str_replace('..', '')` — that is
+defeated by `....//`.
+
+## There are three implementations, and they disagree
+
+| Implementation | Location |
+|---|---|
+| PHP | `lib/Service/ZipEntryService.php::normalizeEntry` |
+| TypeScript | `src/elpx/paths.ts::normalizeEntryPath` |
+| Service Worker | `src/sw/exelearning-sw.js::normalizeEntry` |
+
+The SW copy is an inline mirror of the TS one — deliberately, because the SW is
+loaded out-of-band by the browser and cannot import bundled application code.
+Those two agree. **The PHP one does not.** Measured behaviour:
+
+| Input | PHP | TS / SW |
+|---|---|---|
+| `a/b/c` | `a/b/c` | `a/b/c` |
+| `../escape` | `null` | `null` |
+| `a/b/../c` | **`null`** | **`a/c`** |
+| `a/./b` | **`null`** | **`a/b`** |
+| `a//b` | **`a//b`** | **`a/b`** |
+
+PHP rejects any `.` or `..` segment outright. TS/SW resolve them and only reject
+an attempt to escape the root. PHP also keeps an empty segment from a doubled
+slash; TS/SW collapse it.
+
+**This is not a traversal hole.** Both reject `../escape`; neither escapes the
+package root. It is a *consistency* defect: a package containing `a/b/../c`
+renders in the browser but 404s from the PHP asset controller and the preview
+provider, and the difference is invisible until someone ships such a package.
+
+The docblock on `normalizeEntryPath` claims it "matches the rule used by the
+PHP-side `ZipEntryService`". **That comment is wrong**, which is the dangerous
+part: it tells the next maintainer the two agree.
+
+### What to do about it
+
+- **Do not** treat the two as interchangeable when reasoning about behaviour.
+- If you change one, change all three, and add a shared test vector table so the
+  divergence cannot silently return.
+- Converging them is a behaviour change to a security boundary. That deserves an
+  ADR and its own PR — not a drive-by edit inside an unrelated change.
+
+## Service Worker scope
+
+Only `/apps/exelearning/runtime/` may be intercepted. The SW must never see
+arbitrary Nextcloud URLs. Widening the scope is a security decision, not a
+convenience: it puts the SW in front of authenticated Nextcloud responses.
+
+`RUNTIME_PREFIX` and `ASSET_PREFIX` in `src/elpx/paths.ts` are the single place
+where those URLs are built. Build runtime URLs there, not by string concatenation
+at the call site.
+
+## The iframe
+
+Package content renders in a **sandboxed** iframe. Anything that relaxes the
+sandbox attributes, or adds an origin to what the frame may reach, changes the
+trust boundary between untrusted package content and the Nextcloud session. Write
+an ADR before doing it.
+
+## Packages stay opaque
+
+Do not parse `content.xml`. Do not regenerate, patch or rewrite package contents.
+Saving is a passthrough: the editor's exported bytes are written back unchanged.
+Every deviation from this makes the app responsible for a format it does not own.
+
+## Testing
+
+Entry-path handling is pure and has no excuse for being untested:
+
+- `tests/js/paths.test.ts` — the TS normalizer.
+- `tests/Unit/Service/ZipEntryServiceTest.php` — the PHP one.
+
+Any new case you reason about — a crafted separator, an empty segment, a NUL
+byte, a Windows path, a unicode look-alike — becomes a test case in both, or it
+is not covered.

.agents/skills/nextcloud-app-development/SKILL.md

@@ -0,0 +1,124 @@
+---
+name: nextcloud-app-development
+description: "Use when touching lib/ or appinfo/ in this Nextcloud app: how the app bootstraps, where controllers/services/routes belong, dependency injection, the preview provider, what OCP surface is allowed, and the traps that only show up on a real Nextcloud."
+compatibility: "Nextcloud app framework, PHP 8.1+, namespace OCA\\ExeLearning. Unit tests run with no Nextcloud, no database and no web server."
+---
+
+# Developing this Nextcloud app
+
+## When to use
+
+Before adding or changing anything under `lib/`, `appinfo/` or `templates/`, and
+before deciding whether some new behaviour belongs server-side at all.
+
+## The shape of the app
+
+```text
+appinfo/info.xml         app id, version, dependencies, declared types
+appinfo/routes.php       every HTTP route, one entry per controller method
+lib/AppInfo/Application.php
+                         bootstrap: init script, preview provider registration
+lib/Controller/*.php     the HTTP boundary — one controller per concern
+lib/Service/*.php        the logic — package lookup, permissions, ZIP entries
+lib/Preview/ElpxPreviewProvider.php
+                         Nextcloud preview provider for .elpx
+```
+
+Controllers today: `Asset`, `Editor`, `Package`, `Sw`, `Template`, `Thumbnail`,
+`View`. Services: `ElpxPackageService`, `PermissionService`, `ZipEntryService`.
+
+Keep that split. A controller parses and validates the request, calls a service,
+and returns a response. Logic that could be unit-tested without HTTP belongs in a
+service — that is the whole reason the tests can run with no Nextcloud present.
+
+## Dependency injection
+
+**Constructor injection only.** Never reach into the container, never call a
+singleton locator, never `new` a service inside a controller. Nextcloud's
+`QueryBuilder`-style autowiring resolves constructor type hints; a service that
+takes its collaborators as constructor arguments is also a service you can
+instantiate directly in a unit test with fakes.
+
+`lib/AppInfo/Application.php` is the only place that registers things globally
+(the init script and the preview provider). Resist adding more there: anything
+registered at boot runs for every Nextcloud page load, including pages that have
+nothing to do with this app.
+
+## Routes
+
+Every route goes in `appinfo/routes.php`. Two rules that bite:
+
+- The route name must match `Controller#method` exactly, or Nextcloud 404s with
+  no useful message.
+- A route that serves package bytes must go through `PermissionService` before
+  `ZipEntryService`. Never trust a file id from the request: resolve it through
+  the user's own storage so Nextcloud's own permission model applies.
+
+## What must stay out of the server
+
+From `AGENTS.md`, and worth repeating because it is the most common drift:
+
+- **Do not parse `content.xml`.** The viewer needs `index.html` and the package
+  assets. `ZipEntryService` is deliberately restricted to named entries;
+  `screenshot.png` is the only entry the preview provider pulls out by name.
+- **Keep `.elpx` opaque.** Saving is a passthrough — the editor's exported bytes
+  are written back unchanged. Do not regenerate, patch or rewrite packages
+  server-side.
+- **No non-Nextcloud backend.** Anything server-side is a controller, a service
+  or a preview provider. Authentication, CSRF and permissions come from
+  Nextcloud's APIs, not from anything hand-rolled.
+- **Do not modify Nextcloud core MIME files.** Admin-side configuration is
+  documented in the README.
+
+## The preview provider
+
+`ElpxPreviewProvider` is registered in `Application.php`. Preview generation runs
+in contexts where the user session may not be what you expect and where failures
+are silent — a provider that throws just yields no thumbnail. So:
+
+- Fail by returning null, not by throwing.
+- Do not assume a local file: `ZipEntryService` has a stream fallback that copies
+  to a temp file for object storage and external mounts. Any new entry reader
+  must keep that fallback, or the app breaks on exactly the installs that are
+  hardest to debug.
+- Always `@unlink` temp files in a `finally`.
+
+## Strict types and namespace
+
+PHP 8.1+, `declare(strict_types=1)` in **every** file, namespace
+`OCA\ExeLearning`. English in code, identifiers, comments and documentation.
+
+## Documentation
+
+Use Context7 MCP for Nextcloud app framework and `@nextcloud/*` documentation.
+The framework's APIs move, and a plausible-looking method that does not exist in
+the target Nextcloud version fails only on a real install — long after your unit
+tests went green. Prefer Context7 over recalling an API from memory.
+
+`appinfo/info.xml` declares the supported Nextcloud versions; check it before
+using anything recent.
+
+## Before claiming success
+
+Unit tests run without Nextcloud. That is a feature, and also a limitation: they
+cannot tell you a route is misnamed, a service is unresolvable by the container,
+or a preview provider is not registered. Those need a real install.
+
+```bash
+composer install
+npm run typecheck
+npm test
+vendor/bin/phpunit --configuration tests/phpunit.xml
+make architecture-check
+```
+
+If a change touches routing, DI registration or the preview provider, say
+explicitly in the PR that it was not exercised against a running Nextcloud, if it
+was not. Do not present a green unit suite as evidence for something the unit
+suite structurally cannot check.
+
+## Recording decisions
+
+Decisions about the Nextcloud integration surface — what runs at boot, which OCP
+APIs the app depends on, how permissions are enforced, what the preview provider
+guarantees — are durable. Write an ADR. See the `architecture-records` skill.