From ca355cccdc92fa012a19dc5e4ea87b51beadf84d Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 6 Aug 2026 17:16:56 -0500 Subject: [PATCH 1/2] fix(cli): resolve a launched agent's flight on every orientation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tap launch --agent NAME` resolved the agent's flight once and exported the result as TAP_FLIGHT. Environment variables outrank project and user config, and a process cannot change its own environment, so the launched session was pinned to whatever the flight was at launch. Editing the agent's flight and calling `orient` again did nothing, and the session reported success while staying on the old flight. Export the reference instead of the value: TAP_AGENT names the agent, and resolution reads agents[TAP_AGENT].flight out of the merged config on every load. The variable is still constant — the agent driving a process genuinely cannot change while it runs — but the mapping it points into is re-read at each reload boundary, so a config edit plus `orient` now moves a running session. Resolution happens once, at the end of ConfigService.load, rather than in ActiveFlightName. load is what Reload re-runs, so late binding falls out for free, and every consumer — CLI, MCP orientation, keg listing, config explain — reads one already-resolved Flight() with no call-site changes. It needs the env layer in isolation to tell a direct TAP_FLIGHT (which still wins) from a flight that came from a file layer (which the agent overrides). Precedence is --flight, TAP_FLIGHT, the agent's flight, project config, then the user baseline: direct beats indirect, and naming an agent at launch beats an ambient project default. A TAP_AGENT naming an agent that is not configured warns and falls through rather than failing, since a session cannot fix its own environment. The orientation payload names the active agent so a reader is told where the flight came from instead of being pointed at a `flight:` key the agent silently outranks. --- CLAUDE.md | 2 +- docs/configuration/flights.md | 20 ++- docs/configuration/resolution-order.md | 4 +- docs/configuration/user-config.md | 5 +- integrations/rendered/codex/tapper/.mcp.json | 1 + pkg/cli/cmd_launch.go | 11 +- pkg/cli/cmd_launch_test.go | 6 +- pkg/integrations/adapters/codex.go | 9 +- pkg/integrations/adapters/codex_test.go | 8 +- pkg/mcp/providers.go | 2 +- pkg/mcp/session_agent_flight_test.go | 113 +++++++++++++ pkg/mcp/session_flight.go | 5 +- pkg/mcp/session_transition_test.go | 4 +- pkg/parity/parity_coverage_test.go | 1 + pkg/tapper/config.go | 26 ++- pkg/tapper/config_agent_flight_test.go | 162 +++++++++++++++++++ pkg/tapper/config_env.go | 4 + pkg/tapper/config_service.go | 57 ++++++- pkg/tapper/tap_config.go | 32 ++++ pkg/tapper/tap_launch.go | 16 +- pkg/tapper/tap_launch_test.go | 16 +- pkg/tapper/tap_orient.go | 52 +++++- 22 files changed, 515 insertions(+), 41 deletions(-) create mode 100644 pkg/mcp/session_agent_flight_test.go create mode 100644 pkg/tapper/config_agent_flight_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 6335177..fc3ba69 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -243,7 +243,7 @@ flight; bare `tap use` prints the resolved keg/flight/fallback and the scope that set each. A persisted `flight` auto-applies when `--flight` is omitted. Supported env vars: `TAP_DEFAULT_KEG`, `TAP_FALLBACK_KEG`, `TAP_FLIGHT`, -`TAP_LOG_FILE`, `TAP_LOG_LEVEL`, `TAP_DEFAULT_HUB`, `TAP_FALLBACK_HUB`, +`TAP_AGENT`, `TAP_LOG_FILE`, `TAP_LOG_LEVEL`, `TAP_DEFAULT_HUB`, `TAP_FALLBACK_HUB`, `TAP_DEFAULT_NAMESPACE`, `TAP_FALLBACK_NAMESPACE`, `TAP_DISABLE_ATLAS_HUB`, `TAP_DISABLE_LOCAL_HUB`, `TAP_DISABLE_TELEMETRY` (`1`/`true`/`yes`/`on` for the disable flags). diff --git a/docs/configuration/flights.md b/docs/configuration/flights.md index a703a2a..d9e0fe8 100644 --- a/docs/configuration/flights.md +++ b/docs/configuration/flights.md @@ -169,7 +169,8 @@ capability. stale authority. - Hosted `/mcp` selects the account-wide MCP flight preference. Local initialization and `orient` select explicit `--flight`, then `TAP_FLIGHT`, - the nearest project config, and finally the user baseline. + then the active agent's `flight`, then the nearest project config, and finally + the user baseline. - Hosted self-deletion clears the account preference through the flight foreign key. A local config that still names a deleted flight remains a stale external reference: later `orient` reports it and the session stays in recovery until @@ -185,9 +186,20 @@ capability. `.tapper/config.yaml`; `tap use +slug` uses the resolved default namespace. Config-driven sessions adopt it on their next orientation. - Flight selection precedence is explicit runtime `--flight`, then - `TAP_FLIGHT`, then the nearest project config, then the user baseline written - by `tap bootstrap`. Project selection therefore overrides the machine-wide - bootstrap choice without changing it. + `TAP_FLIGHT`, then the active agent's `flight`, then the nearest project + config, then the user baseline written by `tap bootstrap`. Project selection + therefore overrides the machine-wide bootstrap choice without changing it. +- `tap launch --agent NAME` exports `TAP_AGENT=NAME`, not the flight that agent + currently names. The launched session resolves `agents[NAME].flight` on every + orientation, so editing that agent's flight and calling `orient` again moves + the running session. A resolved flight in the environment could not be + changed after launch, since a process cannot alter its own environment. + `TAP_FLIGHT` and `--flight` are direct and still outrank the agent, so either + one pins a launched session to a flight of its own. +- A `TAP_AGENT` naming an agent that is not configured is reported as a warning + in the orientation payload and the flight falls back to project and user + configuration. It is not fatal: a stale agent name is not something a session + can fix from the inside. - MCP tools have no model-visible `flight` input. Humans change config-driven selection with `tap use --flight @namespace/+slug` (or `tap use +slug`), then the existing session calls `orient`. There is no hidden flight-switch tool. diff --git a/docs/configuration/resolution-order.md b/docs/configuration/resolution-order.md index e88551e..395b7c7 100644 --- a/docs/configuration/resolution-order.md +++ b/docs/configuration/resolution-order.md @@ -16,8 +16,8 @@ If you pass explicit flags, they take precedence: instructions plus cover caps enforced by the MCP surface. Direct CLI commands still use normal keg authorization and do not have access reduced by the flight. Its precedence follows the config cascade: explicit `--flight`, `TAP_FLIGHT`, -the nearest project `flight`, then the user baseline optionally written by -`tap bootstrap`. See [Flights](flights.md). +the active agent's `flight`, the nearest project `flight`, then the user +baseline optionally written by `tap bootstrap`. See [Flights](flights.md). The local creation flags on `tap keg create` (`--project`, `--cwd`, and `--path`) only choose where a new filesystem keg is created. They are not diff --git a/docs/configuration/user-config.md b/docs/configuration/user-config.md index 73e16dc..0475c6f 100644 --- a/docs/configuration/user-config.md +++ b/docs/configuration/user-config.md @@ -168,8 +168,9 @@ preselected when available, and **Skip for now** leaves the current value unchanged. For scripts, pass the inherited global flag explicitly, for example `tap bootstrap --kind local --flight @local/+focused`; bootstrap validates the flight and stores its canonical `@namespace/+slug` reference. If no baseline is -set, MCP starts in recovery-only mode. A project `flight`, `TAP_FLIGHT`, or an -explicit `--flight` on a later command overrides the bootstrap baseline. +set, MCP starts in recovery-only mode. A project `flight`, the active agent's +`flight`, `TAP_FLIGHT`, or an explicit `--flight` on a later command overrides +the bootstrap baseline. ## Hub Resolution Chain diff --git a/integrations/rendered/codex/tapper/.mcp.json b/integrations/rendered/codex/tapper/.mcp.json index b46474e..1ebbec3 100644 --- a/integrations/rendered/codex/tapper/.mcp.json +++ b/integrations/rendered/codex/tapper/.mcp.json @@ -5,6 +5,7 @@ "args": ["mcp"], "env_vars": [ "HOME", + "TAP_AGENT", "TAP_FLIGHT", "XDG_CONFIG_HOME", "XDG_DATA_HOME", diff --git a/pkg/cli/cmd_launch.go b/pkg/cli/cmd_launch.go index 2b9d3a7..46594d3 100644 --- a/pkg/cli/cmd_launch.go +++ b/pkg/cli/cmd_launch.go @@ -36,8 +36,10 @@ An agent is an alias for a (model, flight) pair: flight: "@me/+scratch" Models are provider-qualified so the launcher knows which protocol the harness -must speak. The agent's flight is exported as TAP_FLIGHT, so a tap mcp session -started inside the harness orients to it. +must speak. The agent name is exported as TAP_AGENT, so a tap mcp session +started inside the harness resolves the agent's flight for itself. Editing the +agent's flight and calling orient again therefore moves a running session, +which exporting the resolved flight would not. Arguments after -- are passed through to the harness. @@ -69,7 +71,10 @@ Experimental and unstable: expect this to change or disappear.`, return err } if result.Flight != "" { - if _, err := fmt.Fprintf(out, "flight: %s\n", result.Flight); err != nil { + // "resolves to" rather than "is": the child re-resolves this + // from TAP_AGENT on every orient, so it can change under a + // running session. + if _, err := fmt.Fprintf(out, "flight: %s (resolves to, via agent)\n", result.Flight); err != nil { return err } } diff --git a/pkg/cli/cmd_launch_test.go b/pkg/cli/cmd_launch_test.go index 0773871..c153214 100644 --- a/pkg/cli/cmd_launch_test.go +++ b/pkg/cli/cmd_launch_test.go @@ -47,7 +47,8 @@ func TestLaunchCommand_DryRunResolvesOllamaThroughOpenAI(t *testing.T) { require.Contains(t, out, "codex --oss --local-provider ollama --model qwen3.6:35b-mlx") require.Contains(t, out, "CODEX_OSS_BASE_URL=http://localhost:11434/v1") - require.Contains(t, out, "TAP_FLIGHT=@testuser/+scratch") + require.Contains(t, out, "TAP_AGENT=local") + require.NotContains(t, out, "TAP_FLIGHT=") } func TestLaunchCommand_DryRunResolvesAnthropicThroughEnv(t *testing.T) { @@ -60,7 +61,8 @@ func TestLaunchCommand_DryRunResolvesAnthropicThroughEnv(t *testing.T) { out := string(res.Stdout) require.Contains(t, out, "ANTHROPIC_MODEL=claude-opus-4") - require.Contains(t, out, "TAP_FLIGHT=+dev") + require.Contains(t, out, "TAP_AGENT=opus") + require.NotContains(t, out, "TAP_FLIGHT=") } func TestLaunchCommand_DryRunPassesThroughExtraArgs(t *testing.T) { diff --git a/pkg/integrations/adapters/codex.go b/pkg/integrations/adapters/codex.go index 3442323..1ade0cf 100644 --- a/pkg/integrations/adapters/codex.go +++ b/pkg/integrations/adapters/codex.go @@ -203,9 +203,11 @@ func renderCodexMarketplace() ([]byte, error) { // elsewhere tap mcp fails to authenticate while the same tap in the shell // succeeds, which is precisely how this surfaced in a dev container. // -// TAP_FLIGHT carries `tap launch --agent` flight selection. Without it the -// harness has the flight but the MCP server it spawns does not, so the session -// silently resolves the configured flight instead of the requested one. +// TAP_AGENT carries `tap launch --agent` selection. Without it the harness has +// the agent but the MCP server it spawns does not, so the session silently +// resolves the configured flight instead of the agent's. TAP_FLIGHT stays +// listed because a human may still export it directly to override; the launcher +// itself no longer sets it. func renderCodexMCP() []byte { return []byte(`{ "mcpServers": { @@ -214,6 +216,7 @@ func renderCodexMCP() []byte { "args": ["mcp"], "env_vars": [ "HOME", + "TAP_AGENT", "TAP_FLIGHT", "XDG_CONFIG_HOME", "XDG_DATA_HOME", diff --git a/pkg/integrations/adapters/codex_test.go b/pkg/integrations/adapters/codex_test.go index 3196a4c..c2154bc 100644 --- a/pkg/integrations/adapters/codex_test.go +++ b/pkg/integrations/adapters/codex_test.go @@ -116,9 +116,11 @@ func TestCodexAdapter_RendersNativeMarketplaceAndTwoPlugins(t *testing.T) { } // HOME must be forwarded alongside the XDG roots: tap falls back to it when a // root is unset and when expanding "~", so without it tap mcp fails to - // authenticate under Codex while the same tap works in the shell. TAP_FLIGHT - // carries `tap launch --agent` flight selection through to the server. - wantEnvVars := "HOME,TAP_FLIGHT,XDG_CONFIG_HOME,XDG_DATA_HOME,XDG_STATE_HOME,XDG_CACHE_HOME" + // authenticate under Codex while the same tap works in the shell. TAP_AGENT + // carries `tap launch --agent` selection through to the server, which + // resolves the agent's flight itself; TAP_FLIGHT remains forwarded for a + // human overriding it directly. + wantEnvVars := "HOME,TAP_AGENT,TAP_FLIGHT,XDG_CONFIG_HOME,XDG_DATA_HOME,XDG_STATE_HOME,XDG_CACHE_HOME" if got := strings.Join(tapperMCP.EnvVars, ","); got != wantEnvVars { t.Errorf("tapper MCP env_vars = %q, want %q", got, wantEnvVars) } diff --git a/pkg/mcp/providers.go b/pkg/mcp/providers.go index baf162b..4b101cd 100644 --- a/pkg/mcp/providers.go +++ b/pkg/mcp/providers.go @@ -141,7 +141,7 @@ func (p *localOrientationProvider) Load(ctx context.Context) (*Orientation, erro if listErr == nil && len(flights) == 0 { return p.Render(ctx, tapper.BootstrapFlight("", localBootstrapInstructions(warnings))) } - payload, payloadErr := tapper.BuildOrientationPayload(nil, "", nil, warnings) + payload, payloadErr := tapper.BuildOrientationPayload(nil, "", p.tap.ActiveAgentName(), nil, warnings) return &Orientation{Payload: payload, Warnings: warnings}, payloadErr } flight, err := p.tap.FlightService.GetFlightFresh(ctx, ref) diff --git a/pkg/mcp/session_agent_flight_test.go b/pkg/mcp/session_agent_flight_test.go new file mode 100644 index 0000000..238527e --- /dev/null +++ b/pkg/mcp/session_agent_flight_test.go @@ -0,0 +1,113 @@ +package mcp_test + +import ( + "context" + "testing" + + "github.com/jlrickert/cli-toolkit/toolkit" + sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/require" + + "github.com/jlrickert/tapper/pkg/mcp" + "github.com/jlrickert/tapper/pkg/tapper" +) + +// TestMCP_AgentFlightMovesWithConfig is the regression this whole mechanism +// exists for. `tap launch` used to export the agent's flight as TAP_FLIGHT, so +// a running session could never leave it: env outranks project and user config, +// and a process cannot change its own environment. Editing the agent's flight +// and re-orienting silently did nothing, while the session reported success. +// +// Exporting TAP_AGENT instead makes the flight a reference resolved on every +// orientation, so the edit lands. +func TestMCP_AgentFlightMovesWithConfig(t *testing.T) { + ctx, srv, rt := newAgentOrientationServer(t, "qwen") + session := connectFlightSession(t, ctx, srv, nil) + + require.Contains(t, session.InitializeResult().Instructions, "+alpha") + require.Contains(t, session.InitializeResult().Instructions, "Alpha instructions") + + writeAgentFlight(t, rt, "qwen", "beta") + + oriented := callOrient(t, ctx, session) + require.Contains(t, oriented, "+beta") + require.Contains(t, oriented, "Beta instructions") + require.NotContains(t, oriented, "Alpha instructions") +} + +// The payload names the agent, so a reader who wants a different flight is told +// where the current one came from instead of being pointed at a `flight:` key +// the agent silently outranks. +func TestMCP_AgentIsNamedInTheOrientationPayload(t *testing.T) { + ctx, srv, _ := newAgentOrientationServer(t, "qwen") + session := connectFlightSession(t, ctx, srv, nil) + + oriented := callOrient(t, ctx, session) + require.Contains(t, oriented, "agent `qwen`") + require.Contains(t, oriented, "call `orient` again") +} + +// A direct TAP_FLIGHT still wins, which is the escape hatch for overriding a +// launched session without touching config. +func TestMCP_TapFlightOverridesTheAgentInSession(t *testing.T) { + ctx, srv, _ := newAgentOrientationServerWithEnv(t, map[string]string{ + "TAP_AGENT": "qwen", + "TAP_FLIGHT": "+baseline", + }) + session := connectFlightSession(t, ctx, srv, nil) + + require.Contains(t, callOrient(t, ctx, session), "+baseline") +} + +// A stale agent name is reported in the payload rather than locking the +// session: the agent cannot edit its own environment to fix it. +func TestMCP_UnknownAgentWarnsButKeepsTheSessionUsable(t *testing.T) { + ctx, srv, _ := newAgentOrientationServer(t, "ghost") + session := connectFlightSession(t, ctx, srv, nil) + + oriented := callOrient(t, ctx, session) + require.Contains(t, oriented, `agent "ghost"`) + require.Contains(t, oriented, "not configured") + // The user baseline still governs, so KEG tools stay available. + require.Contains(t, oriented, "+baseline") + require.False(t, callCat(t, ctx, session).IsError) +} + +func newAgentOrientationServer(t *testing.T, agent string) (context.Context, *sdkmcp.Server, *toolkit.Runtime) { + t.Helper() + return newAgentOrientationServerWithEnv(t, map[string]string{"TAP_AGENT": agent}) +} + +// newAgentOrientationServer builds a config-driven session (no static flight) +// whose flight comes from an agent, mirroring what `tap launch` produces. +func newAgentOrientationServerWithEnv(t *testing.T, env map[string]string) (context.Context, *sdkmcp.Server, *toolkit.Runtime) { + t.Helper() + ctx := context.Background() + sb := newTestSandbox(t) + require.NoError(t, sb.Setwd("/home/testuser/project")) + rt := sb.Runtime() + for k, v := range env { + require.NoError(t, rt.Env().Set(k, v)) + } + writeFlight(t, rt, "baseline", "Baseline instructions") + writeFlight(t, rt, "alpha", "Alpha instructions") + writeFlight(t, rt, "beta", "Beta instructions") + // The user baseline is what an unknown or flightless agent falls back to. + writeAgentFlight(t, rt, "qwen", "alpha") + + tap, err := tapper.NewTap(tapper.TapOptions{Runtime: rt}) + require.NoError(t, err) + srv := mcp.NewServer(tap, "test", mcp.KegDefaults{}) + return ctx, srv, rt +} + +// writeAgentFlight rewrites the user config so agent `name` points at +slug, +// keeping the baseline `flight:` underneath it to prove the agent outranks it. +func writeAgentFlight(t *testing.T, rt *toolkit.Runtime, name, slug string) { + t.Helper() + body := "defaultKeg: personal\nfallbackNamespace: local\n" + + "hubs:\n home:\n kind: local\n basePath: ~/kegs\n" + + "flight: +baseline\n" + + "agents:\n " + name + ":\n model: ollama/qwen3.6:35b\n flight: +" + slug + "\n" + require.NoError(t, rt.AtomicWriteFile("/home/testuser/.config/tapper/config.yaml", []byte(body), 0o644)) +} diff --git a/pkg/mcp/session_flight.go b/pkg/mcp/session_flight.go index 66168e1..4d85c85 100644 --- a/pkg/mcp/session_flight.go +++ b/pkg/mcp/session_flight.go @@ -332,7 +332,10 @@ func (g *sessionFlightGate) adoptDeletedFlight(ctx context.Context, target strin } g.calls.Lock() defer g.calls.Unlock() - payload, payloadErr := tapper.BuildOrientationPayload(nil, "", nil, nil) + // No agent name: this is the self-deletion path, where the flight the + // session was running on has just been removed. The gate has no Tap to ask, + // and "your flight is gone" is the whole message. + payload, payloadErr := tapper.BuildOrientationPayload(nil, "", "", nil, nil) if payloadErr != nil { payload = errMCPFlightRequired.Error() } diff --git a/pkg/mcp/session_transition_test.go b/pkg/mcp/session_transition_test.go index 3ff4ac4..6a71386 100644 --- a/pkg/mcp/session_transition_test.go +++ b/pkg/mcp/session_transition_test.go @@ -65,7 +65,7 @@ func (p *fakeSessionBackend) Load(ctx context.Context) (*mcp.Orientation, error) flight := copyTransitionFlight(p.flights[p.active]) p.mu.Unlock() if flight == nil { - payload, err := tapper.BuildOrientationPayload(nil, "", nil, nil) + payload, err := tapper.BuildOrientationPayload(nil, "", "", nil, nil) return &mcp.Orientation{Payload: payload}, err } return p.Render(ctx, flight) @@ -81,7 +81,7 @@ func (p *fakeSessionBackend) Render(_ context.Context, flight *tapper.Flight) (* kegs := []tapper.OrientationKeg{{ Ref: "@local/personal", Namespace: "local", Alias: "personal", Title: "Personal", Role: "admin", Source: "local", FlightCap: "editor", }, {Ref: "@local/other", Namespace: "local", Alias: "other", Title: "Other", Role: "admin", Source: "local", FlightCap: "editor"}} - payload, err := tapper.BuildOrientationPayload(flight, "", kegs, nil) + payload, err := tapper.BuildOrientationPayload(flight, "", "", kegs, nil) if err != nil { return nil, err } diff --git a/pkg/parity/parity_coverage_test.go b/pkg/parity/parity_coverage_test.go index 4bac65f..8c33873 100644 --- a/pkg/parity/parity_coverage_test.go +++ b/pkg/parity/parity_coverage_test.go @@ -146,6 +146,7 @@ var tapMethodsExcluded = map[string]string{ "Use": "writes the project/user keg + flight to config; CLI-only config management by design", "UseStatus": "CLI-only summary of the resolved keg/flight context; config inspection via `tap use`", "ActiveFlightName": "internal pure read of the explicit flight or the loaded cascade's selection; backs Orient and MCP session adoption rather than being an operation of its own", + "ActiveAgentName": "internal pure read of the `tap launch` agent driving the process; reported in orientation and telemetry rather than being an operation of its own", "OrientationForFlight": "internal session-orientation builder used by initialize, orient, and the orient resource", // Dropped from MCP when the surface was unified behind providers: these // operate on machine-local Tapper state or perform tenant administration, diff --git a/pkg/tapper/config.go b/pkg/tapper/config.go index a4d0e5a..09893cc 100644 --- a/pkg/tapper/config.go +++ b/pkg/tapper/config.go @@ -77,9 +77,18 @@ type configDTO struct { // flight is the flight context applied when no --flight flag is given. It is // a flight reference (@namespace/+slug, +slug, or a bare slug) and is // may be set as a user baseline by bootstrap or overridden in project config; - // TAP_FLIGHT and --flight have higher precedence. + // TAP_FLIGHT, the active agent's flight, and --flight have higher precedence. Flight string `yaml:"flight,omitempty"` + // agent names the entry in agents{} driving this process, and is set by + // `tap launch` as TAP_AGENT. It selects a flight indirectly: resolution reads + // agents[agent].flight out of the merged config on every load, so an edit to + // the agent's flight is picked up by the next reload. Exporting the resolved + // flight instead would freeze it for the life of the process, which is + // precisely the bug this field exists to avoid. TAP_FLIGHT and --flight, + // being direct, still outrank it. + Agent string `yaml:"agent,omitempty"` + // kegMap maps a project path or pattern to a keg reference. KegMap []KegMapEntry `yaml:"kegMap"` @@ -345,7 +354,8 @@ func (cfg *Config) FallbackKeg() string { } // Flight returns the persisted flight reference applied when no --flight flag -// is given. +// is given. On a merged config this may have come from the active agent rather +// than from any file — see ConfigService.load. func (cfg *Config) Flight() string { if cfg.data == nil { cfg.data = &configDTO{} @@ -353,6 +363,15 @@ func (cfg *Config) Flight() string { return cfg.data.Flight } +// AgentName returns the name of the agent driving this process, or "" when none +// is selected. It indexes Agents; it is not itself an agent definition. +func (cfg *Config) AgentName() string { + if cfg.data == nil { + cfg.data = &configDTO{} + } + return strings.TrimSpace(cfg.data.Agent) +} + // LookupAliasForTarget previously reverse-mapped a resolved target back to its // configured keg alias. The namespace-centric model has no alias table, so a // target no longer carries a short alias; callers fall back to the canonical @@ -1319,6 +1338,9 @@ func MergeConfig(cfgs ...*Config) *Config { if c.data.Flight != "" { out.data.Flight = c.data.Flight } + if c.data.Agent != "" { + out.data.Agent = c.data.Agent + } if c.data.LogFile != "" { out.data.LogFile = c.data.LogFile } diff --git a/pkg/tapper/config_agent_flight_test.go b/pkg/tapper/config_agent_flight_test.go new file mode 100644 index 0000000..19428fc --- /dev/null +++ b/pkg/tapper/config_agent_flight_test.go @@ -0,0 +1,162 @@ +package tapper_test + +import ( + "testing" + + "github.com/jlrickert/cli-toolkit/sandbox" + "github.com/stretchr/testify/require" + + "github.com/jlrickert/tapper/pkg/tapper" +) + +// agentFlightConfig is a user config carrying two agents: one with a flight and +// one without, so the fall-through case is expressible without a second fixture. +const agentFlightConfig = `fallbackNamespace: local +flight: +user +agents: + qwen: + model: ollama/qwen3.6:35b + flight: +test + flightless: + model: openai/gpt-5 +` + +// newAgentFlightTap builds a Tap whose user config selects flights and agents, +// with an optional project config and environment overlay on top. +func newAgentFlightTap(t *testing.T, projectConfig string, env map[string]string) (*tapper.Tap, *sandbox.Sandbox) { + t.Helper() + opts := []sandbox.Option{} + for k, v := range env { + opts = append(opts, sandbox.WithEnv(k, v)) + } + sb := sandbox.NewSandbox(t, &sandbox.Options{Home: "/home/testuser", User: "testuser"}, opts...) + require.NoError(t, sb.Setwd("/home/testuser/work/project")) + require.NoError(t, sb.Runtime().AtomicWriteFile( + "/home/testuser/.config/tapper/config.yaml", []byte(agentFlightConfig), 0o644)) + if projectConfig != "" { + require.NoError(t, sb.Runtime().AtomicWriteFile( + "/home/testuser/work/project/.tapper/config.yaml", []byte(projectConfig), 0o644)) + } + tap, err := tapper.NewTap(tapper.TapOptions{Runtime: sb.Runtime()}) + require.NoError(t, err) + return tap, sb +} + +func TestAgentFlight_TapAgentSelectsTheAgentsFlight(t *testing.T) { + t.Parallel() + tap, _ := newAgentFlightTap(t, "", map[string]string{"TAP_AGENT": "qwen"}) + + cfg, err := tap.ConfigService.Config() + require.NoError(t, err) + require.Equal(t, "qwen", cfg.AgentName()) + require.Equal(t, "+test", cfg.Flight(), + "the agent's flight must win over the user baseline") +} + +// TAP_FLIGHT is a direct value and the agent only a reference to one, so the +// direct value wins. This is the escape hatch that lets a human override a +// launched session without editing config. +func TestAgentFlight_TapFlightOutranksTheAgent(t *testing.T) { + t.Parallel() + tap, _ := newAgentFlightTap(t, "", map[string]string{ + "TAP_AGENT": "qwen", + "TAP_FLIGHT": "+debug", + }) + + cfg, err := tap.ConfigService.Config() + require.NoError(t, err) + require.Equal(t, "+debug", cfg.Flight()) +} + +// Naming an agent at launch is deliberate, so it outranks an ambient project +// default. This preserves what `tap launch` did when it exported TAP_FLIGHT. +func TestAgentFlight_AgentOutranksProjectConfig(t *testing.T) { + t.Parallel() + tap, _ := newAgentFlightTap(t, "flight: +proj\n", map[string]string{"TAP_AGENT": "qwen"}) + + cfg, err := tap.ConfigService.Config() + require.NoError(t, err) + require.Equal(t, "+test", cfg.Flight()) +} + +func TestAgentFlight_NoAgentLeavesTheCascadeAlone(t *testing.T) { + t.Parallel() + tap, _ := newAgentFlightTap(t, "flight: +proj\n", nil) + + cfg, err := tap.ConfigService.Config() + require.NoError(t, err) + require.Equal(t, "+proj", cfg.Flight()) + require.Empty(t, cfg.AgentName()) +} + +// An agent with no flight contributes nothing rather than clearing the +// selection, matching a launch that had no flight to export. +func TestAgentFlight_AgentWithoutFlightFallsThrough(t *testing.T) { + t.Parallel() + tap, _ := newAgentFlightTap(t, "flight: +proj\n", map[string]string{"TAP_AGENT": "flightless"}) + + cfg, warnings, err := tap.ConfigService.Load() + require.NoError(t, err) + require.Equal(t, "+proj", cfg.Flight()) + require.Empty(t, warnings, "an agent may legitimately carry no flight") +} + +// A stale TAP_AGENT is reported, not fatal: the session cannot fix its own +// environment, and failing hard would brick a harness over a typo. +func TestAgentFlight_UnknownAgentWarnsAndFallsThrough(t *testing.T) { + t.Parallel() + tap, _ := newAgentFlightTap(t, "flight: +proj\n", map[string]string{"TAP_AGENT": "ghost"}) + + cfg, warnings, err := tap.ConfigService.Load() + require.NoError(t, err) + require.Equal(t, "+proj", cfg.Flight()) + + require.Len(t, warnings, 1) + require.Equal(t, "agent", warnings[0].Source) + require.Contains(t, warnings[0].Message, `"ghost"`) +} + +// The regression this whole mechanism exists for: a running process must see an +// edited agent flight after a reload. Exporting a resolved TAP_FLIGHT could not +// do this, because a process cannot change its own environment. +func TestAgentFlight_ReloadPicksUpAnEditedAgentFlight(t *testing.T) { + t.Parallel() + tap, sb := newAgentFlightTap(t, "", map[string]string{"TAP_AGENT": "qwen"}) + + cfg, err := tap.ConfigService.Config() + require.NoError(t, err) + require.Equal(t, "+test", cfg.Flight()) + + require.NoError(t, sb.Runtime().AtomicWriteFile( + "/home/testuser/.config/tapper/config.yaml", + []byte(`fallbackNamespace: local +flight: +user +agents: + qwen: + model: ollama/qwen3.6:35b + flight: +admin +`), 0o644)) + + // Still the old value: configuration is fixed until something reloads. + cfg, err = tap.ConfigService.Config() + require.NoError(t, err) + require.Equal(t, "+test", cfg.Flight()) + + tap.ConfigService.Reload() + cfg, err = tap.ConfigService.Config() + require.NoError(t, err) + require.Equal(t, "+admin", cfg.Flight(), + "a reload must re-resolve the agent's flight, not reuse the launch-time value") +} + +func TestAgentFlight_ExplainCreditsTheAgent(t *testing.T) { + t.Parallel() + tap, _ := newAgentFlightTap(t, "flight: +proj\n", map[string]string{"TAP_AGENT": "qwen"}) + + results, err := tap.ConfigExplain(t.Context(), tapper.ConfigExplainOptions{Field: "flight"}) + require.NoError(t, err) + require.Len(t, results, 1) + require.Equal(t, "+test", results[0].Value) + require.Equal(t, `agent "qwen"`, results[0].Source, + "explain must name the agent rather than the project config it overrode") +} diff --git a/pkg/tapper/config_env.go b/pkg/tapper/config_env.go index f83c284..58cfd4e 100644 --- a/pkg/tapper/config_env.go +++ b/pkg/tapper/config_env.go @@ -10,6 +10,7 @@ var tapEnvVarKeys = []string{ "DEFAULT_KEG", "FALLBACK_KEG", "FLIGHT", + "AGENT", "LOG_FILE", "LOG_LEVEL", "DEFAULT_HUB", @@ -41,6 +42,9 @@ func configFromEnvMap(envMap map[string]string) *Config { if v, ok := envMap["flight"]; ok { cfg.data.Flight = v } + if v, ok := envMap["agent"]; ok { + cfg.data.Agent = v + } if v, ok := envMap["log_file"]; ok { cfg.data.LogFile = v } diff --git a/pkg/tapper/config_service.go b/pkg/tapper/config_service.go index cf448d2..3565ba5 100644 --- a/pkg/tapper/config_service.go +++ b/pkg/tapper/config_service.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "sync" "github.com/jlrickert/cli-toolkit/cfgcascade" @@ -66,7 +67,12 @@ type resolved struct { userErr error project *Config projectErr error - warnings []ConfigLoadWarning + // env is the env-var layer in isolation. The merged config cannot answer + // "did TAP_FLIGHT set this?", and agent resolution has to know: a direct + // TAP_FLIGHT outranks the flight an agent points at, while a flight coming + // from a file layer does not. + env *Config + warnings []ConfigLoadWarning } // NewConfigService builds a ConfigService rooted at root. @@ -329,6 +335,7 @@ func (s *ConfigService) load() (*resolved, error) { return nil, err } cfg := configFromEnvMap(envMap) + out.env = cfg if cfg == nil { return nil, os.ErrNotExist } @@ -369,9 +376,57 @@ func (s *ConfigService) load() (*resolved, error) { if out.merged == nil { out.merged = &Config{data: &configDTO{}} } + if warning := applyAgentFlight(out.merged, out.env); warning != nil { + out.warnings = append(out.warnings, *warning) + } return out, nil } +// applyAgentFlight resolves the active agent's flight into merged, and is why +// `tap launch` can export an agent name instead of a resolved flight. The agent +// is a reference, so the lookup happens on every load; a flight baked into the +// environment at launch would instead be frozen for the life of the process and +// no amount of reloading could move it. +// +// It sits between the env and project layers of the cascade rather than inside +// it, because the cascade merges whole Configs by rank and this rule needs two +// layers at once: the agents map comes from the file layers, while the decision +// to apply it at all depends on the env layer. Running here also means every +// consumer of ConfigService.Config sees one already-resolved flight. +// +// A returned warning means the selection named an agent that is not configured. +// That is reported rather than fatal: the session is still usable on whatever +// the file layers select, and a hard failure over a stale TAP_AGENT would brick +// a harness for a typo it cannot fix from the inside. +func applyAgentFlight(merged, env *Config) *ConfigLoadWarning { + if merged == nil { + return nil + } + name := merged.AgentName() + if name == "" { + return nil + } + // A direct TAP_FLIGHT outranks the agent's indirect one, so leave it be. + if env != nil && strings.TrimSpace(env.Flight()) != "" { + return nil + } + entry, ok := merged.Agent(name) + if !ok { + return &ConfigLoadWarning{ + Source: "agent", + Message: fmt.Sprintf( + "agent %q is selected but not configured, so its flight could not be applied; "+ + "the flight falls back to project and user configuration", name), + } + } + // An agent without a flight selects no flight, matching a launch that had + // none to export. + if flight := strings.TrimSpace(entry.Flight); flight != "" { + _ = merged.SetFlight(flight) + } + return nil +} + // ResolveTarget resolves a keg selector to a keg target. When the selector is // empty it uses defaultKeg, then fallbackKeg. The selector is parsed as a keg // reference and turned into a concrete target by Config.ResolveAlias (the diff --git a/pkg/tapper/tap_config.go b/pkg/tapper/tap_config.go index 7225623..d141633 100644 --- a/pkg/tapper/tap_config.go +++ b/pkg/tapper/tap_config.go @@ -216,6 +216,7 @@ var ConfigExplainFields = []string{ "defaultKeg", "fallbackKeg", "flight", + "agent", "logFile", "logLevel", "defaultHub", @@ -242,6 +243,8 @@ func configFieldGetter(cfg *Config, field string) string { return cfg.FallbackKeg() case "flight": return cfg.Flight() + case "agent": + return cfg.AgentName() case "logFile": return cfg.LogFile() case "logLevel": @@ -311,9 +314,14 @@ func (t *Tap) ConfigExplain(ctx context.Context, opts ConfigExplainOptions) ([]C mergedVal := configFieldGetter(merged, field) // Walk from most-specific to least-specific to find which source set this value. + // The agent sits between the env and file layers for flight only, mirroring + // the resolution order in ConfigService.load — otherwise this would report a + // project config that the agent's flight actually overrode. source := "default" if envVal := configFieldGetter(envCfg, field); envVal != "" { source = "env vars" + } else if agentName := agentFlightSource(merged, field); agentName != "" { + source = fmt.Sprintf("agent %q", agentName) } else if projVal := configFieldGetter(projectCfg, field); projVal != "" { source = "project config" } else if userVal := configFieldGetter(userCfg, field); userVal != "" { @@ -331,6 +339,30 @@ func (t *Tap) ConfigExplain(ctx context.Context, opts ConfigExplainOptions) ([]C } // loadEnvConfig builds a Config from TAP_* env vars, or returns nil if none are set. +// agentFlightSource names the agent that supplied field's value, or "" when the +// agent did not. Only "flight" can come from an agent; the agent's own entry is +// a plain config value like any other. +func agentFlightSource(merged *Config, field string) string { + if field != "flight" || merged == nil { + return "" + } + name := merged.AgentName() + if name == "" { + return "" + } + entry, ok := merged.Agent(name) + if !ok { + return "" + } + // Compare against the merged value rather than assuming the overlay ran: a + // TAP_FLIGHT override is caught by the env branch before this one, but an + // agent whose flight is empty never contributed and must not be credited. + if flight := strings.TrimSpace(entry.Flight); flight != "" && flight == merged.Flight() { + return name + } + return "" +} + func (t *Tap) loadEnvConfig() *Config { getenv := t.Runtime.Env().Get envMap := make(map[string]string) diff --git a/pkg/tapper/tap_launch.go b/pkg/tapper/tap_launch.go index 49cb7ab..78e34a3 100644 --- a/pkg/tapper/tap_launch.go +++ b/pkg/tapper/tap_launch.go @@ -70,6 +70,10 @@ type LaunchOptions struct { // applied on top of the inherited environment; StripEnv names variables removed // from it. Neither contains a secret value — a forwarded key is reported by the // variable it came from. +// +// Flight is what the agent points at right now, reported for the operator's +// benefit. It is not what gets exported: the child resolves the flight itself +// from TAP_AGENT, so this value can go stale the moment the config changes. type LaunchResult struct { Harness string Agent string @@ -349,11 +353,13 @@ func (t *Tap) ResolveLaunch(opts LaunchOptions) (*LaunchResult, error) { if env == nil { env = map[string]string{} } - // The launched process resolves its own flight through the normal chain, - // where TAP_FLIGHT outranks project and user config. No new plumbing. - if flight := strings.TrimSpace(agent.Flight); flight != "" { - env["TAP_FLIGHT"] = flight - } + // Export the agent, not the flight it currently resolves to. The launched + // process looks up agents[TAP_AGENT].flight on every config load, so editing + // the agent's flight and re-orienting moves a running session. Exporting + // TAP_FLIGHT here instead would pin the value into an environment that + // cannot be changed after exec, leaving the session stuck on whatever the + // flight was at launch no matter what the config later said. + env["TAP_AGENT"] = agentName // Subscription mode has to remove inherited credentials, which an overlay // cannot express: appending can override a variable but never unset one. diff --git a/pkg/tapper/tap_launch_test.go b/pkg/tapper/tap_launch_test.go index 5ce3885..0b5a7c8 100644 --- a/pkg/tapper/tap_launch_test.go +++ b/pkg/tapper/tap_launch_test.go @@ -97,7 +97,10 @@ func TestResolveLaunch_AnthropicOnClaude(t *testing.T) { // Claude Code takes its model through the environment, not a flag. require.Equal(t, []string{"claude"}, got.Argv) require.Equal(t, "claude-opus-4", got.Env["ANTHROPIC_MODEL"]) - require.Equal(t, "+dev", got.Env["TAP_FLIGHT"]) + // The agent, not the flight it currently names: the child re-resolves the + // flight on every load so a config edit can move a running session. + require.Equal(t, "opus", got.Env["TAP_AGENT"]) + require.NotContains(t, got.Env, "TAP_FLIGHT") } // Codex has first-class local-provider support and configures it through @@ -118,7 +121,8 @@ func TestResolveLaunch_OllamaOnCodexUsesOSSProvider(t *testing.T) { require.Equal(t, "http://localhost:11434/v1", got.Env["CODEX_OSS_BASE_URL"]) require.NotContains(t, got.Env, "OPENAI_BASE_URL") require.NotContains(t, got.Env, "OPENAI_API_KEY") - require.Equal(t, "@testuser/+scratch", got.Env["TAP_FLIGHT"]) + require.Equal(t, "local", got.Env["TAP_AGENT"]) + require.NotContains(t, got.Env, "TAP_FLIGHT") } func TestResolveLaunch_OpenAIOnCodexLeavesDefaultEndpoint(t *testing.T) { @@ -130,7 +134,9 @@ func TestResolveLaunch_OpenAIOnCodexLeavesDefaultEndpoint(t *testing.T) { require.Equal(t, []string{"codex", "--model", "gpt-5"}, got.Argv) require.NotContains(t, got.Env, "OPENAI_BASE_URL") - // An agent may omit its flight; nothing is exported in that case. + // An agent may omit its flight. The agent is still exported — resolution + // simply finds no flight on it and falls through to project/user config. + require.Equal(t, "hosted", got.Env["TAP_AGENT"]) require.NotContains(t, got.Env, "TAP_FLIGHT") } @@ -304,7 +310,9 @@ func TestResolveLaunch_ReadsAgentsFromProjectConfig(t *testing.T) { got, err := tap.ResolveLaunch(tapper.LaunchOptions{Harness: "codex", Agent: "proj"}) require.NoError(t, err) require.Equal(t, "gpt-5", got.Model) - require.Equal(t, "+proj", got.Env["TAP_FLIGHT"]) + require.Equal(t, "proj", got.Env["TAP_AGENT"]) + // Still reported, so a dry run can show what the agent currently points at. + require.Equal(t, "+proj", got.Flight) } // A context cap means the same thing to a user on either harness but is spelled diff --git a/pkg/tapper/tap_orient.go b/pkg/tapper/tap_orient.go index b0f5e32..60df38c 100644 --- a/pkg/tapper/tap_orient.go +++ b/pkg/tapper/tap_orient.go @@ -48,7 +48,7 @@ func (t *Tap) Orient(ctx context.Context, opts OrientOptions) (string, error) { flightName := t.ActiveFlightName(opts.Flight) flight, flightNote := t.resolveOrientFlight(ctx, flightName) available, warnings := t.orientKegListing(ctx, flight) - return BuildOrientationPayload(flight, flightNote, available, warnings) + return BuildOrientationPayload(flight, flightNote, t.ActiveAgentName(), available, warnings) } // OrientationForFlight builds orientation from an already-resolved immutable @@ -56,7 +56,7 @@ func (t *Tap) Orient(ctx context.Context, opts OrientOptions) (string, error) { // refresh policy first, then atomically publish the returned payload. func (t *Tap) OrientationForFlight(ctx context.Context, flight *Flight) (string, []OrientationKeg, []string, error) { available, warnings := t.orientKegListing(ctx, flight) - payload, err := BuildOrientationPayload(flight, "", available, warnings) + payload, err := BuildOrientationPayload(flight, "", t.ActiveAgentName(), available, warnings) return payload, available, warnings, err } @@ -79,6 +79,20 @@ func (t *Tap) ActiveFlightName(explicit string) string { return strings.TrimSpace(cfg.Flight()) } +// ActiveAgentName reports the agent driving this process, or "" when none is +// selected. Like ActiveFlightName it is a pure read of the current snapshot; +// the value only changes when a caller reloads. +func (t *Tap) ActiveAgentName() string { + if t == nil || t.ConfigService == nil { + return "" + } + cfg, err := t.ConfigService.Config() + if err != nil || cfg == nil { + return "" + } + return cfg.AgentName() +} + func (t *Tap) resolveOrientFlight(ctx context.Context, name string) (*Flight, string) { name = strings.TrimSpace(name) if name == "" || t == nil || t.FlightService == nil { @@ -108,12 +122,21 @@ func (t *Tap) orientKegListing(ctx context.Context, flight *Flight) ([]Orientati if t == nil || t.ConfigService == nil { return nil, []string{"KEG listing unavailable: no config service is configured."} } - cfg, err := t.ConfigService.Config() + cfg, loadWarnings, err := t.ConfigService.Load() if err != nil { return nil, []string{fmt.Sprintf("KEG listing unavailable: %v", err)} } var warnings []string + // Agent-selection warnings are the one config-load class that belongs in the + // payload: they explain why the session is on a different flight than the + // user expects, and the reader is the only one who can fix it. The rest stay + // out so orientation does not turn into a config linter. + for _, w := range loadWarnings { + if w.Source == "agent" { + warnings = append(warnings, w.Message) + } + } seen := map[string]struct{}{} var out []OrientationKeg for _, hubName := range t.allHubNames(cfg) { @@ -326,8 +349,10 @@ func kegRefLabel(target *keg.Target) string { } // BuildOrientationPayload renders the provider-neutral orientation document -// from one immutable flight snapshot and its effective KEG listing. -func BuildOrientationPayload(flight *Flight, flightNote string, kegs []OrientationKeg, warnings []string) (string, error) { +// from one immutable flight snapshot and its effective KEG listing. agent names +// the `tap launch` agent driving the session, or "" when a human is; it is +// reported because it explains where the flight came from and how to change it. +func BuildOrientationPayload(flight *Flight, flightNote, agent string, kegs []OrientationKeg, warnings []string) (string, error) { var b strings.Builder b.WriteString("# KEG System\n\n") b.WriteString(orientPurpose) @@ -394,6 +419,12 @@ func BuildOrientationPayload(flight *Flight, flightNote string, kegs []Orientati b.WriteString("2. Ask the user to select a flight in Tapper configuration. ") b.WriteString("Flights are selected outside MCP; an agent cannot select one itself.\n") b.WriteString("3. Call `orient` again on this same connection to pick it up.\n\n") + if agent != "" { + b.WriteString("This session was launched as agent `") + b.WriteString(agent) + b.WriteString("`, so giving that agent a `flight` in Tapper configuration ") + b.WriteString("is the most direct fix.\n\n") + } } if flight != nil { @@ -413,6 +444,17 @@ func BuildOrientationPayload(flight *Flight, flightNote string, kegs []Orientati b.WriteString(flight.Name) b.WriteString("`\n\n") } + if agent != "" { + // Naming the agent tells the reader where the flight came from and + // how to move it. Without this the flight looks like a fixed + // property of the session, and the user is told to edit `flight:` + // in config — which the agent's own flight silently outranks. + b.WriteString("This session is driven by agent `") + b.WriteString(agent) + b.WriteString("`. Unless `TAP_FLIGHT` or `--flight` overrides it, the flight above ") + b.WriteString("comes from that agent's `flight` in Tapper configuration: change it ") + b.WriteString("there and call `orient` again to move this session.\n\n") + } if flightNote != "" { b.WriteString(flightNote) b.WriteString("\n\n") From b1ccd764342abe13a347f921cbf24dc4905465f9 Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 6 Aug 2026 17:17:06 -0500 Subject: [PATCH 2/2] feat(tapper): report the driving agent in invocation telemetry The reporter injects the active agent alongside the client version, so neither the CLI nor the MCP call site has to remember to set it, and agent-driven usage separates from human usage in aggregate. It is resolved once at construction: the agent driving a process cannot change while it runs. The alias is user-chosen config text, not an identifier, and is omitted when a human is driving. It is covered by the existing disableTelemetry opt-out and validated by the hub like every other text field. Requires a hub that accepts the field; the hub's decoder rejects unknown fields, so an older hub refuses the batch. That degrades to no telemetry rather than failing a command. --- pkg/tapper/invocation_telemetry.go | 30 +++++++++++++++++++++++-- pkg/tapper/invocation_telemetry_test.go | 25 +++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pkg/tapper/invocation_telemetry.go b/pkg/tapper/invocation_telemetry.go index d781ec7..f4979aa 100644 --- a/pkg/tapper/invocation_telemetry.go +++ b/pkg/tapper/invocation_telemetry.go @@ -24,7 +24,12 @@ const ( // InvocationEvent is the privacy-minimized shape accepted by the Tapper Hub // invocation telemetry endpoint. Callers must set exactly one of Command or -// Tool according to Surface. ClientVersion is injected by the reporter. +// Tool according to Surface. ClientVersion and Agent are injected by the +// reporter. +// +// Agent is the configured `tap launch` agent alias driving the process, empty +// when a human is. It is a user-chosen label, not an identifier, and separates +// agent-driven usage from human usage in aggregate. type InvocationEvent struct { Surface string `json:"surface"` Command string `json:"command,omitempty"` @@ -33,6 +38,7 @@ type InvocationEvent struct { Success bool `json:"success"` Interactive *bool `json:"interactive,omitempty"` ClientVersion string `json:"client_version"` + Agent string `json:"agent,omitempty"` } // InvocationReporter accepts best-effort telemetry without blocking the @@ -48,6 +54,7 @@ type httpInvocationReporter struct { endpoint string token string version string + agent string client *http.Client queue chan InvocationEvent @@ -69,6 +76,7 @@ type invocationReporterOptions struct { batchSize int flushInterval time.Duration requestTimeout time.Duration + agent string } // NewInvocationReporter resolves the user-scope reporting destination and @@ -79,7 +87,23 @@ func NewInvocationReporter(rt *toolkit.Runtime, configService *ConfigService, ve if !ok { return nil } - return newHTTPInvocationReporter(endpoint, token, version, invocationReporterOptions{}) + return newHTTPInvocationReporter(endpoint, token, version, invocationReporterOptions{ + agent: resolveTelemetryAgent(configService), + }) +} + +// resolveTelemetryAgent reads the active agent from the merged config, which is +// where TAP_AGENT lands. It is resolved once at construction rather than per +// event: the agent driving a process cannot change while it runs. +func resolveTelemetryAgent(configService *ConfigService) string { + if configService == nil { + return "" + } + cfg, err := configService.Config() + if err != nil || cfg == nil { + return "" + } + return cfg.AgentName() } func resolveInvocationTelemetryTarget(rt *toolkit.Runtime, configService *ConfigService) (string, string, bool) { @@ -138,6 +162,7 @@ func newHTTPInvocationReporter(endpoint, token, version string, opts invocationR endpoint: endpoint, token: token, version: version, + agent: opts.agent, client: opts.client, queue: make(chan InvocationEvent, opts.queueSize), done: make(chan struct{}), @@ -153,6 +178,7 @@ func (r *httpInvocationReporter) Report(event InvocationEvent) { return } event.ClientVersion = r.version + event.Agent = r.agent r.mu.Lock() defer r.mu.Unlock() if r.closed || r.disabled.Load() { diff --git a/pkg/tapper/invocation_telemetry_test.go b/pkg/tapper/invocation_telemetry_test.go index c046366..89c3c70 100644 --- a/pkg/tapper/invocation_telemetry_test.go +++ b/pkg/tapper/invocation_telemetry_test.go @@ -195,6 +195,31 @@ func TestHTTPInvocationReporterTimeoutAndOlderHubAreBestEffort(t *testing.T) { }) } +// The agent is injected by the reporter, like the client version, so neither +// the CLI nor the MCP call site has to remember to set it. +func TestHTTPInvocationReporterInjectsAgent(t *testing.T) { + bodies := make(chan []byte, 1) + client := &http.Client{Transport: telemetryRoundTripFunc(func(req *http.Request) (*http.Response, error) { + body, _ := io.ReadAll(req.Body) + bodies <- body + return &http.Response{StatusCode: http.StatusNoContent, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil + })} + r := newHTTPInvocationReporter("https://hub.example.com/api/v1/telemetry/invocations", "token", "test", invocationReporterOptions{ + client: client, batchSize: 1, flushInterval: time.Hour, agent: "qwen", + }) + r.Report(InvocationEvent{Surface: "mcp", Tool: "list"}) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + r.Close(ctx) + + select { + case body := <-bodies: + require.Contains(t, string(body), `"agent":"qwen"`) + default: + t.Fatal("no telemetry batch was sent") + } +} + func TestHTTPInvocationReporterConcurrentReportAndClose(t *testing.T) { client := &http.Client{Transport: telemetryRoundTripFunc(func(_ *http.Request) (*http.Response, error) { return &http.Response{StatusCode: http.StatusNoContent, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil