From 8d80c51a9c742c8c0ec90d914a293dd192109081 Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Sat, 22 Aug 2026 04:45:06 +0200 Subject: [PATCH] Refuse a run that would lose a plugin it could not order (#114) `internal/posture` ordered a plugin's releases by publication time and treated index 0 as the release the run exists to publish. Where no release in the set carried a time, none of `newerFirst`'s clauses fired, the order fell through to the tag, and the highest tag arrived at index 0 -- the string decisions/manifest-schema.md refuses as a version answering the question the publication time exists to answer. A defect in that release then stopped the whole run, taking every other plugin's catalogue down with it, over a published release nobody here can repair. The classification now says such a set has no newest release, so every defect in it is a named skip. That alone moves the failure rather than removing it: the plugin resolves, contributes nothing, and the catalogue is quietly one entry shorter with a zero exit, which decisions/failure-posture.md spends its longest section on being the worse of the two. So `catalogue.JudgeDropped` lands with it and refuses a run in which a plugin that resolved produces no entry and nothing stops. decisions/failure-posture.md places both, and one correction beside them. Its last section said nothing refused any of the posture and named five issues as where the refusals would arrive; all five are closed and their refusals are in the tree, so that sentence had stopped being true. It was found while writing the two conditions above, which had to say what refuses them and could not do that beside a paragraph claiming nothing does. The means is Go and the existing suites, because both rules are refusable properties over values the packages already hold and both are proven by fixtures the gate already runs. No dependency is added and go.sum is untouched. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- decisions/failure-posture.md | 68 +++++++++++++++++++++++++-- internal/catalogue/catalogue.go | 42 +++++++++++++++++ internal/catalogue/catalogue_test.go | 69 ++++++++++++++++++++++++++++ internal/posture/posture.go | 17 ++++++- internal/posture/posture_test.go | 59 ++++++++++++++++++++++++ 5 files changed, 251 insertions(+), 4 deletions(-) diff --git a/decisions/failure-posture.md b/decisions/failure-posture.md index 16a8ff1..34ca992 100644 --- a/decisions/failure-posture.md +++ b/decisions/failure-posture.md @@ -49,6 +49,15 @@ A version cap, an ordering rule or a byte-format rule that cannot be applied. Th are this project's own rules and a run that cannot apply one has a bug rather than an input problem. +A run in which a plugin that resolved would contribute no entry. The declaration +answered, the repository has a finished release, and the catalogue the run is +about to place is one entry shorter than the one before it. A server polling the +address reads that as a plugin that has stopped existing, and it is the same +shape as the empty catalogue below with one plugin instead of all of them, so it +is fatal for the same reason. Naming the skips in the output does not cover it: +the output is read by whoever is watching a run and the catalogue is read by +every server. + ## What is a loud skip The run continues, publishes what it could resolve, and names every skipped release @@ -69,6 +78,47 @@ disagree. All four cases and their reasons are `decisions/artifact-checksum-pairing.md`, and this file only places them: on an older release they are skips, on the newest one they are fatal. +Every release in a set where none of them carries a publication time. This is the +case an ordering rule cannot be applied to, and it does not fall on the fatal side +above, because that sentence reasons from a run that cannot apply one of its own +rules having a bug. A release list with no times in it is an input rather than a +bug, and no release in such a set is ever classified as the newest: the only order +left is the tag, which `decisions/manifest-schema.md` refuses as a version string, +and taking the highest tag for the newest release would stop the world over a +release nobody here can repair. + +What that leaves is a plugin whose releases were all skipped and which publishes +nothing, and that is the fatal condition above rather than an acceptable outcome. +The two land together on purpose. Without the second, this decision converts a +run that stops too eagerly into a catalogue that loses a plugin quietly with a +zero exit, and the second of those is the one this file spends its longest +section on. + +No repository in the declared set is in this state: + + for f in sources/*.json; do + r=$(basename "$f" .json) + printf "%s %s\n" "$r" "$(gh api "repos/Flowfin/jellyfin-plugin-$r/releases?per_page=100" \ + --jq '[.[] | select(.published_at == null)] | length')" + done + discover 0 + invites 0 + metadata-sync 0 + requests 0 + server-pairing 0 + share-links 0 + smart-collections 0 + sso 0 + stats 0 + watchlist 0 + watch-sync 0 + whisper-subtitles 0 + +Run 2026-08-22. So this rule refuses fixtures and is expected to go on doing +so until some upstream repository publishes a release +list without dates, which means its first real instance will surprise somebody +and the fixtures are the only thing standing between it and dead code. + A release trimmed by the per-target cap. Named as trimmed rather than as defective, because a run that reports it the same way as a broken release teaches everybody to ignore both. `decisions/version-cap.md` is the rule. @@ -128,6 +178,18 @@ found nothing. ## What refuses a violation -Nothing yet. This file is prose and prose does not stop a run. The conditions above -become refusals in #24, #25, #27 and #28, and the verdict shape they share is #18. -Until those land, the posture is a rule the generator does not exist to break. +`internal/posture` classifies one plugin's releases and `internal/catalogue` +refuses the run, and which conditions each of them decides is printed by the +suite rather than listed here, because a list in this file drifts against the +code that holds it: + + go test ./internal/posture ./internal/catalogue -count=1 -v | grep '^=== RUN' + +This section said nothing refused any of it, and named #24, #25, #27, #28 and +#18 as where the refusals would arrive. All five are closed and their refusals +are in the tree, so the sentence had stopped being true. It was found while +adding the two conditions above, which needed to say what refuses them and could +not do that beside a paragraph claiming nothing does. + +What is still prose is the shape of the report, which is a judgement about +wording that no reading of the tree makes. diff --git a/internal/catalogue/catalogue.go b/internal/catalogue/catalogue.go index 66ece08..ff6d067 100644 --- a/internal/catalogue/catalogue.go +++ b/internal/catalogue/catalogue.go @@ -31,6 +31,7 @@ import ( "errors" "fmt" "io" + "strings" "sync" "flowfin.dev/hub/internal/identity" @@ -83,6 +84,9 @@ func (r Route) Publish(ctx context.Context, out io.Writer, declarations []source if err := posture.Judge(plans); err != nil { return err } + if err := JudgeDropped(plans); err != nil { + return err + } if err := Judge(plugins); err != nil { return err } @@ -214,6 +218,44 @@ func identityOf(plugin string, releases []sources.Release, fetch pairing.Fetch) return identity.Fields{}, ¬e } +// JudgeDropped refuses a run in which a plugin that resolved contributes no +// entry to the catalogue and nothing stops. +// +// A plan carrying no versions and no stops is exactly that plugin. Build's three +// exclusions are an unreadable identity, a stop, and an entry with no versions, +// and the first two put a note in Stops, so what is left is a declaration that +// resolved, was classified, and left the catalogue one entry shorter than the +// run before it, with a zero exit and every skip named. +// +// Naming the skips is not enough on its own, which is why this is separate from +// the report. A server polling the address reads a plugin that has stopped being +// offered rather than a run log, and decisions/failure-posture.md puts a +// catalogue that quietly shrank on the fatal side for the same reason it puts an +// empty one there. +// +// One classification reaches this today and it is the one it was written for: a +// release list in which nothing carries a publication time has no newest +// release, so every defect in it is a skip and the plugin publishes nothing. No +// repository in the declared set is in that state, so this refuses fixtures and +// is expected to go on doing so. +func JudgeDropped(plans []posture.Plan) error { + var dropped []string + for _, p := range plans { + if len(p.Versions) > 0 || len(p.Stops) > 0 { + continue + } + dropped = append(dropped, fmt.Sprintf( + "%s: %d release(s) classified, none publishable and none stopping the run", + p.Plugin, len(p.Skips))) + } + if len(dropped) == 0 { + return nil + } + return fmt.Errorf("%d resolved plugin(s) produced no entry, and a catalogue one entry shorter is not placed: "+ + "a server reading it shows the operator a plugin that has stopped existing, and the run reports success\n %s", + len(dropped), strings.Join(dropped, "\n ")) +} + // Judge refuses a catalogue with no entries in it. // // decisions/failure-posture.md spends its longest section on this one: a run diff --git a/internal/catalogue/catalogue_test.go b/internal/catalogue/catalogue_test.go index ceb6756..8cf2996 100644 --- a/internal/catalogue/catalogue_test.go +++ b/internal/catalogue/catalogue_test.go @@ -12,6 +12,7 @@ import ( "time" "flowfin.dev/hub/internal/pairing" + "flowfin.dev/hub/internal/posture" "flowfin.dev/hub/internal/publish" "flowfin.dev/hub/internal/sources" "flowfin.dev/hub/manifest" @@ -133,6 +134,14 @@ func without(release sources.Release, suffix string) sources.Release { return release } +// undated returns the same release with the publication time taken off, which +// is what internal/sources leaves when a response carries none. A field removed +// rather than a condition invented, in the same way without() removes an asset. +func undated(release sources.Release) sources.Release { + release.Published = time.Time{} + return release +} + // routeInto is a route placing into a directory of its own, with the target's // directory already made, which publish.Place requires and does not create. func routeInto(t *testing.T, l listing, w world) (Route, string) { @@ -453,6 +462,66 @@ func TestTheLocationIsChangedWithoutTouchingWhatProducesTheBytes(t *testing.T) { // TestAnEmptyCatalogueIsRefusedRatherThanPlaced is the guard the package comment // says no run reaches today. It is judged here, where the state can be made. +// TestARunThatWouldDropAResolvedPluginIsRefusedRatherThanPlaced is the second +// half of #114, and the catalogue carries a second plugin that publishes +// perfectly so that the failure it refuses is the live one rather than the empty +// catalogue Judge already refuses. +// +// The dropped plugin resolves, both of its releases are defective, and neither +// carries a publication time, so nothing in its set is the newest and both +// defects are skips. Before this rule the run placed one entry, exited zero, and +// the address answered with a catalogue the first plugin had disappeared from. +func TestARunThatWouldDropAResolvedPluginIsRefusedRatherThanPlaced(t *testing.T) { + w := world{} + l := listing{ + "an-account/jellyfin-plugin-a-plugin": []sources.Release{ + undated(without(published(w, "a-plugin", "2.0.0-stable", 20, "2.0.0.0", "A Plugin"), ".zip.md5sum")), + undated(without(published(w, "a-plugin", "1.0.0-stable", 10, "1.0.0.0", "A Plugin"), ".zip.md5sum")), + }, + "an-account/jellyfin-plugin-another-plugin": []sources.Release{ + published(w, "another-plugin", "1.0.0-stable", 10, "1.0.0.0", "Another Plugin"), + }, + } + route, _ := routeInto(t, l, w) + + var out strings.Builder + err := route.Publish(context.Background(), &out, declaring(t, "a-plugin", "another-plugin")) + if err == nil { + t.Fatalf("a plugin was dropped from the catalogue and the run exited zero:\n%s", out.String()) + } + if !strings.Contains(err.Error(), "a-plugin") { + t.Errorf("the refusal does not name the plugin that was dropped: %v", err) + } + if _, statErr := os.Stat(route.Target.Path(route.Root)); statErr == nil { + t.Fatalf("a run that would have lost a plugin placed a file:\n%s", placedBytes(t, route)) + } + // The skips are still named, which is the half the report owes and the + // reason the refusal is a separate rule rather than a louder report. + for _, phrase := range []string{"skipped", "2.0.0-stable", "no-usable-sidecar"} { + if !strings.Contains(out.String(), phrase) { + t.Errorf("the run output does not name the skip with %q:\n%s", phrase, out.String()) + } + } +} + +// TestAPluginThatPublishesSomethingIsNotReadAsDropped is the near-miss beside +// it. JudgeDropped reads a plan that produced nothing, so a plan that produced +// one entry out of two releases has to pass it, and that is the ordinary state +// of every run today. +func TestAPluginThatPublishesSomethingIsNotReadAsDropped(t *testing.T) { + w := world{} + good := published(w, "a-plugin", "2.0.0-stable", 20, "2.0.0.0", "A Plugin") + bad := without(published(w, "a-plugin", "1.0.0-stable", 10, "1.0.0.0", "A Plugin"), ".zip.md5sum") + + plans := []posture.Plan{posture.Of("a-plugin", []sources.Release{good, bad}, w.fetch)} + if len(plans[0].Versions) != 1 || len(plans[0].Skips) != 1 { + t.Fatalf("the fixture does not produce one entry and one skip: %+v", plans[0]) + } + if err := JudgeDropped(plans); err != nil { + t.Fatalf("a plugin that published a version was read as dropped: %v", err) + } +} + func TestAnEmptyCatalogueIsRefusedRatherThanPlaced(t *testing.T) { if err := Judge(nil); err == nil { t.Fatal("a catalogue with nothing in it was accepted") diff --git a/internal/posture/posture.go b/internal/posture/posture.go index af6d9f2..7180fd3 100644 --- a/internal/posture/posture.go +++ b/internal/posture/posture.go @@ -15,6 +15,12 @@ // decisions/manifest-schema.md refuses the tag as a version string and there is // nothing else in a release list to order by. // +// Where a release list carries no publication time at all, that question has no +// answer and the classification says so rather than borrowing the tag: nothing +// in such a set is the newest, so no defect in it stops the run. What that +// leaves is a plugin that resolved and published nothing, which +// decisions/failure-posture.md makes fatal one layer up. +// // A skip is never silent. A manifest that is short because releases were skipped // and one that is short because there was nothing to add are the same file, and // only the output tells them apart. @@ -112,7 +118,16 @@ func Of(plugin string, releases []sources.Release, fetch pairing.Fetch) Plan { // The first one is the newest for this plugin and channel, which is the // release this run exists to publish. A defect in it stops the run and // the same defect below it does not. - newest := i == 0 + // + // A release carrying no publication time is never that one, however far + // up the order it landed. Where no release in the set carries a time, + // none of newerFirst's clauses fires, the order falls through to the tag + // and the highest tag arrives here at index 0 -- which is the tag + // answering the question decisions/manifest-schema.md refuses it for. + // Nothing in the set is then the newest, every defect in it is a named + // skip, and the plugin contributing no entry afterwards is refused by + // catalogue.JudgeDropped rather than passing quietly. + newest := i == 0 && !ordered[i].Published.IsZero() entry, note, err := read(plugin, release, fetch) switch { diff --git a/internal/posture/posture_test.go b/internal/posture/posture_test.go index 0fe7f22..f409ec6 100644 --- a/internal/posture/posture_test.go +++ b/internal/posture/posture_test.go @@ -75,6 +75,15 @@ func without(release sources.Release, suffix string) sources.Release { return release } +// undated returns the same release with the publication time taken off, which +// is what the layer below leaves when a response carries none. It is a field +// this fixture removes rather than a condition it invents, in the same way +// without() removes an asset. +func undated(release sources.Release) sources.Release { + release.Published = time.Time{} + return release +} + // TestAnUnusableHistoricalReleaseIsSkippedByNameAndTheRestArePublished is the // first clause of the Done-when of #28. func TestAnUnusableHistoricalReleaseIsSkippedByNameAndTheRestArePublished(t *testing.T) { @@ -196,6 +205,56 @@ func TestAReleaseWithNoPublicationTimeIsNotTakenForTheNewest(t *testing.T) { } } +// TestNoReleaseInTheSetCarriesAPublicationTime pins the answer #114 was +// decided with: where nothing in the set can be placed in time, nothing in it is +// the newest, so a defect anywhere in it is a named skip rather than a stop. +// +// Both releases are defective and both are undated, so the outcome does not turn +// on which of them the tag order happened to put first. Reading the tag as the +// order instead is what this refuses, and the tag is the string +// decisions/manifest-schema.md refuses as a version. +func TestNoReleaseInTheSetCarriesAPublicationTime(t *testing.T) { + w := world{} + higher := undated(without(published(w, "2.0.0-stable", 30, "2.0.0.0", "10.11.0.0"), ".zip.md5sum")) + lower := undated(without(published(w, "1.0.0-stable", 10, "1.0.0.0", "10.11.0.0"), ".zip.md5sum")) + + plan := Of("a-plugin", []sources.Release{higher, lower}, w.fetch) + + if len(plan.Stops) != 0 { + t.Fatalf("a release nothing could place in time stopped the run: %v", plan.Stops) + } + if len(plan.Versions) != 0 { + t.Fatalf("a defective release was published: %+v", plan.Versions) + } + if len(plan.Skips) != 2 { + t.Fatalf("the run named %d of the 2 releases it skipped: %v", len(plan.Skips), plan.Skips) + } + for _, skip := range plan.Skips { + if skip.Condition != "no-usable-sidecar" { + t.Errorf("%s carries condition %q", skip.Release, skip.Condition) + } + } + if err := Judge([]Plan{plan}); err != nil { + t.Errorf("a set of named skips stopped the run: %v", err) + } +} + +// TestOneDatedReleaseInTheSetIsStillTheNewest is the near-miss beside the test +// above. The rule is about a set nothing can order, not about the undated +// release, so one dated release is enough to make the question answerable again +// and a defect in it is fatal as before. +func TestOneDatedReleaseInTheSetIsStillTheNewest(t *testing.T) { + w := world{} + dated := without(published(w, "1.0.0-stable", 10, "1.0.0.0", "10.11.0.0"), ".zip.md5sum") + higherTag := undated(published(w, "2.0.0-stable", 30, "2.0.0.0", "10.11.0.0")) + + plan := Of("a-plugin", []sources.Release{higherTag, dated}, w.fetch) + + if len(plan.Stops) != 1 || plan.Stops[0].Release != "1.0.0-stable" { + t.Fatalf("the one release that could be placed in time was not the newest: %v %v", plan.Stops, plan.Skips) + } +} + func TestAReadThatDidNotHappenStopsTheRunWhereverItWas(t *testing.T) { // The case decisions/failure-posture.md says most needs to be fatal, because // its symptom is a short list and a short list looks exactly like success. It