Artifact view: link back to the session's channels (PR + Slack thread)

cmd/oap/internal/chatcmd/sessionviewlink.go

@@ -38,7 +38,7 @@ type tuiSessionViewMinter struct {
 	webdBaseURL string
 }
 
-func (m *tuiSessionViewMinter) MintSessionViewLink(sessionRef string, _ identity.Principal, _ string) (string, error) {
+func (m *tuiSessionViewMinter) MintSessionViewLink(sessionRef string, _ identity.Principal) (string, error) {
 	return channelkinds.ComposeSessionViewURL(m.webdBaseURL, sessionRef)
 }
 

internal/cmd/channelsd/artifact_view_minter_test.go

@@ -38,9 +38,8 @@ func TestNewArtifactViewMinter_HappyPath(t *testing.T) {
 	artifactID := "art-abc123"
 	sessionRef := "default/sess-1"
 	subject := identity.RawSubject("user:alice")
-	backLink := "https://example.slack.com/archives/C1/p170?thread_ts=170&cid=C1"
 
-	u, err := m.MintArtifactViewLink(artifactID, sessionRef, subject, backLink)
+	u, err := m.MintArtifactViewLink(artifactID, sessionRef, subject)
 	require.NoError(t, err, "MintArtifactViewLink should succeed")
 	assert.True(t, strings.HasPrefix(u, base+"/artifact-view?"), "URL must start with base+/artifact-view?")
 	assert.Contains(t, u, "d=", "URL must contain d= query param")
@@ -57,7 +56,6 @@ func TestNewArtifactViewMinter_HappyPath(t *testing.T) {
 	require.NoError(t, err, "Verify with aud=webd must succeed — audience-override regression guard")
 	assert.Equal(t, artifactID, p.ArtifactID)
 	assert.Equal(t, passthroughlink.AudienceWebd, p.Audience)
-	assert.Equal(t, backLink, p.BackLink)
 }
 
 // TestNewArtifactViewMinter_EmptyBaseURL verifies that a missing webd base URL
@@ -66,7 +64,7 @@ func TestNewArtifactViewMinter_EmptyBaseURL(t *testing.T) {
 	signer := passthroughlink.New(make([]byte, 32))
 	m := newArtifactViewMinter(signer, func() string { return "" })
 
-	u, err := m.MintArtifactViewLink("art-1", "ns/sess", identity.RawSubject(""), "")
+	u, err := m.MintArtifactViewLink("art-1", "ns/sess", identity.RawSubject(""))
 	require.NoError(t, err, "empty base URL must return nil error (clean skip)")
 	assert.Empty(t, u, "empty base URL must return empty URL string")
 }

internal/cmd/channelsd/minter_wiring_race_test.go

@@ -30,7 +30,7 @@ import (
 type stubArtifactViewMinter struct{ id string }
 
 func (s *stubArtifactViewMinter) MintArtifactViewLink(
-	_, _ string, _ identity.Principal, _ string,
+	_, _ string, _ identity.Principal,
 ) (string, error) {
 	return "https://webd.example/artifact-view?stub=" + s.id, nil
 }

internal/cmd/channelsd/session_view_minter.go

@@ -36,14 +36,14 @@ func newSessionViewMinter(webdBaseURL func() string) *sessionViewMinter {
 }
 
 // MintSessionViewLink returns "<trusted-base>/session-view/<ns>/<name>" for
-// the given sessionRef ("ns/name"). subject and backLink are accepted for
-// interface parity with channelkinds.SessionViewMinter (and
-// ArtifactViewMinter) but are deliberately NOT embedded in the URL — the
+// the given sessionRef ("ns/name"). subject is accepted for interface parity
+// with channelkinds.SessionViewMinter (and ArtifactViewMinter) but is
+// deliberately NOT embedded in the URL — the
 // session-view page is a durable plain-path link, not a signed capability;
 // CheckInteract at open time is the sole authorization boundary. Returns
 // ("", nil) when the webd base URL is empty (webd not yet installed or the
 // ConfigMap not yet populated) — callers treat an empty URL as a clean skip.
-func (m *sessionViewMinter) MintSessionViewLink(sessionRef string, _ identity.Principal, _ string) (string, error) {
+func (m *sessionViewMinter) MintSessionViewLink(sessionRef string, _ identity.Principal) (string, error) {
 	if m == nil || m.WebdBaseURL == nil {
 		return "", fmt.Errorf("session view minter not configured")
 	}

internal/cmd/channelsd/session_view_minter_test.go

@@ -29,7 +29,7 @@ func TestNewSessionViewMinter_HappyPath(t *testing.T) {
 	m := newSessionViewMinter(func() string { return "https://webd.example" })
 
 	subj := identity.RawSubject("user:alice")
-	u, err := m.MintSessionViewLink("ns1/sess1", subj, "")
+	u, err := m.MintSessionViewLink("ns1/sess1", subj)
 	require.NoError(t, err, "MintSessionViewLink should succeed")
 	assert.Equal(t, "https://webd.example/session-view/ns1/sess1", u,
 		"URL must be a plain path with no ?d=&sig= — access is CheckInteract at open time, not a signed capability")
@@ -39,7 +39,7 @@ func TestNewSessionViewMinter_HappyPath(t *testing.T) {
 // the base URL doesn't produce a double slash.
 func TestNewSessionViewMinter_TrimsTrailingSlash(t *testing.T) {
 	m := newSessionViewMinter(func() string { return "https://webd.example/" })
-	u, err := m.MintSessionViewLink("ns1/sess1", identity.Principal{}, "")
+	u, err := m.MintSessionViewLink("ns1/sess1", identity.Principal{})
 	require.NoError(t, err)
 	assert.Equal(t, "https://webd.example/session-view/ns1/sess1", u)
 }
@@ -48,7 +48,7 @@ func TestNewSessionViewMinter_TrimsTrailingSlash(t *testing.T) {
 // are URL-escaped independently.
 func TestNewSessionViewMinter_EscapesSegments(t *testing.T) {
 	m := newSessionViewMinter(func() string { return "https://webd.example" })
-	u, err := m.MintSessionViewLink("my ns/my sess", identity.Principal{}, "")
+	u, err := m.MintSessionViewLink("my ns/my sess", identity.Principal{})
 	require.NoError(t, err)
 	assert.Equal(t, "https://webd.example/session-view/my%20ns/my%20sess", u)
 }
@@ -57,7 +57,7 @@ func TestNewSessionViewMinter_EscapesSegments(t *testing.T) {
 // URL returns ("", nil) — a clean skip, not an error.
 func TestNewSessionViewMinter_EmptyBaseURL(t *testing.T) {
 	m := newSessionViewMinter(func() string { return "" })
-	u, err := m.MintSessionViewLink("ns1/sess1", identity.Principal{}, "")
+	u, err := m.MintSessionViewLink("ns1/sess1", identity.Principal{})
 	require.NoError(t, err, "empty base URL must return nil error (clean skip)")
 	assert.Empty(t, u, "empty base URL must return empty URL string")
 }
@@ -66,7 +66,7 @@ func TestNewSessionViewMinter_EmptyBaseURL(t *testing.T) {
 // sessionRef without a "/" is rejected rather than producing a malformed URL.
 func TestNewSessionViewMinter_MalformedSessionRef_ReturnsError(t *testing.T) {
 	m := newSessionViewMinter(func() string { return "https://webd.example" })
-	_, err := m.MintSessionViewLink("no-slash", identity.Principal{}, "")
+	_, err := m.MintSessionViewLink("no-slash", identity.Principal{})
 	require.Error(t, err, "sessionRef without ns/name must be rejected")
 	assert.Contains(t, err.Error(), "sessionRef")
 }

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",

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)
+			}
+		})
+	}
+}

internal/cmd/webd/main.go

@@ -786,26 +786,26 @@ var _ sessions.Deps = (*artifactViewDeps)(nil)
 // ArtifactID; the returned artifactID is always "" for this purpose. Any
 // other purpose (including empty) is rejected — the bearer capability is
 // fail-closed: a link that doesn't match a known purpose grants nothing.
-func (d *artifactViewDeps) VerifyLink(raw string) (string, string, string, error) {
+func (d *artifactViewDeps) VerifyLink(raw string) (string, string, error) {
 	p, err := d.cookieSigner.Verify(raw,
 		passthroughlink.WithExpectedIssuer(passthroughlink.IssuerChannelsd),
 		passthroughlink.WithExpectedAudience(passthroughlink.AudienceWebd))
 	if err != nil {
-		return "", "", "", err
+		return "", "", err
 	}
 	switch p.Purpose {
 	case passthroughlink.PurposeArtifactView:
 		if p.ArtifactID == "" || p.SessionRef == "" {
-			return "", "", "", fmt.Errorf("link missing artifactId/sessionRef")
+			return "", "", fmt.Errorf("link missing artifactId/sessionRef")
 		}
-		return p.ArtifactID, p.SessionRef, p.BackLink, nil
+		return p.ArtifactID, p.SessionRef, nil
 	case passthroughlink.PurposeSessionView:
 		if p.SessionRef == "" {
-			return "", "", "", fmt.Errorf("link missing sessionRef")
+			return "", "", fmt.Errorf("link missing sessionRef")
 		}
-		return "", p.SessionRef, p.BackLink, nil
+		return "", p.SessionRef, nil
 	default:
-		return "", "", "", fmt.Errorf("link purpose %q is not supported", p.Purpose)
+		return "", "", fmt.Errorf("link purpose %q is not supported", p.Purpose)
 	}
 }
 
@@ -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

internal/cmd/webd/origin_links.go

@@ -0,0 +1,79 @@
+package main
+
+import (
+	"context"
+	"net/url"
+
+	"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
+		}
+		link, ok := p.OriginLink(b)
+		if !ok || link.URL == "" || seen[link.URL] {
+			return
+		}
+		// Generic backstop: these hrefs derive from the user-writable
+		// External map and are rendered as <a href>. A kind vouches for its
+		// own host (slack: *.slack.com), but every link must at least be an
+		// https URL — never a javascript:/data: scheme or a relative ref —
+		// before it reaches the toolbar. Fail-closed: drop anything that is
+		// not.
+		if !isHTTPSOriginHref(link.URL) {
+			return
+		}
+		seen[link.URL] = true
+		out = append(out, artifactview.OriginLink{Href: link.URL, Label: link.Label, Icon: link.Icon})
+	}
+	add(sess.Spec.InputChannel)
+	add(sess.Spec.OutputChannel)
+	return out
+}
+
+// isHTTPSOriginHref reports whether href is an https URL with a host — the
+// scheme backstop every origin link must pass before it is rendered as an
+// <a href>, independent of any per-kind host check. Rejects javascript:/data:
+// schemes, plain http, and relative refs.
+func isHTTPSOriginHref(href string) bool {
+	parsed, err := url.Parse(href)
+	return err == nil && parsed.Scheme == "https" && parsed.Host != ""
+}
+
+// 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. Best-effort by design: an origin link is a
+// convenience affordance, never a gate, so no failure here is surfaced as one.
+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)
+}