diff --git a/docs/changes/unreleased/2468-collections-factory-job-feature.md b/docs/changes/unreleased/2468-collections-factory-job-feature.md new file mode 100644 index 0000000000..2be520b63e --- /dev/null +++ b/docs/changes/unreleased/2468-collections-factory-job-feature.md @@ -0,0 +1,11 @@ +--- +kind: added +title: Engines whose collections hold Factory jobs announce collections.factory_job +pr: 2468 +surface: [app] +invalidates: [] +--- + +A client requires `collections.factory_job` in `hello.features` before it relies +on adding a `factory_job` ref to a collection. An engine that announces only +`collections` may refuse that kind with `bad_params`. diff --git a/docs/design/app-v3/PROTOCOL.md b/docs/design/app-v3/PROTOCOL.md index 2b746cbd82..7e9c5637b4 100644 --- a/docs/design/app-v3/PROTOCOL.md +++ b/docs/design/app-v3/PROTOCOL.md @@ -296,6 +296,10 @@ keeps the value in a private file the agent's commands read (the model's own Collection refs include `factory_job`, with a canonical positive decimal job `id` and a required `factory` field. The pair addresses the same job as `factory.job`. Other ref kinds have no `factory`. Several collections can reference the same job. +Engines whose collections hold `factory_job` refs announce +`collections.factory_job` in `hello.features`. A client requires it before it +relies on adding a job: an engine that announces only `collections` may refuse +the kind with `bad_params`. Factory job targets carry the recorded title, state word and repository; an unavailable Factory produces an `unknown` target and status. A missing job produces a `deleted` target. Resolution uses one bounded batch across nested diff --git a/internal/appwire/collections_factory_job.go b/internal/appwire/collections_factory_job.go new file mode 100644 index 0000000000..c71b0e062f --- /dev/null +++ b/internal/appwire/collections_factory_job.go @@ -0,0 +1,17 @@ +package appwire + +// FeatureCollectionsFactoryJob names an engine whose collections hold +// factory_job references: a Factory job by its factory and job number. A +// client requires it before it relies on adding a job to a collection: an +// engine that announces only "collections" may refuse the kind with bad_params. +const FeatureCollectionsFactoryJob = "collections.factory_job" + +// withCollectionsFactoryJobFeature is hello carrying collections.factory_job +// when the engine serves collections: this wire checks the factory_job kind +// and passes it to every collector. +func withCollectionsFactoryJobFeature(hello Hello, e Engine) Hello { + if _, ok := e.(Collector); !ok { + return hello + } + return withFeature(hello, FeatureCollectionsFactoryJob) +} diff --git a/internal/appwire/collections_factory_job_feature_test.go b/internal/appwire/collections_factory_job_feature_test.go new file mode 100644 index 0000000000..24a6ee2bf2 --- /dev/null +++ b/internal/appwire/collections_factory_job_feature_test.go @@ -0,0 +1,38 @@ +package appwire + +import ( + "slices" + "testing" +) + +// An engine announces collections.factory_job only when it serves +// collections, so a client can tell before it adds a job. +func TestCollectionsFactoryJobFeatureFollowsCollections(t *testing.T) { + if h := withCollectionsFactoryJobFeature(Hello{}, &fakeEngine{}); len(h.Features) != 0 { + t.Fatalf("an engine without collections announces %v", h.Features) + } + h := withCollectionsFactoryJobFeature(Hello{}, &fakeCollector{fakeEngine: &fakeEngine{}}) + if len(h.Features) != 1 || h.Features[0] != FeatureCollectionsFactoryJob { + t.Fatalf("a collector announces %v", h.Features) + } +} + +// The announced feature is served: a factory_job ref with its factory reaches +// the collector, and one without a factory is refused before it. +func TestCollectionsFactoryJobFeatureIsServed(t *testing.T) { + w, engine := startCollectorWire(t) + if !slices.Contains(features(w.next()), FeatureCollectionsFactoryJob) { + t.Fatal("hello lacks collections.factory_job") + } + w.send(`{"id":1,"method":"collections.add","params":{"id":"c1","refs":[{"kind":"factory_job","id":"7","factory":"f"}]}}`) + if got := w.next(); got["error"] != nil { + t.Fatalf("factory_job add = %v", got) + } + if len(engine.added) != 1 || engine.added[0].Kind != RefFactoryJob || engine.added[0].ID != "7" || engine.added[0].Factory != "f" { + t.Fatalf("collector got %+v", engine.added) + } + w.send(`{"id":2,"method":"collections.add","params":{"id":"c1","refs":[{"kind":"factory_job","id":"7"}]}}`) + if got := w.next(); got["error"] == nil || got["error"].(map[string]any)["code"] != CodeBadParams { + t.Fatalf("factory_job without a factory = %v", got) + } +} diff --git a/internal/appwire/collections_test.go b/internal/appwire/collections_test.go index 782a02cdae..7b06bfac33 100644 --- a/internal/appwire/collections_test.go +++ b/internal/appwire/collections_test.go @@ -106,12 +106,12 @@ func features(hello map[string]any) []any { // engine without collections answers its methods unknown_method. func TestCollectionsFeatureFollowsTheEngine(t *testing.T) { w, _ := startCollectorWire(t) - if got := features(w.next()); len(got) != 3 || got[0] != "x" || got[1] != FeatureAgentEffortRungs || got[2] != FeatureCollections { + if got := features(w.next()); len(got) != 4 || got[0] != "x" || got[1] != FeatureAgentEffortRungs || got[2] != FeatureCollections || got[3] != FeatureCollectionsFactoryJob { t.Fatalf("features = %v", got) } plain := startWire(t, &fakeEngine{}) for _, f := range features(plain.next()) { - if f == FeatureCollections { + if f == FeatureCollections || f == FeatureCollectionsFactoryJob { t.Fatal("an engine without collections claims them") } } diff --git a/internal/appwire/server.go b/internal/appwire/server.go index 0e20f284df..5f4cfe85ed 100644 --- a/internal/appwire/server.go +++ b/internal/appwire/server.go @@ -289,6 +289,7 @@ func (s *Server) Serve(ctx context.Context, in io.Reader, hello Hello, engine En hello = withFactoryFeaturesFeature(hello, engine) hello = withFactoryRepoIssuesFeature(hello, engine) hello = withFactoryGroupPolicyFeature(hello, engine) + hello = withCollectionsFactoryJobFeature(hello, engine) s.Emit(EventHello, hello) if warmer, ok := engine.(Warmer); ok { go warmer.Warm()