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