From 04642445c34cee023b41ce173de34473052756bf Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:33:32 -0400 Subject: [PATCH 01/14] feat(channels): add OriginLinkProvider optional kind interface --- pkg/channels/channelkinds/kind.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/channels/channelkinds/kind.go b/pkg/channels/channelkinds/kind.go index f82b7fb3..a40cf84e 100644 --- a/pkg/channels/channelkinds/kind.go +++ b/pkg/channels/channelkinds/kind.go @@ -729,6 +729,18 @@ type SessionOwnerProvider interface { SessionOwner(ev InboundEvent) (subject string, provides bool) } +// OriginLinkProvider is an OPTIONAL Kind capability — discovered by +// type-assertion, never a method on Kind — that turns one of a session's +// channel bindings into a human-facing URL back to where the conversation +// lives: a Slack thread, a GitHub pull request. A kind with no such surface +// does not implement it and contributes no link. ok=false means "this binding +// has no resolvable origin yet" (a Slack thread not rooted, a permalink that +// failed to resolve) and the caller drops it silently. Consumed by webd's +// artifact-view page to offer a "back to origin" link per distinct origin. +type OriginLinkProvider interface { + OriginLink(binding *spiceboxv1alpha1.ChannelBinding) (url string, ok bool) +} + // OwnerGroupProvider is an optional Kind capability: return the channel's live // membership subject-set ref (e.g. "slack_channel:#member") for use as an // owner source when this kind is the output channel, and whether one exists. From 5e8240cfb80d88db9413a5010082f14c50ee269d Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:35:38 -0400 Subject: [PATCH 02/14] feat(github): OriginLink from persisted html_url or pr-key reconstruction --- pkg/channels/channelkinds/github/kind.go | 23 ++++++++++ .../channelkinds/github/origin_link_test.go | 44 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 pkg/channels/channelkinds/github/origin_link_test.go diff --git a/pkg/channels/channelkinds/github/kind.go b/pkg/channels/channelkinds/github/kind.go index 047f7215..31d847ba 100644 --- a/pkg/channels/channelkinds/github/kind.go +++ b/pkg/channels/channelkinds/github/kind.go @@ -159,6 +159,29 @@ func (Kind) FeatureSupport() map[channelfeatures.Feature]channelkinds.FeatureReq // output, composed by the tool that posts it, not by this kind). func (Kind) RenderMention(externalID string) string { return externalID } +var _ channelkinds.OriginLinkProvider = Kind{} + +// OriginLink returns a human-facing URL to the pull request this binding +// correlates to, satisfying channelkinds.OriginLinkProvider. It prefers the +// host-correct html_url persisted at session creation +// (External["origin_url"] — correct on a GitHub Enterprise Server host), and +// falls back to reconstructing a github.com URL from the "pr:/#" +// Key for sessions created before that value was persisted. ok=false when +// neither is available. +func (Kind) OriginLink(binding *spiceboxv1alpha1.ChannelBinding) (string, bool) { + if binding == nil { + return "", false + } + if u := binding.External["origin_url"]; u != "" { + return u, true + } + owner, repo, number, err := parseChannelKey(binding.Key) + if err != nil { + return "", false + } + return fmt.Sprintf("https://github.com/%s/%s/pull/%d", owner, repo, number), true +} + // SupportedMentionLookups returns nil — a pull request's participants have no // AP directory to look up against. func (Kind) SupportedMentionLookups() []channelkinds.MentionLookupKind { return nil } diff --git a/pkg/channels/channelkinds/github/origin_link_test.go b/pkg/channels/channelkinds/github/origin_link_test.go new file mode 100644 index 00000000..e4999335 --- /dev/null +++ b/pkg/channels/channelkinds/github/origin_link_test.go @@ -0,0 +1,44 @@ +package github + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" +) + +func TestKind_OriginLink(t *testing.T) { + k := Kind{} + cases := []struct { + name string + binding *spiceboxv1alpha1.ChannelBinding + wantURL string + wantOK bool + }{ + { + name: "persisted html_url wins (GHES host-correct)", + binding: &spiceboxv1alpha1.ChannelBinding{Kind: "github", Key: "pr:demo-org/platform#42", + External: map[string]string{"origin_url": "https://ghe.example.com/demo-org/platform/pull/42"}}, + wantURL: "https://ghe.example.com/demo-org/platform/pull/42", wantOK: true, + }, + { + name: "no persisted url: reconstruct github.com from pr key", + binding: &spiceboxv1alpha1.ChannelBinding{Kind: "github", Key: "pr:demo-org/platform#42"}, + wantURL: "https://github.com/demo-org/platform/pull/42", wantOK: true, + }, + { + name: "unparseable key, no url: ok=false", + binding: &spiceboxv1alpha1.ChannelBinding{Kind: "github", Key: "garbage"}, + wantOK: false, + }, + {name: "nil binding: ok=false", binding: nil, wantOK: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, ok := k.OriginLink(tc.binding) + assert.Equal(t, tc.wantOK, ok) + assert.Equal(t, tc.wantURL, got) + }) + } +} From 79eb7a2dfc9a5ad1c26abfac52e4ab8e748d0391 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:36:36 -0400 Subject: [PATCH 03/14] feat(slack): OriginLink from persisted thread permalink --- pkg/channels/channelkinds/slack/kind.go | 17 +++++++++++ .../channelkinds/slack/origin_link_test.go | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 pkg/channels/channelkinds/slack/origin_link_test.go diff --git a/pkg/channels/channelkinds/slack/kind.go b/pkg/channels/channelkinds/slack/kind.go index 290a647d..91d83f5e 100644 --- a/pkg/channels/channelkinds/slack/kind.go +++ b/pkg/channels/channelkinds/slack/kind.go @@ -425,6 +425,23 @@ func (*Kind) RenderMention(externalID string) string { return "<@" + externalID + ">" } +var _ channelkinds.OriginLinkProvider = (*Kind)(nil) + +// OriginLink returns the canonical Slack thread permalink persisted onto the +// binding at first thread-root (External["permalink"], resolved by the sender +// via chat.getPermalink), satisfying channelkinds.OriginLinkProvider. ok=false +// when it is absent — the thread is not rooted yet, or permalink resolution +// failed — and the caller drops the link. +func (*Kind) OriginLink(binding *spiceboxv1alpha1.ChannelBinding) (string, bool) { + if binding == nil { + return "", false + } + if u := binding.External["permalink"]; u != "" { + return u, true + } + return "", false +} + // SupportedMentionLookups returns the set of identifier shapes this // Slack kind can resolve. v1 supports all three (email, name, any). // The implementations are in pkg/channels/channelkinds/slack/lookup.go. diff --git a/pkg/channels/channelkinds/slack/origin_link_test.go b/pkg/channels/channelkinds/slack/origin_link_test.go new file mode 100644 index 00000000..1884cb1a --- /dev/null +++ b/pkg/channels/channelkinds/slack/origin_link_test.go @@ -0,0 +1,28 @@ +package slack + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" +) + +func TestKind_OriginLink(t *testing.T) { + k := &Kind{} + t.Run("persisted permalink present", func(t *testing.T) { + u, ok := k.OriginLink(&spiceboxv1alpha1.ChannelBinding{Kind: "slack", + External: map[string]string{"permalink": "https://demo.slack.com/archives/C1/p1700000000000100"}}) + assert.True(t, ok) + assert.Equal(t, "https://demo.slack.com/archives/C1/p1700000000000100", u) + }) + t.Run("no permalink yet: ok=false", func(t *testing.T) { + _, ok := k.OriginLink(&spiceboxv1alpha1.ChannelBinding{Kind: "slack", + External: map[string]string{"channel_id": "C1", "thread_ts": "1700000000.000100"}}) + assert.False(t, ok) + }) + t.Run("nil binding: ok=false", func(t *testing.T) { + _, ok := k.OriginLink(nil) + assert.False(t, ok) + }) +} From f625febc956224ca1c26e39ecdb7021cef826fe4 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:40:11 -0400 Subject: [PATCH 04/14] feat(slack): resolve + return canonical thread permalink on first send --- pkg/channels/channelkinds/slack/sender.go | 25 +++++++-- .../slack/sender_permalink_test.go | 53 +++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 pkg/channels/channelkinds/slack/sender_permalink_test.go diff --git a/pkg/channels/channelkinds/slack/sender.go b/pkg/channels/channelkinds/slack/sender.go index 0d2bc87c..ad7a3cd6 100644 --- a/pkg/channels/channelkinds/slack/sender.go +++ b/pkg/channels/channelkinds/slack/sender.go @@ -371,7 +371,7 @@ func (s *slackSender) Send(ctx context.Context, sess channelkinds.SessionInfo, e } if rootTS != "" { return channelkinds.SubChannelSendResult{ - External: map[string]string{"thread_ts": rootTS, "channel_id": channelID}, + External: s.threadOriginExternal(ctx, logger, channelID, rootTS), }, nil } } @@ -460,15 +460,30 @@ func (s *slackSender) Send(ctx context.Context, sess channelkinds.SessionInfo, e rootTS = postedTS // plain cron first-send: the reply itself is the root } return channelkinds.SubChannelSendResult{ - External: map[string]string{ - "thread_ts": rootTS, - "channel_id": ch, - }, + External: s.threadOriginExternal(ctx, logger, ch, rootTS), }, nil } return channelkinds.SubChannelSendResult{}, nil } +// threadOriginExternal builds the SubChannelSendResult.External a first-send +// thread-root hands back to channelsd's outbound relay: the thread_ts/channel_id +// the relay keys the binding on, plus a best-effort canonical permalink +// (chat.getPermalink) that webd's artifact-view page renders as a "back to +// origin" link. The permalink is resolved here because only the sender holds a +// Slack client. A getPermalink failure is logged and the permalink omitted — it +// must never block the send, exactly as the former click-time lookup did. +func (s *slackSender) threadOriginExternal(ctx context.Context, logger interface{ Info(string, ...any) }, channelID, rootTS string) map[string]string { + ext := map[string]string{"thread_ts": rootTS, "channel_id": channelID} + if pl, err := s.client.GetPermalinkContext(ctx, &slackapi.PermalinkParameters{Channel: channelID, Ts: rootTS}); err != nil { + logger.Info("slack first-send: getPermalink failed; thread origin link omitted", + "channel", channelID, "thread", rootTS, "err", err.Error()) + } else if pl != "" { + ext["permalink"] = pl + } + return ext +} + // buildUserMessageBlocks composes the Block Kit block set for the agent's // final user_message chat.postMessage. Layout: // diff --git a/pkg/channels/channelkinds/slack/sender_permalink_test.go b/pkg/channels/channelkinds/slack/sender_permalink_test.go new file mode 100644 index 00000000..ca468604 --- /dev/null +++ b/pkg/channels/channelkinds/slack/sender_permalink_test.go @@ -0,0 +1,53 @@ +package slack + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" + "github.com/authzed/openagentprimitives/pkg/channels/channelkinds" +) + +func firstSendSession() channelkinds.SessionInfo { + return channelkinds.SessionInfo{ + Namespace: "default", Name: "cron-session", + Channel: &spiceboxv1alpha1.ChannelBinding{ + Name: "slack-eng", Kind: "slack", + // thread_ts intentionally absent: the cron/bot first-send case that + // roots the thread. + External: map[string]string{"channel_id": "C01ABCDEF"}, + }, + } +} + +// TestSender_FirstSend_CapturesPermalink: on the send that roots the thread, +// the sender resolves the canonical permalink and returns it in +// SubChannelSendResult.External so the relay can persist it for the +// artifact-view origin link. +func TestSender_FirstSend_CapturesPermalink(t *testing.T) { + c := &fakeSlackClient{postedTS: "1234.5678", getPermalinkResult: "https://slack.test/archives/C01ABCDEF/p12345678"} + s := newSender(c) + res, err := s.Send(context.Background(), firstSendSession(), userMessageEnvelope(t, "first cron post")) + require.NoError(t, err, "Send") + assert.Equal(t, "1234.5678", res.External["thread_ts"], "thread root still captured") + assert.Equal(t, "https://slack.test/archives/C01ABCDEF/p12345678", res.External["permalink"], "canonical permalink captured") + require.Len(t, c.getPermalinkCalls, 1, "getPermalink called once on first root") + assert.Equal(t, "C01ABCDEF", c.getPermalinkCalls[0].Channel) + assert.Equal(t, "1234.5678", c.getPermalinkCalls[0].Ts) +} + +// TestSender_FirstSend_PermalinkFailureOmitsLink: a getPermalink failure must +// never block the send; the thread still roots and permalink is simply absent. +func TestSender_FirstSend_PermalinkFailureOmitsLink(t *testing.T) { + c := &fakeSlackClient{postedTS: "1234.5678", getPermalinkErr: errors.New("channels:read missing")} + s := newSender(c) + res, err := s.Send(context.Background(), firstSendSession(), userMessageEnvelope(t, "first cron post")) + require.NoError(t, err, "send must succeed despite getPermalink failure") + assert.Equal(t, "1234.5678", res.External["thread_ts"], "thread root still captured") + _, has := res.External["permalink"] + assert.False(t, has, "permalink omitted when getPermalink fails") +} From 8823d7d441f7d61692451f03c788523f52a8749e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:42:49 -0400 Subject: [PATCH 05/14] feat(relay): persist slack thread permalink alongside thread_ts --- pkg/channels/channelsd/outbound/relay.go | 30 ++++++---- .../outbound/relay_permalink_test.go | 56 +++++++++++++++++++ 2 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 pkg/channels/channelsd/outbound/relay_permalink_test.go diff --git a/pkg/channels/channelsd/outbound/relay.go b/pkg/channels/channelsd/outbound/relay.go index 6ea8808a..3c66cfc8 100644 --- a/pkg/channels/channelsd/outbound/relay.go +++ b/pkg/channels/channelsd/outbound/relay.go @@ -1065,6 +1065,15 @@ func (r *Relay) patchOutputChannel(ctx context.Context, sess *spiceboxv1alpha1.A return nil } key := fmt.Sprintf("thread:%s:%s", channelID, threadTS) + // The canonical thread permalink (chat.getPermalink), when the Sender + // resolved one on this first-root send, rides the same patch as the + // thread_ts it is derived from — webd reads it off the binding for the + // artifact-view "back to origin" link. Omitted (never written empty) when + // resolution failed, so a later root can still supply it. + extPatch := map[string]string{"channel_id": channelID, "thread_ts": threadTS} + if pl := captured["permalink"]; pl != "" { + extPatch["permalink"] = pl + } // Patch BOTH spec.outputChannel.{key,external} AND // metadata.labels[LabelOutputChannelKey] in a single MergePatch so // the index label is in sync with the spec atomically — there is no @@ -1079,11 +1088,8 @@ func (r *Relay) patchOutputChannel(ctx context.Context, sess *spiceboxv1alpha1.A }, "spec": map[string]any{ "outputChannel": map[string]any{ - "key": key, - "external": map[string]string{ - "channel_id": channelID, - "thread_ts": threadTS, - }, + "key": key, + "external": extPatch, }, }, } @@ -1112,6 +1118,9 @@ func (r *Relay) patchOutputChannel(ctx context.Context, sess *spiceboxv1alpha1.A } mergedExternal["channel_id"] = channelID mergedExternal["thread_ts"] = threadTS + if pl := captured["permalink"]; pl != "" { + mergedExternal["permalink"] = pl + } sess.Spec.OutputChannel.External = mergedExternal sess.Spec.OutputChannel.Key = key @@ -1172,6 +1181,10 @@ func (r *Relay) patchInputChannelThreadTS(ctx context.Context, sess *spiceboxv1a return nil } key := fmt.Sprintf("thread:%s:%s", channelID, threadTS) + extPatch := map[string]string{"channel_id": channelID, "thread_ts": threadTS} + if pl := captured["permalink"]; pl != "" { + extPatch["permalink"] = pl + } // Patch spec.inputChannel.external AND LabelChannelKey atomically so // the inbound pipeline's label lookup sees the new thread root without // a window where the key is set but the label isn't (or vice versa). @@ -1183,11 +1196,8 @@ func (r *Relay) patchInputChannelThreadTS(ctx context.Context, sess *spiceboxv1a }, "spec": map[string]any{ "inputChannel": map[string]any{ - "key": key, - "external": map[string]string{ - "channel_id": channelID, - "thread_ts": threadTS, - }, + "key": key, + "external": extPatch, }, }, } diff --git a/pkg/channels/channelsd/outbound/relay_permalink_test.go b/pkg/channels/channelsd/outbound/relay_permalink_test.go new file mode 100644 index 00000000..5c7a0f4a --- /dev/null +++ b/pkg/channels/channelsd/outbound/relay_permalink_test.go @@ -0,0 +1,56 @@ +package outbound + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client" + + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" +) + +const testPermalink = "https://slack.test/archives/C01ABCDEF/p12345678" + +// TestRelay_PatchOutputChannel_PersistsPermalink: the canonical thread permalink +// the sender captured on first root is written onto spec.outputChannel.external +// alongside thread_ts, both persisted and reflected in memory, so webd can read +// it off the session for the artifact-view origin link. +func TestRelay_PatchOutputChannel_PersistsPermalink(t *testing.T) { + nc := connectNATS(t) + sess := cronSession("cron-session", map[string]string{"channel_id": "C01ABCDEF"}) + cli := fakeClientWith(t, sess) + r := &Relay{NC: nc, K8s: cli} + + err := r.patchOutputChannel(context.Background(), sess, map[string]string{ + "thread_ts": "1234.5678", "channel_id": "C01ABCDEF", "permalink": testPermalink, + }) + require.NoError(t, err, "patchOutputChannel") + + assert.Equal(t, testPermalink, sess.Spec.OutputChannel.External["permalink"], "in-memory reflection carries permalink") + + var patched spiceboxv1alpha1.AgentSession + require.NoError(t, cli.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "cron-session"}, &patched)) + assert.Equal(t, testPermalink, patched.Spec.OutputChannel.External["permalink"], "persisted permalink") + assert.Equal(t, "1234.5678", patched.Spec.OutputChannel.External["thread_ts"], "thread_ts still persisted") +} + +// TestRelay_PatchOutputChannel_NoPermalink_WritesNone: when the sender captured +// no permalink (getPermalink failed), no permalink key is written. +func TestRelay_PatchOutputChannel_NoPermalink_WritesNone(t *testing.T) { + nc := connectNATS(t) + sess := cronSession("cron-session", map[string]string{"channel_id": "C01ABCDEF"}) + cli := fakeClientWith(t, sess) + r := &Relay{NC: nc, K8s: cli} + + err := r.patchOutputChannel(context.Background(), sess, map[string]string{ + "thread_ts": "1234.5678", "channel_id": "C01ABCDEF", + }) + require.NoError(t, err, "patchOutputChannel") + + var patched spiceboxv1alpha1.AgentSession + require.NoError(t, cli.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "cron-session"}, &patched)) + _, has := patched.Spec.OutputChannel.External["permalink"] + assert.False(t, has, "no permalink key when none captured") +} From 2dcfede2a9b8ca33f69054a0a7c033fc387ba4bb Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:46:38 -0400 Subject: [PATCH 06/14] feat(github): derive PR origin_url onto the session input binding --- internal/cmd/channelsd/webhook_inbound.go | 19 ++++- .../channelsd/webhook_inbound_origin_test.go | 77 +++++++++++++++++++ pkg/channels/channelkinds/github/receiver.go | 24 ++++++ .../github/receiver_origin_test.go | 45 +++++++++++ pkg/channels/channelkinds/webhook.go | 13 ++++ 5 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 internal/cmd/channelsd/webhook_inbound_origin_test.go create mode 100644 pkg/channels/channelkinds/github/receiver_origin_test.go diff --git a/internal/cmd/channelsd/webhook_inbound.go b/internal/cmd/channelsd/webhook_inbound.go index 2eea8b40..aaf4e2d4 100644 --- a/internal/cmd/channelsd/webhook_inbound.go +++ b/internal/cmd/channelsd/webhook_inbound.go @@ -77,8 +77,14 @@ func webhookInboundHandler(ctx context.Context, cli client.Client, pl channelkin // the owner grant is an addition to the session, never a gate on it, so a // derivation failure logs and the delivery dispatches without one. var preTurn map[string]string + // external carries kind-specific routing/origin metadata onto the session's + // input binding (spec.inputChannel.external — the pipeline copies it). Today + // github's TriggerOriginURLProvider populates origin_url (the PR html_url) so + // webd's artifact-view page can link back to the pull request. + var external map[string]string if k, ok := chregistry.Get(ch.Spec.Kind); ok { - if op, ok := k.WebhookReceiver(channelkinds.Deps{}).(channelkinds.TriggerOwnerProvider); ok { + recv := k.WebhookReceiver(channelkinds.Deps{}) + if op, ok := recv.(channelkinds.TriggerOwnerProvider); ok { subject, has, oerr := op.TriggerOwnerSubject(&ch, p.DeliveryEvent, p.RawDelivery) switch { case oerr != nil: @@ -88,6 +94,16 @@ func webhookInboundHandler(ctx context.Context, cli client.Client, pl channelkin preTurn = map[string]string{spiceboxv1alpha1.AnnotationTriggerOwnerSubject: subject} } } + if op, ok := recv.(channelkinds.TriggerOriginURLProvider); ok { + u, has, oerr := op.TriggerOriginURL(&ch, p.DeliveryEvent, p.RawDelivery) + switch { + case oerr != nil: + logger.Info("webhook inbound: origin url derivation failed; session input binding will have no origin link", + "ns", p.ChannelNamespace, "name", p.ChannelName, "key", p.ChannelKey, "err", oerr.Error()) + case has: + external = map[string]string{"origin_url": u} + } + } } dec, err := pl.Deliver(ctx, channelkinds.InboundEvent{ Channel: &ch, @@ -97,6 +113,7 @@ func webhookInboundHandler(ctx context.Context, cli client.Client, pl channelkin RawDelivery: p.RawDelivery, DeliveryEvent: p.DeliveryEvent, PreTurnAnnotations: preTurn, + External: external, }) if err != nil { logger.Info("webhook inbound: pipeline.Deliver errored", diff --git a/internal/cmd/channelsd/webhook_inbound_origin_test.go b/internal/cmd/channelsd/webhook_inbound_origin_test.go new file mode 100644 index 00000000..ca18ff45 --- /dev/null +++ b/internal/cmd/channelsd/webhook_inbound_origin_test.go @@ -0,0 +1,77 @@ +package main + +import ( + "context" + "encoding/json" + "testing" + + "github.com/nats-io/nats.go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/authzed/openagentprimitives/pkg/channels/channelkinds" +) + +// TestWebhookInboundHandler_DerivesOriginURL pins the handoff that lets the +// artifact-view page link back to the PR: the github receiver's +// TriggerOriginURLProvider derives the PR html_url from the verified delivery, +// and the handler places it on InboundEvent.External["origin_url"], which the +// pipeline copies onto the session's input binding. +func TestWebhookInboundHandler_DerivesOriginURL(t *testing.T) { + cli := fake.NewClientBuilder().WithScheme(attachScheme(t)).WithObjects(webhookTestChannel()).Build() + pipe := &fakeWebhookPipeline{dec: channelkinds.InboundDecision{Outcome: channelkinds.OutcomeRouted}} + logger, _ := captureLogger() + payload := webhookTestPayload() + payload.DeliveryEvent = "pull_request" + payload.RawDelivery = []byte(`{"action":"opened","number":42,` + + `"pull_request":{"html_url":"https://github.com/demo-org/platform/pull/42",` + + `"user":{"login":"demo-user","id":4172237}},` + + `"repository":{"full_name":"demo-org/platform"}}`) + raw, err := json.Marshal(payload) + require.NoError(t, err) + + webhookInboundHandler(context.Background(), cli, pipe, logger, &nats.Msg{Data: raw}) + + require.True(t, pipe.Called()) + assert.Equal(t, "https://github.com/demo-org/platform/pull/42", + pipe.observed().External["origin_url"], + "the PR html_url, derived from the verified delivery, must reach the session input binding") +} + +// A delivery the provider cannot derive an origin URL from still dispatches — +// the origin link is additive, never a gate — and an undecodable delivery is +// logged, not silent. +func TestWebhookInboundHandler_OriginURLDerivationFailure_StillDispatches(t *testing.T) { + cases := []struct { + name string + event string + rawDelivery []byte + wantLog string + }{ + {name: "non-pull_request event: no origin_url, no log", event: "ping", + rawDelivery: []byte(`{"zen":"x"}`)}, + {name: "undecodable delivery: no origin_url, logged", event: "pull_request", + rawDelivery: []byte("not json"), wantLog: "origin url derivation failed"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cli := fake.NewClientBuilder().WithScheme(attachScheme(t)).WithObjects(webhookTestChannel()).Build() + pipe := &fakeWebhookPipeline{dec: channelkinds.InboundDecision{Outcome: channelkinds.OutcomeRouted}} + logger, read := captureLogger() + payload := webhookTestPayload() + payload.DeliveryEvent = tc.event + payload.RawDelivery = tc.rawDelivery + raw, err := json.Marshal(payload) + require.NoError(t, err) + + webhookInboundHandler(context.Background(), cli, pipe, logger, &nats.Msg{Data: raw}) + + require.True(t, pipe.Called(), "the session must still start; the origin link is additive, not a gate") + assert.NotContains(t, pipe.observed().External, "origin_url") + if tc.wantLog != "" { + assert.Contains(t, read(), tc.wantLog) + } + }) + } +} diff --git a/pkg/channels/channelkinds/github/receiver.go b/pkg/channels/channelkinds/github/receiver.go index 3e322696..8019f861 100644 --- a/pkg/channels/channelkinds/github/receiver.go +++ b/pkg/channels/channelkinds/github/receiver.go @@ -247,6 +247,30 @@ func (receiver) TriggerOwnerSubject(ch *spiceboxv1alpha1.Channel, event string, var _ channelkinds.TriggerOwnerProvider = receiver{} +// TriggerOriginURL returns the pull request's own html_url from a verified +// pull_request delivery, satisfying channelkinds.TriggerOriginURLProvider. The +// URL is host-correct — it is whatever host the delivery came from, so a GitHub +// Enterprise Server install yields its own host rather than github.com. ok=false +// (not an error) for a non-pull_request event or a payload that carries no +// html_url; the channelsd inbound handler then starts the session without the +// origin link. Re-decodes the same narrow prEvent, preserving its "a payload +// change upstream cannot alter behavior without a code change" property. +func (receiver) TriggerOriginURL(_ *spiceboxv1alpha1.Channel, event string, body []byte) (string, bool, error) { + if event != "pull_request" { + return "", false, nil + } + var ev prEvent + if err := json.Unmarshal(body, &ev); err != nil { + return "", false, fmt.Errorf("github: decode pull_request payload for origin url: %w", err) + } + if ev.PullRequest.HTMLURL == "" { + return "", false, nil + } + return ev.PullRequest.HTMLURL, true, nil +} + +var _ channelkinds.TriggerOriginURLProvider = receiver{} + // TriggerSlotInstances names this pull request as the one instance a class's // slots may bind at session mint. // diff --git a/pkg/channels/channelkinds/github/receiver_origin_test.go b/pkg/channels/channelkinds/github/receiver_origin_test.go new file mode 100644 index 00000000..63766af0 --- /dev/null +++ b/pkg/channels/channelkinds/github/receiver_origin_test.go @@ -0,0 +1,45 @@ +package github + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestTriggerOriginURL_ReturnsPRHTMLURL pins that a verified pull_request +// delivery yields the PR's own html_url (host-correct — it is whatever host the +// delivery came from, so GitHub Enterprise is covered). Driven from the real +// payload fixture, same as the owner/facts tests, so a wrong json tag can't pass. +func TestTriggerOriginURL_ReturnsPRHTMLURL(t *testing.T) { + u, ok, err := receiver{}.TriggerOriginURL(testChannel(t), "pull_request", fixtureBody(t, "pull_request_opened.json")) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, "https://github.com/demo-org/platform/pull/42", u) +} + +func TestTriggerOriginURL_NonPROrMissing(t *testing.T) { + cases := []struct { + name string + event string + body string + }{ + {name: "non-pull_request event: ok=false", event: "ping", body: `{"zen":"x"}`}, + {name: "pull_request without html_url: ok=false", event: "pull_request", + body: `{"action":"opened","number":7,"pull_request":{},"repository":{"full_name":"demo-org/platform"}}`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + u, ok, err := receiver{}.TriggerOriginURL(testChannel(t), tc.event, []byte(tc.body)) + require.NoError(t, err) + assert.False(t, ok) + assert.Empty(t, u) + }) + } +} + +func TestTriggerOriginURL_UndecodableBodyErrors(t *testing.T) { + _, ok, err := receiver{}.TriggerOriginURL(testChannel(t), "pull_request", []byte("not json")) + require.Error(t, err) + assert.False(t, ok) +} diff --git a/pkg/channels/channelkinds/webhook.go b/pkg/channels/channelkinds/webhook.go index 1bc13f90..e38e811a 100644 --- a/pkg/channels/channelkinds/webhook.go +++ b/pkg/channels/channelkinds/webhook.go @@ -195,6 +195,19 @@ type TriggerOwnerProvider interface { TriggerOwnerSubject(ch *spiceboxv1alpha1.Channel, event string, body []byte) (subject string, ok bool, err error) } +// TriggerOriginURLProvider is an OPTIONAL WebhookReceiver capability — the same +// type-assertion shape as TriggerOwnerProvider — that derives a human-facing +// origin URL from a VERIFIED delivery (a pull request's html_url), so the +// session's input binding can record where the trigger came from and webd's +// artifact-view page can link back to it. The URL is a provider-supplied +// STRUCTURAL field (GitHub's own html_url), host-correct for GitHub Enterprise +// where reconstructing a github.com URL would be wrong. Best-effort: ok=false +// when the delivery names no such URL, and the channelsd inbound handler starts +// the session without the link rather than failing. +type TriggerOriginURLProvider interface { + TriggerOriginURL(ch *spiceboxv1alpha1.Channel, event string, body []byte) (url string, ok bool, err error) +} + // TriggerSlotInstance is one resource id a verified webhook delivery names, // paired with the resource type a class slot's fillFrom must match it // against. A slot binds it only when the slot's own resourceType agrees — From dea153df2feef07af62e4e0617d1a006d8b1cb37 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:50:49 -0400 Subject: [PATCH 07/14] feat(webd): derive artifact-view origin links from the session bindings --- internal/cmd/webd/origin_links.go | 59 ++++++++++++ internal/cmd/webd/origin_links_test.go | 90 +++++++++++++++++++ .../webui/artifactview/artifactview_test.go | 7 ++ pkg/web/webui/artifactview/deps.go | 17 ++++ pkg/web/webui/artifactview/live_test.go | 3 + 5 files changed, 176 insertions(+) create mode 100644 internal/cmd/webd/origin_links.go create mode 100644 internal/cmd/webd/origin_links_test.go diff --git a/internal/cmd/webd/origin_links.go b/internal/cmd/webd/origin_links.go new file mode 100644 index 00000000..cb631d9d --- /dev/null +++ b/internal/cmd/webd/origin_links.go @@ -0,0 +1,59 @@ +package main + +import ( + "context" + + "sigs.k8s.io/controller-runtime/pkg/client" + + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" + "github.com/authzed/openagentprimitives/pkg/channels/channelkinds" + channelregistry "github.com/authzed/openagentprimitives/pkg/channels/channelkinds/registry" + "github.com/authzed/openagentprimitives/pkg/web/webui/artifactview" +) + +// originLinksFrom walks the session's input then output channel bindings, +// dispatches each through its kind's OriginLinkProvider (the registry, never an +// `if kind == …` switch), and returns the resolved links de-duplicated by Href +// in input-first order. A kind implementing no OriginLinkProvider, or a binding +// with no resolvable origin yet (a Slack thread not rooted, a permalink that +// failed to resolve), contributes nothing. Pure function of the session so it is +// unit-testable without a cluster. +func originLinksFrom(sess *spiceboxv1alpha1.AgentSession) []artifactview.OriginLink { + var out []artifactview.OriginLink + seen := map[string]bool{} + add := func(b *spiceboxv1alpha1.ChannelBinding) { + if b == nil { + return + } + k, ok := channelregistry.Get(b.Kind) + if !ok { + return + } + p, ok := k.(channelkinds.OriginLinkProvider) + if !ok { + return + } + u, ok := p.OriginLink(b) + if !ok || u == "" || seen[u] { + return + } + seen[u] = true + out = append(out, artifactview.OriginLink{Href: u, ChannelKind: b.Kind}) + } + add(sess.Spec.InputChannel) + add(sess.Spec.OutputChannel) + return out +} + +// OriginLinks implements artifactview.Deps: it loads the session and derives its +// origin links. A Get failure logs and returns no links — never an error, the +// toolbar simply shows nothing — matching ChannelKind's best-effort contract. +func (d *artifactViewDeps) OriginLinks(ctx context.Context, ns, sess string) []artifactview.OriginLink { + var cur spiceboxv1alpha1.AgentSession + if err := d.K8s().Get(ctx, client.ObjectKey{Namespace: ns, Name: sess}, &cur); err != nil { + d.logger.Error(err, "artifactview: OriginLinks session lookup failed; no origin links", + "ns", ns, "sess", sess) + return nil + } + return originLinksFrom(&cur) +} diff --git a/internal/cmd/webd/origin_links_test.go b/internal/cmd/webd/origin_links_test.go new file mode 100644 index 00000000..5a21ee0f --- /dev/null +++ b/internal/cmd/webd/origin_links_test.go @@ -0,0 +1,90 @@ +package main + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" + "github.com/authzed/openagentprimitives/pkg/web/webui/artifactview" +) + +func ghBinding(url string) *spiceboxv1alpha1.ChannelBinding { + b := &spiceboxv1alpha1.ChannelBinding{Name: "demo-reviewbot-gh", Kind: "github", Key: "pr:demo-org/platform#42"} + if url != "" { + b.External = map[string]string{"origin_url": url} + } + return b +} + +func slackBinding(permalink string) *spiceboxv1alpha1.ChannelBinding { + b := &spiceboxv1alpha1.ChannelBinding{Name: "demo-reviewbot-slack", Kind: "slack", + External: map[string]string{"channel_id": "C1", "thread_ts": "1700.1"}} + if permalink != "" { + b.External["permalink"] = permalink + } + return b +} + +// TestOriginLinksFrom exercises the walk over input+output bindings, the +// registry dispatch to each kind's OriginLinkProvider, and de-duplication — +// the registry is populated by webd's own blank imports (github, slack). +func TestOriginLinksFrom(t *testing.T) { + const prURL = "https://github.com/demo-org/platform/pull/42" + const slackURL = "https://demo.slack.com/archives/C1/p17001" + + cases := []struct { + name string + in *spiceboxv1alpha1.ChannelBinding + out *spiceboxv1alpha1.ChannelBinding + want []artifactview.OriginLink + }{ + { + name: "reviewbot: github input + slack output, input first", + in: ghBinding(prURL), + out: slackBinding(slackURL), + want: []artifactview.OriginLink{ + {Href: prURL, ChannelKind: "github"}, + {Href: slackURL, ChannelKind: "slack"}, + }, + }, + { + name: "interactive slack: input only, no output", + in: slackBinding(slackURL), + out: nil, + want: []artifactview.OriginLink{{Href: slackURL, ChannelKind: "slack"}}, + }, + { + name: "input==output same thread: de-duped to one link", + in: slackBinding(slackURL), + out: slackBinding(slackURL), + want: []artifactview.OriginLink{{Href: slackURL, ChannelKind: "slack"}}, + }, + { + name: "kubectl-driven: no bindings, empty", + in: nil, + out: nil, + want: nil, + }, + { + name: "github reconstructs from key when origin_url absent", + in: ghBinding(""), + out: nil, + want: []artifactview.OriginLink{{Href: prURL, ChannelKind: "github"}}, + }, + { + name: "slack output without permalink: omitted", + in: ghBinding(prURL), + out: slackBinding(""), + want: []artifactview.OriginLink{{Href: prURL, ChannelKind: "github"}}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + sess := &spiceboxv1alpha1.AgentSession{} + sess.Spec.InputChannel = tc.in + sess.Spec.OutputChannel = tc.out + assert.Equal(t, tc.want, originLinksFrom(sess)) + }) + } +} diff --git a/pkg/web/webui/artifactview/artifactview_test.go b/pkg/web/webui/artifactview/artifactview_test.go index 9f6844f7..70bd17a8 100644 --- a/pkg/web/webui/artifactview/artifactview_test.go +++ b/pkg/web/webui/artifactview/artifactview_test.go @@ -37,6 +37,7 @@ type fakeDeps struct { checkInteract func(ctx context.Context, ns, sess, subject string) (bool, error) artifactMeta func(ctx context.Context, ns, sess, artifactID string) (string, string, error) channelKind func(ctx context.Context, ns, sess string) (string, error) + originLinks func(ctx context.Context, ns, sess string) []artifactview.OriginLink sessionViews func(ctx context.Context, ns, sess string) []string resolveRender func(ctx context.Context, ns, sess, artifactID string) (string, error) contentRender func(ctx context.Context, ns, sess, artifactID string) (string, bool, error) @@ -82,6 +83,12 @@ func (f *fakeDeps) ChannelKind(ctx context.Context, ns, sess string) (string, er } return f.channelKind(ctx, ns, sess) } +func (f *fakeDeps) OriginLinks(ctx context.Context, ns, sess string) []artifactview.OriginLink { + if f.originLinks == nil { + return nil + } + return f.originLinks(ctx, ns, sess) +} func (f *fakeDeps) ResolveRender(ctx context.Context, ns, sess, artifactID string) (string, error) { return f.resolveRender(ctx, ns, sess, artifactID) } diff --git a/pkg/web/webui/artifactview/deps.go b/pkg/web/webui/artifactview/deps.go index 284265d9..c671c72b 100644 --- a/pkg/web/webui/artifactview/deps.go +++ b/pkg/web/webui/artifactview/deps.go @@ -29,6 +29,14 @@ type Deps interface { // used to pick the thread-link icon in the live-view toolbar. Returns "" // (with nil error) when the kind is unknown. ChannelKind(ctx context.Context, ns, sess string) (string, error) + // OriginLinks returns the de-duplicated, input-first set of "back to origin" + // links for the session — a GitHub pull request, a Slack thread — each + // derived from a channel binding by that kind's OriginLinkProvider. Empty for + // a kubectl-driven session, or one whose bound kinds expose no origin link. + // Never errors: a lookup failure degrades to fewer links and is logged by the + // implementation. Read at page build so it reflects whatever the session's + // bindings currently carry, rather than anything baked into a link. + OriginLinks(ctx context.Context, ns, sess string) []OriginLink // SessionViews returns the granted session_views.interactions for the // session's AgentClass — the interaction kinds a browser view of this session // may submit (e.g. "user_message"). Nil when the capability is absent, @@ -117,6 +125,15 @@ type Deps interface { Logger() logr.Logger } +// OriginLink is one "back to where this came from" link shown in the +// artifact-view toolbar: the Href to open and the ChannelKind that selects its +// icon and tooltip. Built by Deps.OriginLinks from the session's channel +// bindings; the artifactview package never resolves kinds itself. +type OriginLink struct { + Href string `json:"href"` + ChannelKind string `json:"channelKind"` +} + // StatusSnapshot mirrors the session's presentation status for the live-view. type StatusSnapshot struct { Phase string `json:"phase"` // AgentSession phase, e.g. "Running" diff --git a/pkg/web/webui/artifactview/live_test.go b/pkg/web/webui/artifactview/live_test.go index 1b8b4ad2..29a918b4 100644 --- a/pkg/web/webui/artifactview/live_test.go +++ b/pkg/web/webui/artifactview/live_test.go @@ -89,6 +89,9 @@ func (f *liveFakeDeps) ArtifactMeta(ctx context.Context, ns, sess, artifactID st func (f *liveFakeDeps) ChannelKind(ctx context.Context, ns, sess string) (string, error) { return "", nil } +func (f *liveFakeDeps) OriginLinks(ctx context.Context, ns, sess string) []OriginLink { + return nil // the live-socket tests don't exercise origin links +} func (f *liveFakeDeps) SessionViews(ctx context.Context, ns, sess string) []string { return nil // the live-socket tests don't exercise session_views } From e101bc74a4885d77a027965108fefd26a147355e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 14:56:10 -0400 Subject: [PATCH 08/14] feat(artifactview): page props carry session-derived origin links; drop ChannelKind --- internal/cmd/webd/main.go | 15 ------ .../webui/artifactview/artifactview_test.go | 46 +++++++++---------- pkg/web/webui/artifactview/deps.go | 4 -- pkg/web/webui/artifactview/live_test.go | 3 -- pkg/web/webui/artifactview/page.go | 25 +++++----- 5 files changed, 34 insertions(+), 59 deletions(-) diff --git a/internal/cmd/webd/main.go b/internal/cmd/webd/main.go index c04674ce..45bae0d5 100644 --- a/internal/cmd/webd/main.go +++ b/internal/cmd/webd/main.go @@ -1238,21 +1238,6 @@ func (d *artifactViewDeps) ArtifactMeta(ctx context.Context, ns, sess, artifactI return head.Name, head.Description, nil } -// ChannelKind returns the AgentSession's input-channel kind (e.g. "slack") so the -// live-view toolbar can pick the right thread-link icon. Returns "" (with nil -// error) when the session has no input channel (kubectl-driven sessions). On a -// Get error it returns ("", err) so the page can log and fall back. -func (d *artifactViewDeps) ChannelKind(ctx context.Context, ns, sess string) (string, error) { - var cur spiceboxv1alpha1.AgentSession - if err := d.K8s().Get(ctx, client.ObjectKey{Namespace: ns, Name: sess}, &cur); err != nil { - return "", err - } - if cur.Spec.InputChannel == nil { - return "", nil - } - return cur.Spec.InputChannel.Kind, nil -} - // SessionViews implements artifactview.Deps: returns the granted // session_views.interactions for the session's AgentClass, resolved by // agentcaps.ResolveSessionViews from spec — NEVER status, so a capability diff --git a/pkg/web/webui/artifactview/artifactview_test.go b/pkg/web/webui/artifactview/artifactview_test.go index 70bd17a8..8862106b 100644 --- a/pkg/web/webui/artifactview/artifactview_test.go +++ b/pkg/web/webui/artifactview/artifactview_test.go @@ -36,7 +36,6 @@ type fakeDeps struct { checkView func(ctx context.Context, artifactID, subject string) (bool, error) checkInteract func(ctx context.Context, ns, sess, subject string) (bool, error) artifactMeta func(ctx context.Context, ns, sess, artifactID string) (string, string, error) - channelKind func(ctx context.Context, ns, sess string) (string, error) originLinks func(ctx context.Context, ns, sess string) []artifactview.OriginLink sessionViews func(ctx context.Context, ns, sess string) []string resolveRender func(ctx context.Context, ns, sess, artifactID string) (string, error) @@ -77,12 +76,6 @@ func (f *fakeDeps) ArtifactMeta(ctx context.Context, ns, sess, artifactID string } return f.artifactMeta(ctx, ns, sess, artifactID) } -func (f *fakeDeps) ChannelKind(ctx context.Context, ns, sess string) (string, error) { - if f.channelKind == nil { - return "", nil - } - return f.channelKind(ctx, ns, sess) -} func (f *fakeDeps) OriginLinks(ctx context.Context, ns, sess string) []artifactview.OriginLink { if f.originLinks == nil { return nil @@ -233,9 +226,6 @@ func fakeAV() *fakeDeps { checkView: func(ctx context.Context, artifactID, subject string) (bool, error) { return true, nil }, - channelKind: func(ctx context.Context, ns, sess string) (string, error) { - return "slack", nil - }, resolveRender: func(ctx context.Context, ns, sess, artifactID string) (string, error) { return "ar-1", nil }, @@ -518,10 +508,13 @@ func TestShell_ShowsArtifactTitleAndDescription(t *testing.T) { assert.Contains(t, body, "Live view — Quarterly Report", "document title carries the artifact name") } -func TestShell_RendersBackLinkWhenPresent(t *testing.T) { +func TestShell_RendersOriginLinksWhenPresent(t *testing.T) { av := fakeAV() - av.verifyLink = func(raw string) (string, string, string, error) { - return "artifact-x", "default/s1", "https://example.slack.com/archives/C1/p170?thread_ts=170&cid=C1", nil + av.originLinks = func(ctx context.Context, ns, sess string) []artifactview.OriginLink { + return []artifactview.OriginLink{ + {Href: "https://github.com/demo-org/platform/pull/42", ChannelKind: "github"}, + {Href: "https://example.slack.com/archives/C1/p170", ChannelKind: "slack"}, + } } s := newServer(t, "user:abc", av) @@ -530,21 +523,23 @@ func TestShell_RendersBackLinkWhenPresent(t *testing.T) { require.Equal(t, http.StatusOK, rec.Code) body := rec.Body.String() - // The back link is now passed to the client via props (json key "backLink"); - // the React app renders the "Back to thread" affordance. The bootstrap JSON is - // HTML-escaped, so the URL's "&" appears as "&" — match the stable prefix. - assert.Contains(t, body, `"backLink":"https://example.slack.com/archives/C1/p170`, "bootstrap props carry the back-link URL") + // The origin links are passed to the client via props (json key "originLinks"); + // the React app renders one "back to origin" affordance per entry. + assert.Contains(t, body, `"originLinks":[`, "bootstrap props carry the origin links array") + assert.Contains(t, body, `"href":"https://github.com/demo-org/platform/pull/42"`, "props carry the PR link") + assert.Contains(t, body, `"channelKind":"github"`, "props carry the PR link's channel kind") + assert.Contains(t, body, `"href":"https://example.slack.com/archives/C1/p170"`, "props carry the Slack thread link") } -func TestShell_OmitsBackLinkWhenEmpty(t *testing.T) { - av := fakeAV() // fakeAV's verifyLink returns "" back-link +func TestShell_OriginLinksEmptyIsAnArray(t *testing.T) { + av := fakeAV() // fakeAV's originLinks unset → the page coerces nil to [] s := newServer(t, "user:abc", av) rec := httptest.NewRecorder() s.ServeHTTP(rec, req(trustedHost, "/artifact-view?d=AA&sig=BB")) require.Equal(t, http.StatusOK, rec.Code) - assert.Contains(t, rec.Body.String(), `"backLink":""`, "bootstrap props carry an empty back-link when none is set") + assert.Contains(t, rec.Body.String(), `"originLinks":[]`, "empty origin links serialize as an array, never null") } func TestShell_FallsBackToArtifactIDWhenNoName(t *testing.T) { @@ -676,7 +671,10 @@ func TestShell_UnsignedParams_AdminPath(t *testing.T) { assert.Equal(t, "user:admin", gotSubject, "CheckView runs with the authenticated cookie subject") assert.Equal(t, "artifact-x", gotArtifactID, "CheckView runs with the query artifact id") assert.Contains(t, rec.Body.String(), `data-app="artifact-view"`, "shell mounts the React app") - assert.Contains(t, rec.Body.String(), `"backLink":""`, "the unsigned admin path carries no back link") + // Origin links are session-derived now, not carried in the link, so the + // unsigned/durable path produces them too (the whole point of the change). + // This fake returns none, so an empty array — never null. + assert.Contains(t, rec.Body.String(), `"originLinks":[]`, "the unsigned admin path still derives origin links from the session") } // TestShell_UnsignedParams_ForbiddenStillGated confirms the unsigned path is NOT @@ -1010,9 +1008,9 @@ func TestUnboundSessionRef_RefusedOnEveryRoute(t *testing.T) { return []artifactview.RevisionMeta{{Seq: 1, RevisionID: "rev-1", RenderName: "ar-1"}}, nil } var touched []string - av.channelKind = func(ctx context.Context, ns, sess string) (string, error) { - touched = append(touched, "ChannelKind "+ns+"/"+sess) - return "slack", nil + av.originLinks = func(ctx context.Context, ns, sess string) []artifactview.OriginLink { + touched = append(touched, "OriginLinks "+ns+"/"+sess) + return nil } av.artifactMeta = func(ctx context.Context, ns, sess, artifactID string) (string, string, error) { touched = append(touched, "ArtifactMeta "+ns+"/"+sess) diff --git a/pkg/web/webui/artifactview/deps.go b/pkg/web/webui/artifactview/deps.go index c671c72b..32f39e79 100644 --- a/pkg/web/webui/artifactview/deps.go +++ b/pkg/web/webui/artifactview/deps.go @@ -25,10 +25,6 @@ type Deps interface { // agent-supplied title/summary). Both empty (with nil error) when the head // has no name/description — the shell falls back to the artifact id. ArtifactMeta(ctx context.Context, ns, sess, artifactID string) (name, description string, err error) - // ChannelKind returns the AgentSession's input-channel kind (e.g. "slack"), - // used to pick the thread-link icon in the live-view toolbar. Returns "" - // (with nil error) when the kind is unknown. - ChannelKind(ctx context.Context, ns, sess string) (string, error) // OriginLinks returns the de-duplicated, input-first set of "back to origin" // links for the session — a GitHub pull request, a Slack thread — each // derived from a channel binding by that kind's OriginLinkProvider. Empty for diff --git a/pkg/web/webui/artifactview/live_test.go b/pkg/web/webui/artifactview/live_test.go index 29a918b4..43a40542 100644 --- a/pkg/web/webui/artifactview/live_test.go +++ b/pkg/web/webui/artifactview/live_test.go @@ -86,9 +86,6 @@ func (f *liveFakeDeps) PreviewChildRender(ctx context.Context, ns, sess, revID s func (f *liveFakeDeps) ArtifactMeta(ctx context.Context, ns, sess, artifactID string) (string, string, error) { return "", "", nil } -func (f *liveFakeDeps) ChannelKind(ctx context.Context, ns, sess string) (string, error) { - return "", nil -} func (f *liveFakeDeps) OriginLinks(ctx context.Context, ns, sess string) []OriginLink { return nil // the live-socket tests don't exercise origin links } diff --git a/pkg/web/webui/artifactview/page.go b/pkg/web/webui/artifactview/page.go index d6030d97..307ac8bc 100644 --- a/pkg/web/webui/artifactview/page.go +++ b/pkg/web/webui/artifactview/page.go @@ -22,8 +22,10 @@ type artifactViewProps struct { HostURL string `json:"hostUrl"` ContentURL string `json:"contentUrl"` // raw render URL; empty alongside an empty HostURL SandboxOrigin string `json:"sandboxOrigin"` // bare scheme://host; "" means "no bridge" - BackLink string `json:"backLink"` // deep link back to the originating thread; "" when unsigned - ChannelKind string `json:"channelKind"` // originating channel kind for the thread icon; "" ⇒ generic + // OriginLinks are the "back to origin" links derived from the session's + // channel bindings (a GitHub PR, a Slack thread) — one per distinct origin, + // input first. Always an array (never null) so the client can map over it. + OriginLinks []OriginLink `json:"originLinks"` // Interactions lists the session_views interaction kinds this session's // class grants (e.g. ["user_message"]) — empty when the capability is // absent/inactive. The client shows the chat/annotator only when this is @@ -45,7 +47,7 @@ type artifactViewProps struct { // token, and returns props for the React shell. Failures return a // *webui.PageError so the framework renders a styled system page. // -// Every session-scoped lookup below (ChannelKind, ArtifactMeta, SessionViews, +// Every session-scoped lookup below (OriginLinks, ArtifactMeta, SessionViews, // ResolveRender) takes its (ns, sess) from the binding, so the shell's props — // including the Ns/Name the client stamps onto its /interact posts — can only // ever name the session the artifact actually lives in. @@ -56,14 +58,12 @@ func shellPageBuild(av Deps) func(context.Context, *http.Request) (any, webui.Pa return nil, webui.PageMeta{}, denial.pageError() } artifactID, ns, sess, sessionRef := b.artifactID, b.ns, b.sess, b.sessionRef() - // Best-effort: the thread-link icon depends on the originating channel - // kind. A lookup failure is non-fatal — the client falls back to a - // generic icon when channelKind is "". - channelKind, ckErr := av.ChannelKind(ctx, ns, sess) - if ckErr != nil { - av.Logger().Error(ckErr, "artifactview page: ChannelKind errored; falling back to generic icon", - "sessionRef", sessionRef) - channelKind = "" + // "Back to origin" links derived from the session's channel bindings. A + // best-effort lookup (OriginLinks never errors), coerced to a non-nil + // slice so the bootstrap JSON is always an array the client can map over. + originLinks := av.OriginLinks(ctx, ns, sess) + if originLinks == nil { + originLinks = []OriginLink{} } title, description := artifactID, "" if name, desc, merr := av.ArtifactMeta(ctx, ns, sess, artifactID); merr != nil { @@ -112,8 +112,7 @@ func shellPageBuild(av Deps) func(context.Context, *http.Request) (any, webui.Pa // it must be the bare scheme://host — and "" when the configured value // is not one, which the client reads as "no bridge", not "any origin". SandboxOrigin: sandboxOrigin(av), - BackLink: b.backLink, - ChannelKind: channelKind, + OriginLinks: originLinks, Interactions: interactions, Ns: ns, Name: sess, From 8663f938d9655699f8f66c9b3e673ebf7bad0a85 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 14 Oct 2026 15:02:19 -0400 Subject: [PATCH 09/14] feat(artifactview-ui): multi-origin link toolbar; rebuild embedded bundle --- .../artifactview/ui/ArtifactView.test.tsx | 29 +++++++++++++++++-- .../webui/artifactview/ui/ArtifactView.tsx | 4 ++- pkg/web/webui/artifactview/ui/ThreadLink.tsx | 13 ++++++--- pkg/web/webui/artifactview/ui/icons.tsx | 8 +++-- pkg/web/webui/artifactview/ui/types.ts | 14 +++++++-- ....D-zSSgc9.js => IdentityShell.TrX4KGTT.js} | 2 +- ...r.lKrMgKCy.js => NoticeBanner.Cb0ydJUm.js} | 2 +- ...atForm.BuKUBhkS.js => PatForm.CdXVdvKG.js} | 2 +- ...ne.D55RHpak.js => StartupLine.dTU6wATp.js} | 4 +-- .../{admin.C9jbAoMt.js => admin.MaoyR7Ir.js} | 2 +- .../webassets/dist/artifact-view.6EeNZWad.js | 1 + .../webassets/dist/artifact-view.D4Y_FHws.js | 1 - .../{badge.CIlGQKPh.js => badge.DvYAzVxu.js} | 2 +- ...bed.B187PZKG.js => chat-embed.3cfWODlt.js} | 2 +- ...{health.DtWM4D-G.js => health.C6C7lBkp.js} | 2 +- ...WBBM.js => identity-link-form.Mixfe5u_.js} | 2 +- ....BpTVpI-G.js => identity-link.wPfONoEL.js} | 2 +- ...FvlFsRb.js => identity-portal.Gej2sCAH.js} | 2 +- ...hi.js => identity-verify-warn.BDNNThsq.js} | 2 +- pkg/web/webui/webassets/dist/manifest.json | 26 ++++++++--------- ...kdown.FOTo-3Mz.js => markdown.juvCTczu.js} | 2 +- ...ost.DUWNbgMq.js => mcpui-host.Ct8uvgg3.js} | 2 +- ...ame.DtAiu0oG.js => model-name.Dy57aANF.js} | 2 +- .../{mount.BOEH1oNx.js => mount.xeVglEQp.js} | 2 +- ...w.dtf-izky.js => session-view.CmrYvEPX.js} | 4 +-- .../webui/webassets/dist/sessions.DrV0Blkf.js | 1 - .../webui/webassets/dist/sessions.iwFSJDZX.js | 1 + ...tings.Bs9xT9hr.js => settings.BfL9tuGX.js} | 2 +- ...{system.DT_ax8I5.js => system.DIAUHtwl.js} | 2 +- ...{vendor.C5MRNvUt.js => vendor.DRgfXKmB.js} | 2 +- 30 files changed, 93 insertions(+), 49 deletions(-) rename pkg/web/webui/webassets/dist/{IdentityShell.D-zSSgc9.js => IdentityShell.TrX4KGTT.js} (73%) rename pkg/web/webui/webassets/dist/{NoticeBanner.lKrMgKCy.js => NoticeBanner.Cb0ydJUm.js} (83%) rename pkg/web/webui/webassets/dist/{PatForm.BuKUBhkS.js => PatForm.CdXVdvKG.js} (85%) rename pkg/web/webui/webassets/dist/{StartupLine.D55RHpak.js => StartupLine.dTU6wATp.js} (71%) rename pkg/web/webui/webassets/dist/{admin.C9jbAoMt.js => admin.MaoyR7Ir.js} (99%) create mode 100644 pkg/web/webui/webassets/dist/artifact-view.6EeNZWad.js delete mode 100644 pkg/web/webui/webassets/dist/artifact-view.D4Y_FHws.js rename pkg/web/webui/webassets/dist/{badge.CIlGQKPh.js => badge.DvYAzVxu.js} (67%) rename pkg/web/webui/webassets/dist/{chat-embed.B187PZKG.js => chat-embed.3cfWODlt.js} (50%) rename pkg/web/webui/webassets/dist/{health.DtWM4D-G.js => health.C6C7lBkp.js} (72%) rename pkg/web/webui/webassets/dist/{identity-link-form.vFiHWBBM.js => identity-link-form.Mixfe5u_.js} (65%) rename pkg/web/webui/webassets/dist/{identity-link.BpTVpI-G.js => identity-link.wPfONoEL.js} (86%) rename pkg/web/webui/webassets/dist/{identity-portal.BFvlFsRb.js => identity-portal.Gej2sCAH.js} (90%) rename pkg/web/webui/webassets/dist/{identity-verify-warn.CN_Li5hi.js => identity-verify-warn.BDNNThsq.js} (79%) rename pkg/web/webui/webassets/dist/{markdown.FOTo-3Mz.js => markdown.juvCTczu.js} (92%) rename pkg/web/webui/webassets/dist/{mcpui-host.DUWNbgMq.js => mcpui-host.Ct8uvgg3.js} (97%) rename pkg/web/webui/webassets/dist/{model-name.DtAiu0oG.js => model-name.Dy57aANF.js} (85%) rename pkg/web/webui/webassets/dist/{mount.BOEH1oNx.js => mount.xeVglEQp.js} (98%) rename pkg/web/webui/webassets/dist/{session-view.dtf-izky.js => session-view.CmrYvEPX.js} (93%) delete mode 100644 pkg/web/webui/webassets/dist/sessions.DrV0Blkf.js create mode 100644 pkg/web/webui/webassets/dist/sessions.iwFSJDZX.js rename pkg/web/webui/webassets/dist/{settings.Bs9xT9hr.js => settings.BfL9tuGX.js} (99%) rename pkg/web/webui/webassets/dist/{system.DT_ax8I5.js => system.DIAUHtwl.js} (83%) rename pkg/web/webui/webassets/dist/{vendor.C5MRNvUt.js => vendor.DRgfXKmB.js} (99%) diff --git a/pkg/web/webui/artifactview/ui/ArtifactView.test.tsx b/pkg/web/webui/artifactview/ui/ArtifactView.test.tsx index b77aafa2..3facb5d1 100644 --- a/pkg/web/webui/artifactview/ui/ArtifactView.test.tsx +++ b/pkg/web/webui/artifactview/ui/ArtifactView.test.tsx @@ -32,8 +32,7 @@ const baseProps = { hostUrl: "", contentUrl: "", sandboxOrigin: "https://example.com", - backLink: "/thread/1", - channelKind: "slack", + originLinks: [] as { href: string; channelKind: string }[], ns: "default", name: "sess-1", artifactId: "art-1", @@ -217,3 +216,29 @@ describe("ArtifactView – chat panel gating", () => { vi.unstubAllGlobals(); }); }); + +describe("ArtifactView – origin links toolbar", () => { + it("renders one external link per origin, with the right hrefs", () => { + const { container } = render( + , + ); + // ThreadLink is the only toolbar anchor opened in a new tab (Download uses + // the download attribute, not target=_blank). + const links = Array.from(container.querySelectorAll('a[target="_blank"]')) as HTMLAnchorElement[]; + expect(links.map((a) => a.getAttribute("href"))).toEqual([ + "https://github.com/demo-org/platform/pull/42", + "https://example.slack.com/archives/C1/p170", + ]); + }); + + it("renders no origin link when there are none", () => { + const { container } = render(); + expect(container.querySelectorAll('a[target="_blank"]').length).toBe(0); + }); +}); diff --git a/pkg/web/webui/artifactview/ui/ArtifactView.tsx b/pkg/web/webui/artifactview/ui/ArtifactView.tsx index 6ecfde9f..e0cf6509 100644 --- a/pkg/web/webui/artifactview/ui/ArtifactView.tsx +++ b/pkg/web/webui/artifactview/ui/ArtifactView.tsx @@ -277,7 +277,9 @@ export function ArtifactView(props: ArtifactViewProps) { >