Posting a malformed trigger to the reference leaves a task file that throws on its next start #126

New issue

Closed

opened 2026-08-14 00:56:38 +02:00 by Claude · 1 comment

Claude commented

2026-08-14 00:56:38 +02:00

Collaborator

A hazard for anyone measuring against the reference, found the hard way during #95.

Upstream's Triggers setter calls SaveTriggers before GetTrigger validates what was saved. So:

POST /ScheduledTasks/{id}/Triggers   [{"Type":"DailyTrigger"}]      (no TimeOfDayTicks)
  -> 400, and a file is left at config/ScheduledTasks/<guid>.js

The file then throws inside ScheduledTaskWorker's constructor on the next start. The instance does not come back cleanly until somebody deletes or repairs it — and the failure names neither the task nor the file, the same way M6 found that a failed migration reads as ObjectDisposedException: LoggerFactory.

It was hit and repaired during #95 (MoveTrickplayImages' file back to [], verified, and the reference confirmed healthy afterwards: 20 tasks, all idle, every trigger at its documented default).

What to do about it

Not a defect in this port — it is upstream's, and it is not ours to fix. What is missing is that nothing warns the next person. Two cheap options, and the first is probably enough:

A line in crates/jellyfin-harness/README.md beside the other rules about what a replay does to the reference, saying that trigger posts are a write with a failure mode that outlives the request, and that a scenario carrying one must post only well-formed triggers.
A check in cargo xtask reference that reports whether any config/ScheduledTasks/*.js fails to parse, so a broken fixture is a message rather than a container that will not start.

The general rule this is an instance of is already in the harness README — a replay that writes to the reference puts the pair out of agreement — but the version that bites here is worse than disagreement, because it survives a restore of nothing but the database.

House rules

Tests run under cargo nextest; upstream's source is the checkout in upstream/ at the pin; generated files are regenerated rather than edited; comments say what the code does, not how it got there. crates/jellyfin-harness/README.md says what a diff means and what an ignore entry must carry. A server started off current main binds UDP 7359 — set [discovery] enabled = false in a scratch config when another one is running.

A hazard for anyone measuring against the reference, found the hard way during #95. Upstream's `Triggers` setter calls `SaveTriggers` **before** `GetTrigger` validates what was saved. So: ``` POST /ScheduledTasks/{id}/Triggers [{"Type":"DailyTrigger"}] (no TimeOfDayTicks) -> 400, and a file is left at config/ScheduledTasks/<guid>.js ``` The file then throws inside `ScheduledTaskWorker`'s constructor on the next start. The instance does not come back cleanly until somebody deletes or repairs it — and the failure names neither the task nor the file, the same way M6 found that a failed migration reads as `ObjectDisposedException: LoggerFactory`. It was hit and repaired during #95 (`MoveTrickplayImages`' file back to `[]`, verified, and the reference confirmed healthy afterwards: 20 tasks, all idle, every trigger at its documented default). ## What to do about it Not a defect in this port — it is upstream's, and it is not ours to fix. What is missing is that **nothing warns the next person**. Two cheap options, and the first is probably enough: - A line in `crates/jellyfin-harness/README.md` beside the other rules about what a replay does to the reference, saying that trigger posts are a write with a failure mode that outlives the request, and that a scenario carrying one must post only well-formed triggers. - A check in `cargo xtask reference` that reports whether any `config/ScheduledTasks/*.js` fails to parse, so a broken fixture is a message rather than a container that will not start. The general rule this is an instance of is already in the harness README — a replay that writes to the reference puts the pair out of agreement — but the version that bites here is worse than disagreement, because it survives a restore of nothing but the database. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**; generated files are regenerated rather than edited; comments say what the code does, not how it got there. `crates/jellyfin-harness/README.md` says what a diff means and what an ignore entry must carry. A server started off current `main` binds UDP 7359 — set `[discovery] enabled = false` in a scratch config when another one is running.

Claude added the

testing

labels

2026-08-14 00:56:38 +02:00

Claude referenced this issue

2026-08-14 02:01:20 +02:00

Nothing runs on a schedule: the triggers a dashboard renders are decoration #95

Claude commented

2026-08-20 18:29:08 +02:00

Author

Collaborator

Landed in dadcb8c — the first of the two options, as suggested.

crates/ephyra-harness/README.md now carries the warning beside the other rule
about what a replay leaves on the reference, rather than only under
tasks.json. The tasks.json bullet cross-references it instead of restating
the mechanism.

One correction to the issue while writing it: a restore does clear the file,
as long as it is the config tree rather than the database. Fixture::install
does remove_dir_all on the target and copies the snapshot's whole config
over it, excluding only metadata, and the default snapshot carries no
ScheduledTasks directory — so cargo xtask reference up clears a stray one.
Only putting the database back on its own leaves it. The README says which, and
that writing the file back to [] repairs it in place.

The second option — a check in cargo xtask reference that reports a
config/ScheduledTasks/*.js which will not parse — is not done. Say if it
is still wanted; the README line is what this issue judged sufficient.

Landed in `dadcb8c` — the first of the two options, as suggested. `crates/ephyra-harness/README.md` now carries the warning beside the other rule about what a replay leaves on the reference, rather than only under `tasks.json`. The `tasks.json` bullet cross-references it instead of restating the mechanism. One correction to the issue while writing it: **a restore does clear the file**, as long as it is the config tree rather than the database. `Fixture::install` does `remove_dir_all` on the target and copies the snapshot's whole `config` over it, excluding only `metadata`, and the default snapshot carries no `ScheduledTasks` directory — so `cargo xtask reference up` clears a stray one. Only putting the database back on its own leaves it. The README says which, and that writing the file back to `[]` repairs it in place. The second option — a check in `cargo xtask reference` that reports a `config/ScheduledTasks/*.js` which will not parse — is **not** done. Say if it is still wanted; the README line is what this issue judged sufficient.

Claude closed this issue

2026-08-20 18:29:16 +02:00