Skip to content
Open
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
52 changes: 52 additions & 0 deletions services/tuttid/service/cli/framework/framework_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,3 +360,55 @@ func (f fakeWorkspaceCatalog) Startup(context.Context) (*workspacebiz.Summary, e
func (fakeWorkspaceCatalog) Get(_ context.Context, workspaceID string) (workspacebiz.Summary, error) {
return workspacebiz.Summary{ID: workspaceID}, nil
}

// Regression test for #2193: an advertise-required field with a hidden
// alternate must render as an anyOf branch instead of a hard required entry.
func TestSchemaRendersAnyOfForAdvertisedRequiredWithAlternate(t *testing.T) {
type input struct {
AgentID string `cli:"agent-id" advertise-required:"true" advertise-alt:"provider" hint:"Use agent list --json to discover available agents."`
Prompt string `cli:"prompt" validate:"required"`
Provider string `cli:"provider" hidden:"true"`
}
schema := Schema(FromStruct[input]())

required, _ := schema["required"].([]string)
if len(required) != 1 || required[0] != "prompt" {
t.Fatalf("schema required = %#v, want only prompt", required)
}
branches, ok := schema["anyOf"].([]map[string]any)
if !ok || len(branches) != 2 {
t.Fatalf("schema anyOf = %#v, want one branch per accepted spelling", schema["anyOf"])
}
first, _ := branches[0]["required"].([]string)
second, _ := branches[1]["required"].([]string)
if len(first) != 1 || first[0] != "agent-id" || len(second) != 1 || second[0] != "provider" {
t.Fatalf("anyOf branches required = [%#v %#v], want [[agent-id] [provider]]", first, second)
}
for _, branch := range branches {
if branch["type"] != "object" {
t.Fatalf("anyOf branch missing object type: %#v", branch)
}
}
properties := schema["properties"].(map[string]any)
if _, exists := properties["provider"]; exists {
t.Fatal("hidden provider field leaked into advertised properties")
}
}

// Without an alternate, an advertise-required field keeps rendering as a hard
// required entry (existing advertised contracts unchanged).
func TestSchemaKeepsHardRequiredForAdvertisedRequiredWithoutAlternate(t *testing.T) {
type input struct {
AgentID string `cli:"agent-id" advertise-required:"true"`
Prompt string `cli:"prompt"`
}
schema := Schema(FromStruct[input]())

required, _ := schema["required"].([]string)
if len(required) != 1 || required[0] != "agent-id" {
t.Fatalf("schema required = %#v, want [agent-id]", required)
}
if _, exists := schema["anyOf"]; exists {
t.Fatalf("schema anyOf = %#v, want absent", schema["anyOf"])
}
}
38 changes: 30 additions & 8 deletions services/tuttid/service/cli/framework/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ func FromStruct[T any]() InputSpec {
name = kebabCase(field.Name)
}
fieldSpec := FieldSpec{
Name: name,
Type: schemaType(field.Type),
Description: strings.TrimSpace(field.Tag.Get("description")),
Hidden: field.Tag.Get("hidden") == "true",
AdvertisedRequired: field.Tag.Get("advertise-required") == "true",
Hint: strings.TrimSpace(field.Tag.Get("hint")),
Default: typedDefault(field.Type, field.Tag.Get("default")),
Name: name,
Type: schemaType(field.Type),
Description: strings.TrimSpace(field.Tag.Get("description")),
Hidden: field.Tag.Get("hidden") == "true",
AdvertisedRequired: field.Tag.Get("advertise-required") == "true",
AdvertiseAlternates: parseCSVTag(field.Tag.Get("advertise-alt")),
Hint: strings.TrimSpace(field.Tag.Get("hint")),
Default: typedDefault(field.Type, field.Tag.Get("default")),
}
applyValidateTag(&fieldSpec, field.Tag.Get("validate"))
fieldSpec.Enum = parseCSVTag(field.Tag.Get("enum"))
Expand All @@ -58,6 +59,7 @@ func Schema(input InputSpec) map[string]any {
}
properties := schema["properties"].(map[string]any)
required := []string{}
var alternatives []map[string]any
for _, field := range input.Fields {
if field.Hidden {
continue
Expand Down Expand Up @@ -86,13 +88,33 @@ func Schema(input InputSpec) map[string]any {
property["default"] = field.Default
}
properties[field.Name] = property
if field.Required || field.AdvertisedRequired {
switch {
case field.Required:
required = append(required, field.Name)
case field.AdvertisedRequired && len(field.AdvertiseAlternates) > 0:
// The advertised contract keeps this field required, but a hidden
// compatibility selector can satisfy the handler instead. Emit one
// anyOf branch per accepted spelling so the invocation validator
// accepts any one of them (issue #2193). A single branch listing
// every name would require all of them at once.
for _, name := range append([]string{field.Name}, field.AdvertiseAlternates...) {
alternatives = append(alternatives, map[string]any{
// The invocation validator only applies "required" to
// branches that declare an object type.
"type": "object",
"required": []string{name},
})
}
case field.AdvertisedRequired:
required = append(required, field.Name)
}
}
if len(required) > 0 {
schema["required"] = required
}
if len(alternatives) > 0 {
schema["anyOf"] = alternatives
}
return schema
}

Expand Down
17 changes: 11 additions & 6 deletions services/tuttid/service/cli/framework/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,17 @@ type FieldSpec struct {
// AdvertisedRequired keeps the canonical capability contract strict while
// allowing a hidden compatibility selector to satisfy runtime validation.
AdvertisedRequired bool
Required bool
Hint string
Min *int64
Max *int64
Enum []string
Default any
// AdvertiseAlternates names hidden input fields that can satisfy an
// AdvertisedRequired field at runtime (e.g. the deprecated --provider
// selector standing in for --agent-id). Schema() expresses this as an
// anyOf branch so invocation validation accepts either spelling.
AdvertiseAlternates []string
Required bool
Hint string
Min *int64
Max *int64
Enum []string
Default any
}

type InputSpec struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

type composerOptionsInput struct {
AgentID string `cli:"agent-id" advertise-required:"true" hint:"Use agent list --json to discover available agents."`
AgentID string `cli:"agent-id" advertise-required:"true" advertise-alt:"provider" hint:"Use agent list --json to discover available agents."`
Cwd string `cli:"cwd"`
Locale string `cli:"locale"`
Model string `cli:"model"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1099,9 +1099,21 @@ func TestStartCommandRequiresOneSelectorAndPrompt(t *testing.T) {
if !ok {
t.Fatalf("required schema = %#v", command.Capability.InputSchema["required"])
}
if len(required) != 2 || required[0] != "agent-id" || required[1] != "prompt" {
if len(required) != 1 || required[0] != "prompt" {
t.Fatalf("required = %#v", required)
}
// agent-id is advertise-required with the hidden legacy --provider as its
// alternate, so it renders as an anyOf branch rather than a hard required
// entry (issue #2193).
branches, ok := command.Capability.InputSchema["anyOf"].([]map[string]any)
if !ok || len(branches) != 2 {
t.Fatalf("anyOf schema = %#v, want one branch per accepted spelling", command.Capability.InputSchema["anyOf"])
}
first, _ := branches[0]["required"].([]string)
second, _ := branches[1]["required"].([]string)
if len(first) != 1 || first[0] != "agent-id" || len(second) != 1 || second[0] != "provider" {
t.Fatalf("anyOf branches required = [%#v %#v], want [[agent-id] [provider]]", first, second)
}

for name, input := range map[string]map[string]any{
"missing agent id": {"model": "gpt-5", "prompt": "do work"},
Expand Down Expand Up @@ -1943,9 +1955,21 @@ func TestAgentStartCommandAllowsOmittedModel(t *testing.T) {
if !ok {
t.Fatalf("required schema = %#v", command.Capability.InputSchema["required"])
}
if len(required) != 2 || required[0] != "agent-id" || required[1] != "prompt" {
if len(required) != 1 || required[0] != "prompt" {
t.Fatalf("required = %#v", required)
}
// agent-id is advertise-required with the hidden legacy --provider as its
// alternate, so it renders as an anyOf branch rather than a hard required
// entry (issue #2193).
branches, ok := command.Capability.InputSchema["anyOf"].([]map[string]any)
if !ok || len(branches) != 2 {
t.Fatalf("anyOf schema = %#v, want one branch per accepted spelling", command.Capability.InputSchema["anyOf"])
}
first, _ := branches[0]["required"].([]string)
second, _ := branches[1]["required"].([]string)
if len(first) != 1 || first[0] != "agent-id" || len(second) != 1 || second[0] != "provider" {
t.Fatalf("anyOf branches required = [%#v %#v], want [[agent-id] [provider]]", first, second)
}
_, err := command.Handler(context.Background(), cliservice.InvokeRequest{Input: map[string]any{"model": "gpt-5"}})
if !errors.Is(err, cliservice.ErrInvalidInput) {
t.Fatalf("err = %v, want ErrInvalidInput", err)
Expand Down Expand Up @@ -2605,3 +2629,55 @@ func TestActivePeersReturnsServiceProjection(t *testing.T) {
t.Fatalf("output = %#v", output.Value)
}
}

// End-to-end regression for #2193: the schema the framework generates for the
// real start command must let the legacy hidden --provider selector through
// the daemon's invocation validator. This exercises the generator output, not
// a hand-written schema.
func TestStartCommandSchemaPassesInvocationValidationForLegacySelector(t *testing.T) {
command := newTestProvider(fakeWorkspaceCatalog{startup: workspacebiz.Summary{ID: "workspace-1"}}, &fakeAgentSessions{}).newStartCommand()

registry, err := cliservice.NewRegistryFromProviders(singleCommandProvider{command: command})
if err != nil {
t.Fatalf("NewRegistryFromProviders: %v", err)
}

// The pre-binding validator must accept both spellings and reject neither.
cases := []struct {
name string
input map[string]any
wantErr bool
}{
{"canonical agent-id", map[string]any{"agent-id": agenttargetbiz.IDLocalCodex, "prompt": "hi"}, false},
{"legacy provider", map[string]any{"provider": "codex", "prompt": "hi"}, false},
{"no selector", map[string]any{"prompt": "hi"}, true},
}
for _, tc := range cases {
_, err := registry.Invoke(context.Background(), cliservice.InvokeRequest{
CommandID: command.Capability.ID,
Input: tc.input,
})
if tc.wantErr && err == nil {
t.Fatalf("%s: accepted, want rejection", tc.name)
}
if !tc.wantErr {
// The handler may still fail (fake catalog lookups); what matters
// is that the invocation validator did not reject the input.
if err != nil && strings.Contains(err.Error(), "does not match any allowed schema") {
t.Fatalf("%s: rejected by invocation validator: %v", tc.name, err)
}
}
}
}

type singleCommandProvider struct {
command cliservice.Command
}

func (p singleCommandProvider) AppID() string {
return "agentcontext-test"
}

func (p singleCommandProvider) Commands() []cliservice.Command {
return []cliservice.Command{p.command}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var sessionActionColumns = []cliservice.TableColumn{
}

type startInput struct {
AgentID string `cli:"agent-id" advertise-required:"true" hint:"Use agent list --json to discover available agents."`
AgentID string `cli:"agent-id" advertise-required:"true" advertise-alt:"provider" hint:"Use agent list --json to discover available agents."`
Cwd string `cli:"cwd"`
DisplayPrompt string `cli:"display-prompt"`
Hidden bool `cli:"hidden"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

type skillBundleInput struct {
AgentID string `cli:"agent-id" advertise-required:"true" hint:"Use agent list --json to discover available agents."`
AgentID string `cli:"agent-id" advertise-required:"true" advertise-alt:"provider" hint:"Use agent list --json to discover available agents."`
AgentSessionID string `cli:"agent-session-id"`
BrowserUse bool `cli:"browser-use"`
ComputerUse bool `cli:"computer-use"`
Expand Down
51 changes: 51 additions & 0 deletions services/tuttid/service/cli/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,54 @@ func testCommandWithPath(id string, path []string) Command {
},
}
}

// Regression test for #2193: an AdvertisedRequired field whose runtime
// requirement can be satisfied by a hidden compatibility selector must not
// reject the legacy spelling at invocation. Schema() expresses the selector as
// an anyOf branch, so validation accepts either --agent-id or --provider.
func TestRegistryInvokeAcceptsLegacyAlternateForAdvertisedRequired(t *testing.T) {
invoked := false
command := testCommand("agentcontext.agent.start")
command.Capability.InputSchema = map[string]any{
"type": "object",
"properties": map[string]any{
"agent-id": map[string]any{"type": "string"},
"prompt": map[string]any{"type": "string"},
},
"required": []string{"prompt"},
"anyOf": []any{
map[string]any{"type": "object", "required": []string{"agent-id"}},
map[string]any{"type": "object", "required": []string{"provider"}},
},
}
command.Handler = func(context.Context, InvokeRequest) (CommandOutput, error) {
invoked = true
return CommandOutput{Kind: OutputModePlain, Text: "started"}, nil
}
registry := newTestRegistry(t, command)

// Canonical spelling.
if _, err := registry.Invoke(context.Background(), InvokeRequest{
CommandID: "agentcontext.agent.start",
Input: map[string]any{"agent-id": "agent-1", "prompt": "hi"},
}); err != nil {
t.Fatalf("Invoke(agent-id): %v", err)
}
// Legacy hidden selector, the path broken since #2004.
if _, err := registry.Invoke(context.Background(), InvokeRequest{
CommandID: "agentcontext.agent.start",
Input: map[string]any{"provider": "claude", "prompt": "hi"},
}); err != nil {
t.Fatalf("Invoke(provider): %v", err)
}
// Neither spelling present is still rejected.
if _, err := registry.Invoke(context.Background(), InvokeRequest{
CommandID: "agentcontext.agent.start",
Input: map[string]any{"prompt": "hi"},
}); err == nil {
t.Fatal("Invoke(no selector): want error, got nil")
}
if !invoked {
t.Fatal("handler was not invoked for valid inputs")
}
}
Loading