Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ require (
github.com/ory/dockertest/v3 v3.12.0
github.com/rs/zerolog v1.34.0
github.com/sigstore/sigstore v1.10.8
github.com/slack-go/slack v0.23.1
github.com/slack-go/slack v0.30.1
github.com/spf13/cobra v1.10.2
github.com/stretchr/testify v1.12.0
github.com/stretchr/testify v1.12.1
github.com/tdewolff/parse/v2 v2.8.13
github.com/tsawler/tabula v1.6.14
github.com/warpstreamlabs/bento v1.17.0
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -860,8 +860,8 @@ github.com/sirupsen/logrus v1.10.1 h1:xi4336Zh11WpU14fXR6I67V3yaTPQYwRx2WEtHbRg4
github.com/sirupsen/logrus v1.10.1/go.mod h1:vsQHnG7xzNsxk3NrwboUiWPnIC3dmbjcGPykD7+tiHk=
github.com/skeema/knownhosts v1.3.1 h1:X2osQ+RAjK76shCbvhHHHVl3ZlgDm8apHEHFqRjnBY8=
github.com/skeema/knownhosts v1.3.1/go.mod h1:r7KTdC8l4uxWRyK2TpQZ/1o5HaSzh06ePQNxPwTcfiY=
github.com/slack-go/slack v0.23.1 h1:ZS5B96wxxYQRwvJ3/vJFtqtUZi3tXhsZCyT44Nv7M80=
github.com/slack-go/slack v0.23.1/go.mod h1:H0yR/YBuRJ39RkE+JpV/d/oEsbanzTRowR82bCN0cEs=
github.com/slack-go/slack v0.30.1 h1:q6FjZssqts6hJ+DRWCnnO2YVUh+E3/UNrC0EIO2xci8=
github.com/slack-go/slack v0.30.1/go.mod h1:radSmEe8GEgE+aK980ZafMR0AQhiZjADptN5k/gOw34=
github.com/snowflakedb/gosnowflake v1.18.1 h1:Nb4AWSnSBWe1UKpKTwCZxjYhYo1JH7GgKhO3wW1kR10=
github.com/snowflakedb/gosnowflake v1.18.1/go.mod h1:7D4+cLepOWrerVsH+tevW3zdMJ5/WrEN7ZceAC6xBv0=
github.com/spf13/afero v1.15.0 h1:b/YBCLWAJdFWJTN9cLhiXXcD7mzKn9Dm86dNnfyQw1I=
Expand Down Expand Up @@ -893,8 +893,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/stretchr/testify v1.12.0 h1:K6Mr6jO9JICuend/5xzTM03ydSV3vdNRYAdPSukj8uI=
github.com/stretchr/testify v1.12.0/go.mod h1:bOYBZb5qJ00vPzWfIqBUZPaxK8jWiXc6d3ErP4Ca9Gw=
github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE=
github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg=
github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8=
github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU=
github.com/tdewolff/parse/v2 v2.8.13 h1:si/8rLw5BZZTWCCiMm9A3f6x+RmqYfrkEeXCgpX5ick=
Expand Down
2 changes: 1 addition & 1 deletion pkg/agent/runner/leakageread_meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (l *Loop) readSidePostForMetaTool(
Tool: &pipeline.ToolCallInfo{
Name: name,
// The bytes the tool was actually called with. The gated path passes
// its stripArgTags'd copy because it strips before executing; this
// its toolenvelope.StripPtJSON'd copy because it strips before executing; this
// path does not strip at all, so the same copy the tool saw is the
// honest answer. No plural declaration reads Args anyway — a
// resource named by an ARG is knowable before the call and takes the
Expand Down
3 changes: 2 additions & 1 deletion pkg/agent/runner/loop_dispatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
lifecyclecore "github.com/authzed/openagentprimitives/pkg/agent/session/lifecycle"
"github.com/authzed/openagentprimitives/pkg/agent/tool"
"github.com/authzed/openagentprimitives/pkg/agent/tool/sandbox"
"github.com/authzed/openagentprimitives/pkg/agent/toolenvelope"
"github.com/authzed/openagentprimitives/pkg/authz/toolguard"
"github.com/authzed/openagentprimitives/pkg/platform/identity"
"github.com/authzed/openagentprimitives/pkg/platform/identity/credupdate"
Expand Down Expand Up @@ -441,7 +442,7 @@ func (l *Loop) executeToolContained(
// The tool must not see pt markup: strip any pt-untrusted envelope from the
// args before the tool runs. Inert (args unchanged) when the args carry no
// envelope.
args = stripArgTags(args)
args, _ = toolenvelope.StripPtJSON(args)

// Re-check revocation IMMEDIATELY before the call runs.
//
Expand Down
63 changes: 0 additions & 63 deletions pkg/agent/runner/pttag_argstrip.go

This file was deleted.

127 changes: 109 additions & 18 deletions pkg/agent/tool/meta/respond.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,16 @@ func New(cfg RespondConfig) tool.Tool {
}

type respondTool struct {
cfg RespondConfig
schema json.RawMessage
desc string
hasAsset bool
cfg RespondConfig
schema json.RawMessage
desc string
hasAsset bool
hasComponents bool
// components validates and projects agent-authored `blocks`. Resolved from
// the outbound kind at construction; nil when the kind advertises no
// "components" capability, or (a wiring bug) advertises it without a
// validator — Execute fails closed on the latter.
components channelkinds.ComponentValidator
}

func newRespondTool(cfg RespondConfig) *respondTool {
Expand All @@ -109,27 +115,33 @@ func newRespondTool(cfg RespondConfig) *respondTool {
}
}

// WHAT a reply may carry and HOW it is authored are the KIND's facts, not
// this tool's: the rules land in model-facing prompt text the runner
// presents as channel-specific and authoritative, so asking the registry
// keeps that promise for every registered kind. An unregistered name yields
// a nil Kind — TextFormattingInstructionsFor answers with the generic
// CommonMark line, and ComponentValidatorFor answers with nil.
kind, _ := chregistry.Get(cfg.ChannelKind)

props := map[string]any{}
required := []string{"text"}
textDesc := "The message text."
if hasMarkdown {
// WHICH markup dialect this surface speaks is the KIND's fact, not this
// tool's: the rules land in model-facing prompt text the runner presents
// as channel-specific and authoritative, so asking the registry keeps
// that promise for every registered kind. An unregistered name yields a
// nil Kind, which TextFormattingInstructionsFor answers with the generic
// CommonMark line.
kind, _ := chregistry.Get(cfg.ChannelKind)
textDesc += " " + channelkinds.TextFormattingInstructionsFor(kind)
} else {
textDesc += " Plain text only — markdown will be sent literally."
}
props["text"] = map[string]any{"type": "string", "description": textDesc}

var components channelkinds.ComponentValidator
if hasComponents {
components = channelkinds.ComponentValidatorFor(kind)
desc := "Optional structured UI components in the channel-kind's native format, rendered as the message body with `text` as the notification preview and fallback. " +
channelkinds.ComponentFormattingInstructionsFor(kind)
props["blocks"] = map[string]any{
"type": "array",
"description": "Optional structured components in the channel-kind's native format.",
"description": desc,
"items": map[string]any{"type": "object"},
}
}

Expand All @@ -156,14 +168,17 @@ func newRespondTool(cfg RespondConfig) *respondTool {
} else {
descParts = " Plain text only on this channel."
}
if hasComponents {
descParts += " Structured `blocks` components are supported."
}
desc := fmt.Sprintf(
"Send a reply to the user via the %s channel.%s Call this whenever you have a message to surface. "+
"After your reply is complete, call await_user_message to yield until the user responds, "+
"or call agent_work_complete if the current work item is done and the session should go idle.",
cfg.ChannelKind, descParts,
)

return &respondTool{cfg: cfg, schema: schemaBytes, desc: desc, hasAsset: hasAsset}
return &respondTool{cfg: cfg, schema: schemaBytes, desc: desc, hasAsset: hasAsset, hasComponents: hasComponents, components: components}
}

func (t *respondTool) Name() string { return "respond_to_user" }
Expand Down Expand Up @@ -194,14 +209,59 @@ func (t *respondTool) Execute(ctx context.Context, args json.RawMessage, sess *t
// Attached are artifact handles to attach; each is gated on the channel's
// asset:* capability.
Attached []string `json:"attached,omitempty"`
// Blocks are agent-authored structured components in the channel-kind's
// native format, gated on the channel's "components" capability and
// validated by the kind before publish.
Blocks json.RawMessage `json:"blocks,omitempty"`
}
if res, ok := tool.ParseArgs(args, &in, t.Name(), `{"text": "the message body"}`); !ok {
return res, nil
}
// Models routinely send an empty optional array ("blocks": [] or null); the
// raw bytes "[]"/"null" are non-empty, so a later len(in.Blocks) > 0 would
// treat them as a present-but-invalid payload and error an otherwise-fine
// plain-text reply (or reject on a non-components channel). Normalize "none"
// to absent so every downstream len()>0 check agrees it isn't there.
in.Blocks = normalizeOptionalBlocks(in.Blocks)
if strings.TrimSpace(in.Text) == "" {
return tool.Result{Content: "respond_to_user: `text` is required and must be non-empty. Pass the full message body you want the user to see, e.g. {\"text\": \"Here are the results: …\"}. If your previous reply was truncated, you may need to shorten it before retrying.", IsError: true, Trusted: true}, nil
}

// Validate agent-authored components BEFORE anything is published: a
// rejection returns IsError with a detailed, correctable message and leaves
// no envelope on the wire, exactly like the attachment-resolution guard
// below. Only the kind knows its component format, so the kind's validator
// (resolved from the outbound binding at construction) is the authority.
//
// pt markup is model-facing only, so the blocks a human sees are the
// STRIPPED copy — and that copy, not the tagged one, is what the validator
// must judge. Stripping can rejoin a guarded token the markers had split
// ("<!chan" + a pt region holding "nel>" publishes as "<!channel>"), so
// validating the tagged bytes would pass a payload whose published form a
// check exists to refuse. The tagged in.Blocks still feeds the leakage gate
// below, which needs the tags to decide, exactly as it does for in.Text.
outBlocks, _ := toolenvelope.StripPtJSON(in.Blocks)
if len(in.Blocks) > 0 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"blocks": [] / "blocks": null now fail the call. len(in.Blocks) > 0 is true for the raw bytes [] and null. On a non-components channel the model gets "blocks are not supported", and on Slack it gets "must contain at least one block" or "must be a JSON array". Before this change these were ignored. Models often send empty optional arrays, so a plain-text reply now errors and costs a retry turn. Treat trimmed null/[] as absent.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — respond_to_user normalizes blocks right after parsing via normalizeOptionalBlocks: a trimmed ""/null/[] becomes absent (nil), so an empty optional array no longer errors a plain-text reply. The single normalization covers every downstream check (not-supported guard, blocks+attached combo, validator, leakage projection, strip/publish, log flag), since they all key off len(in.Blocks) > 0.

if !t.hasComponents {
return tool.Result{Content: "respond_to_user: `blocks` (structured components) are not supported on this channel. Put your reply in `text` instead.", IsError: true, Trusted: true}, nil
}
if len(in.Attached) > 0 {
// blocks + attached in one reply is not supported yet: the
// attachment delivery path posts only Text, so the blocks would be
// silently dropped. Reject with a correctable error rather than lose
// them. Full blocks+files-in-one-reply support is deferred.
return tool.Result{Content: "respond_to_user: `blocks` and `attached` can't be combined in one reply on this channel yet. Send the structured layout in one respond_to_user call and the attachment(s) in another.", IsError: true, Trusted: true}, nil
}
if t.components == nil {
// Advertised "components" but no validator: a wiring bug, not the
// model's fault. Fail closed rather than publish unvalidated blocks.
return tool.Result{Trusted: true}, fmt.Errorf("respond_to_user: channel kind %q advertises components but supplies no validator", t.cfg.ChannelKind)
}
if err := t.components.ValidateComponents(outBlocks); err != nil {
return tool.Result{Content: err.Error(), IsError: true, Trusted: true}, nil
}
}

// Resolve attached artifact handles BEFORE publishing — any failure
// returns IsError without putting the envelope on the wire so the
// model can correct and retry.
Expand Down Expand Up @@ -233,7 +293,23 @@ func (t *respondTool) Execute(ctx context.Context, args json.RawMessage, sess *t
// internal config (approver subject, SpiceDB endpoint) that must not reach
// channel members.
if t.cfg.LeakageGate != nil {
if err := t.cfg.LeakageGate(ctx, sess, in.Text, attachments); err != nil {
// The gate measures TEXT. Components carry their own text, so feed the
// gate the block content too (projected to plain text by the kind) —
// otherwise an agent could route blockable content through `blocks` and
// bypass the gate, a fail-open egress path. Appended to the TAGGED
// in.Text the gate already runs on; if the agent copied pt-tagged
// content into a block it stays tagged in the projection and gets the
// same tag-aware treatment as tagged in.Text, otherwise it falls to the
// gate's coarse check like an attachment. pt markup is stripped from the
// published blocks (outBlocks, validated above in its stripped form),
// mirroring outText, so a human never sees the markup either way.
gateText := in.Text
if len(in.Blocks) > 0 && t.components != nil {
if projected := t.components.ComponentsPlainText(in.Blocks); projected != "" {
gateText = in.Text + "\n\n" + projected
}
}
if err := t.cfg.LeakageGate(ctx, sess, gateText, attachments); err != nil {
logger := log.FromContext(ctx)
pub := func(subject string, data []byte) error {
return publishWithRetry(ctx, t.cfg.NATSPublish, subject, data)
Expand Down Expand Up @@ -296,8 +372,11 @@ func (t *respondTool) Execute(ctx context.Context, args json.RawMessage, sess *t
}

// pt markup is model-facing only: strip any pt-untrusted envelope before the
// human sees the reply. The LeakageGate above ran on the TAGGED in.Text (it
// needs the tags to decide), so the strip is on the outbound copy only.
// human sees the reply. The LeakageGate above ran on the TAGGED in.Text and
// the TAGGED block projection (it needs the tags to decide), so the strip is
// on the outbound copies only — BOTH text and blocks (outBlocks, stripped
// and validated before the gate), so neither surface renders provenance
// markup to a human.
outText := in.Text
if stripped, had := toolenvelope.StripPt(in.Text); had {
outText = stripped
Expand All @@ -309,13 +388,13 @@ func (t *respondTool) Execute(ctx context.Context, args json.RawMessage, sess *t
toolUseID = ids.ToolUseID
}
logger.Info("respond_to_user: publishing user_message envelope",
"session", sess.Namespace+"/"+sess.Name, "toolUseID", toolUseID, "textLen", len(outText), "attachments", len(attachments))
"session", sess.Namespace+"/"+sess.Name, "toolUseID", toolUseID, "textLen", len(outText), "attachments", len(attachments), "components", len(in.Blocks) > 0)
publish := func(subject string, data []byte) error {
return publishWithRetry(ctx, t.cfg.NATSPublish, subject, data)
}
if err := t.cfg.EnvelopeSigner.PublishOut(publish, sess.Namespace, sess.Name,
channelevents.KindUserMessage,
channelevents.OutboundUserMessagePayload{Text: outText, Attachments: attachments},
channelevents.OutboundUserMessagePayload{Text: outText, Attachments: attachments, Components: outBlocks},
); err != nil {
return tool.Result{Content: fmt.Sprintf("publish failed: %v", err), IsError: true, Trusted: true}, nil
}
Expand Down Expand Up @@ -526,6 +605,18 @@ func ownedBySessionNamed(refs []metav1.OwnerReference, name string) bool {
return false
}

// normalizeOptionalBlocks returns nil for a blocks value the model meant as
// "none" — absent, whitespace, JSON null, or an empty array — so downstream
// len()>0 checks treat it as absent rather than as a present-but-empty payload
// that would error the call. A non-empty array is returned unchanged.
func normalizeOptionalBlocks(raw json.RawMessage) json.RawMessage {
switch strings.TrimSpace(string(raw)) {
case "", "null", "[]":
return nil
}
return raw
}

func publishWithRetry(ctx context.Context, publish func(ctx context.Context, subject string, payload []byte) error, subject string, payload []byte) error {
var lastErr error
delay := 100 * time.Millisecond
Expand Down
Loading
Loading