diff --git a/server/pkg/agent/antigravity.go b/server/pkg/agent/antigravity.go index 99fddea01..55182bbc6 100644 --- a/server/pkg/agent/antigravity.go +++ b/server/pkg/agent/antigravity.go @@ -38,6 +38,23 @@ func (b *antigravityBackend) Execute(ctx context.Context, prompt string, opts Ex return nil, fmt.Errorf("agy executable not found at %q: %w", execPath, err) } + // Guard against agy's silent no-op on an unrecognised --model: it exits 0 + // with empty output, which would otherwise surface as a "completed" but + // empty task. opts.Model is the single funnel for both agent.model and the + // daemon-wide MULTICA_ANTIGRAVITY_MODEL default (resolved in daemon.go), so + // validating it here covers every source — UI free-text, API, a persisted + // value, and the env default alike. Reject a non-empty model the installed + // CLI definitively does not advertise, with an actionable error. Validation + // is fail-OPEN: if the `agy models` catalog can't be discovered we let agy + // resolve the value itself rather than blocking the run on a discovery + // hiccup (see antigravityModelError). + if opts.Model != "" { + catalog, _ := ListModels(ctx, "antigravity", execPath) + if err := antigravityModelError(opts.Model, catalog); err != nil { + return nil, err + } + } + timeout := opts.Timeout runCtx, cancel := runContext(ctx, timeout) @@ -215,10 +232,11 @@ var antigravityBlockedArgs = map[string]blockedArgMode{ // shell quoting. agy still exposes no --system-prompt; runtime instructions // are delivered via AGENTS.md in the task workdir. // -// Caveat: agy silently no-ops on a model string it doesn't recognise (empty -// output, exit 0), so callers should only hand over values discovered via -// `agy models`. When opts.Model is empty we omit the flag and agy resolves -// its own default. +// agy silently no-ops on a model string it doesn't recognise (empty output, +// exit 0), so Execute validates opts.Model against the `agy models` catalog +// and rejects an unrecognised value up front (see antigravityModelError) — +// by the time we build argv the value is either empty or known-good. When +// opts.Model is empty we omit the flag and agy resolves its own default. func buildAntigravityArgs(prompt, logPath string, timeout time.Duration, opts ExecOptions, logger *slog.Logger) []string { args := []string{ "-p", prompt, @@ -247,6 +265,31 @@ func buildAntigravityArgs(prompt, logPath string, timeout time.Duration, opts Ex return args } +// antigravityModelError returns an actionable error when `model` is non-empty +// and definitively absent from `available` (the `agy models` catalog); it +// returns nil otherwise. An empty `available` means discovery couldn't produce +// a catalog (agy missing, transient failure) — we fail OPEN there and let agy +// resolve the value, so a discovery hiccup never blocks a run. The match is +// exact because agy's --model wants the precise display string; a near-miss +// (extra space, dropped suffix) is correctly rejected since agy would silently +// no-op on it anyway. +func antigravityModelError(model string, available []Model) error { + if model == "" || len(available) == 0 { + return nil + } + ids := make([]string, 0, len(available)) + for _, m := range available { + if m.ID == model { + return nil + } + ids = append(ids, m.ID) + } + return fmt.Errorf( + "antigravity model %q is not available from `agy models`; pick one of: %s", + model, strings.Join(ids, ", "), + ) +} + // antigravityFormatTimeout renders a Go duration in the `ms` shape the // agy CLI accepts (e.g. 20m0s). Sub-second timeouts round up to 1s so the CLI // doesn't reject the flag. diff --git a/server/pkg/agent/antigravity_test.go b/server/pkg/agent/antigravity_test.go index 742556991..c8b60d6dd 100644 --- a/server/pkg/agent/antigravity_test.go +++ b/server/pkg/agent/antigravity_test.go @@ -214,3 +214,58 @@ func TestReadAntigravityConversationIDMissingFile(t *testing.T) { t.Errorf("expected empty string for empty path, got %q", got) } } + +// TestAntigravityModelError is the regression guard for the silent-no-op fix: +// agy exits 0 with empty output on an unrecognised --model, so Execute must +// reject a non-empty model that isn't in the `agy models` catalog instead of +// letting it run to a fake "completed + empty" success. This covers the same +// validation regardless of whether opts.Model originated from agent.model, a +// persisted/API value, or the daemon-wide MULTICA_ANTIGRAVITY_MODEL default — +// they all collapse to opts.Model before Execute runs this check. +func TestAntigravityModelError(t *testing.T) { + t.Parallel() + + catalog := []Model{ + {ID: "Gemini 3.5 Flash (Medium)", Label: "Gemini 3.5 Flash (Medium)", Provider: "antigravity"}, + {ID: "Claude Opus 4.6 (Thinking)", Label: "Claude Opus 4.6 (Thinking)", Provider: "antigravity"}, + } + + // Exact catalog hit → accepted. + if err := antigravityModelError("Claude Opus 4.6 (Thinking)", catalog); err != nil { + t.Errorf("valid model rejected: %v", err) + } + + // Empty model → accepted (flag omitted, agy resolves its own default). + if err := antigravityModelError("", catalog); err != nil { + t.Errorf("empty model should not error: %v", err) + } + + // Empty / nil catalog → fail open (discovery couldn't produce a list, so we + // can't prove the value is bad — let agy decide rather than block the run). + if err := antigravityModelError("anything at all", nil); err != nil { + t.Errorf("empty catalog should fail open, got: %v", err) + } + + // Unknown model with a known catalog → actionable error that names the + // rejected value and points at `agy models`. THIS is the case that stops + // the silent empty-success. + err := antigravityModelError("Totally Made Up Model", catalog) + if err == nil { + t.Fatal("unknown model should be rejected, not silently accepted") + } + if !strings.Contains(err.Error(), "Totally Made Up Model") { + t.Errorf("error should name the rejected model: %v", err) + } + if !strings.Contains(err.Error(), "agy models") { + t.Errorf("error should point the user at `agy models`: %v", err) + } + + // Near-miss (trailing space / dropped suffix) → still rejected, because agy + // needs the exact display string and would no-op on anything else. + if err := antigravityModelError("Claude Opus 4.6 (Thinking) ", catalog); err == nil { + t.Error("near-miss model (trailing space) should be rejected") + } + if err := antigravityModelError("Claude Opus 4.6", catalog); err == nil { + t.Error("near-miss model (dropped suffix) should be rejected") + } +}