From 32f093ef5933528b6549938c5447ea926069e807 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Tue, 20 May 2025 13:50:17 -0500 Subject: [PATCH 1/2] fix: correct environment variable name for MCP app status slug (cherry-pick #17948) (#17950) Cherry-picked fix: correct environment variable name for MCP app status slug (#17948) Fixed environment variable name for app status slug in Claude MCP configuration from `CODER_MCP_CLAUDE_APP_STATUS_SLUG` to `CODER_MCP_APP_STATUS_SLUG` to maintain consistency with other MCP environment variables. This also caused the User level Claude.md to not contain instructions to report its progress, so it did not receive status reports. Co-authored-by: Thomas Kosiewski --- cli/exp_mcp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/exp_mcp.go b/cli/exp_mcp.go index 6174f0cffbf0e..fb866666daf4a 100644 --- a/cli/exp_mcp.go +++ b/cli/exp_mcp.go @@ -255,7 +255,7 @@ func (*RootCmd) mcpConfigureClaudeCode() *serpent.Command { { Name: "app-status-slug", Description: "The app status slug to use when running the Coder MCP server.", - Env: "CODER_MCP_CLAUDE_APP_STATUS_SLUG", + Env: "CODER_MCP_APP_STATUS_SLUG", Flag: "claude-app-status-slug", Value: serpent.StringOf(&appStatusSlug), }, From 8708d812222dbac749b0957361faa87e488dda75 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 20 May 2025 21:27:28 +0200 Subject: [PATCH 2/2] chore: cherry-pick #17934 into 2.22 (#17952) Cherry-pick of https://github.com/coder/coder/pull/17934 --------- Signed-off-by: Thomas Kosiewski Signed-off-by: Danny Kopping Co-authored-by: Thomas Kosiewski --- coderd/agentapi/manifest.go | 11 +- coderd/agentapi/manifest_test.go | 9 +- coderd/workspaceagents_test.go | 64 ++++++--- coderd/workspaceagentsrpc.go | 13 +- coderd/workspaceagentsrpc_test.go | 209 +++++++++++++++++++----------- flake.nix | 1 + 6 files changed, 192 insertions(+), 115 deletions(-) diff --git a/coderd/agentapi/manifest.go b/coderd/agentapi/manifest.go index db8a0af3946a9..6815e8484b286 100644 --- a/coderd/agentapi/manifest.go +++ b/coderd/agentapi/manifest.go @@ -47,7 +47,6 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest scripts []database.WorkspaceAgentScript metadata []database.WorkspaceAgentMetadatum workspace database.Workspace - owner database.User devcontainers []database.WorkspaceAgentDevcontainer ) @@ -76,10 +75,6 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest if err != nil { return xerrors.Errorf("getting workspace by id: %w", err) } - owner, err = a.Database.GetUserByID(ctx, workspace.OwnerID) - if err != nil { - return xerrors.Errorf("getting workspace owner by id: %w", err) - } return err }) eg.Go(func() (err error) { @@ -98,7 +93,7 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest AppSlugOrPort: "{{port}}", AgentName: workspaceAgent.Name, WorkspaceName: workspace.Name, - Username: owner.Username, + Username: workspace.OwnerUsername, } vscodeProxyURI := vscodeProxyURI(appSlug, a.AccessURL, a.AppHostname) @@ -115,7 +110,7 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest } } - apps, err := dbAppsToProto(dbApps, workspaceAgent, owner.Username, workspace) + apps, err := dbAppsToProto(dbApps, workspaceAgent, workspace.OwnerUsername, workspace) if err != nil { return nil, xerrors.Errorf("converting workspace apps: %w", err) } @@ -123,7 +118,7 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest return &agentproto.Manifest{ AgentId: workspaceAgent.ID[:], AgentName: workspaceAgent.Name, - OwnerUsername: owner.Username, + OwnerUsername: workspace.OwnerUsername, WorkspaceId: workspace.ID[:], WorkspaceName: workspace.Name, GitAuthConfigs: gitAuthConfigs, diff --git a/coderd/agentapi/manifest_test.go b/coderd/agentapi/manifest_test.go index 98e7ccc8c8b52..178e790ab914d 100644 --- a/coderd/agentapi/manifest_test.go +++ b/coderd/agentapi/manifest_test.go @@ -46,9 +46,10 @@ func TestGetManifest(t *testing.T) { Username: "cool-user", } workspace = database.Workspace{ - ID: uuid.New(), - OwnerID: owner.ID, - Name: "cool-workspace", + ID: uuid.New(), + OwnerID: owner.ID, + OwnerUsername: owner.Username, + Name: "cool-workspace", } agent = database.WorkspaceAgent{ ID: uuid.New(), @@ -329,7 +330,6 @@ func TestGetManifest(t *testing.T) { }).Return(metadata, nil) mDB.EXPECT().GetWorkspaceAgentDevcontainersByAgentID(gomock.Any(), agent.ID).Return(devcontainers, nil) mDB.EXPECT().GetWorkspaceByID(gomock.Any(), workspace.ID).Return(workspace, nil) - mDB.EXPECT().GetUserByID(gomock.Any(), workspace.OwnerID).Return(owner, nil) got, err := api.GetManifest(context.Background(), &agentproto.GetManifestRequest{}) require.NoError(t, err) @@ -396,7 +396,6 @@ func TestGetManifest(t *testing.T) { }).Return(metadata, nil) mDB.EXPECT().GetWorkspaceAgentDevcontainersByAgentID(gomock.Any(), agent.ID).Return(devcontainers, nil) mDB.EXPECT().GetWorkspaceByID(gomock.Any(), workspace.ID).Return(workspace, nil) - mDB.EXPECT().GetUserByID(gomock.Any(), workspace.OwnerID).Return(owner, nil) got, err := api.GetManifest(context.Background(), &agentproto.GetManifestRequest{}) require.NoError(t, err) diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 10403f1ac00ae..81aa74a2f2fb5 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -437,25 +437,55 @@ func TestWorkspaceAgentConnectRPC(t *testing.T) { t.Run("Connect", func(t *testing.T) { t.Parallel() - client, db := coderdtest.NewWithDatabase(t, nil) - user := coderdtest.CreateFirstUser(t, client) - r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OrganizationID: user.OrganizationID, - OwnerID: user.UserID, - }).WithAgent().Do() - _ = agenttest.New(t, client.URL, r.AgentToken) - resources := coderdtest.AwaitWorkspaceAgents(t, client, r.Workspace.ID) + for _, tc := range []struct { + name string + apiKeyScope rbac.ScopeName + }{ + { + name: "empty (backwards compat)", + apiKeyScope: "", + }, + { + name: "all", + apiKeyScope: rbac.ScopeAll, + }, + { + name: "no_user_data", + apiKeyScope: rbac.ScopeNoUserData, + }, + { + name: "application_connect", + apiKeyScope: rbac.ScopeApplicationConnect, + }, + } { + t.Run(tc.name, func(t *testing.T) { + client, db := coderdtest.NewWithDatabase(t, nil) + user := coderdtest.CreateFirstUser(t, client) + r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OrganizationID: user.OrganizationID, + OwnerID: user.UserID, + }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { + for _, agent := range agents { + agent.ApiKeyScope = string(tc.apiKeyScope) + } - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + return agents + }).Do() + _ = agenttest.New(t, client.URL, r.AgentToken) + resources := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID).AgentNames([]string{}).Wait() - conn, err := workspacesdk.New(client). - DialAgent(ctx, resources[0].Agents[0].ID, nil) - require.NoError(t, err) - defer func() { - _ = conn.Close() - }() - conn.AwaitReachable(ctx) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + conn, err := workspacesdk.New(client). + DialAgent(ctx, resources[0].Agents[0].ID, nil) + require.NoError(t, err) + defer func() { + _ = conn.Close() + }() + conn.AwaitReachable(ctx) + }) + } }) t.Run("FailNonLatestBuild", func(t *testing.T) { diff --git a/coderd/workspaceagentsrpc.go b/coderd/workspaceagentsrpc.go index 43da35410f632..2dcf65bd8c7d5 100644 --- a/coderd/workspaceagentsrpc.go +++ b/coderd/workspaceagentsrpc.go @@ -76,17 +76,8 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) { return } - owner, err := api.Database.GetUserByID(ctx, workspace.OwnerID) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Internal error fetching user.", - Detail: err.Error(), - }) - return - } - logger = logger.With( - slog.F("owner", owner.Username), + slog.F("owner", workspace.OwnerUsername), slog.F("workspace_name", workspace.Name), slog.F("agent_name", workspaceAgent.Name), ) @@ -170,7 +161,7 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) { }) streamID := tailnet.StreamID{ - Name: fmt.Sprintf("%s-%s-%s", owner.Username, workspace.Name, workspaceAgent.Name), + Name: fmt.Sprintf("%s-%s-%s", workspace.OwnerUsername, workspace.Name, workspaceAgent.Name), ID: workspaceAgent.ID, Auth: tailnet.AgentCoordinateeAuth{ID: workspaceAgent.ID}, } diff --git a/coderd/workspaceagentsrpc_test.go b/coderd/workspaceagentsrpc_test.go index 3f1f1a2b8a764..233c5665310f6 100644 --- a/coderd/workspaceagentsrpc_test.go +++ b/coderd/workspaceagentsrpc_test.go @@ -13,6 +13,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" @@ -22,88 +23,148 @@ import ( func TestWorkspaceAgentReportStats(t *testing.T) { t.Parallel() - tickCh := make(chan time.Time) - flushCh := make(chan int, 1) - client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ - WorkspaceUsageTrackerFlush: flushCh, - WorkspaceUsageTrackerTick: tickCh, - }) - user := coderdtest.CreateFirstUser(t, client) - r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OrganizationID: user.OrganizationID, - OwnerID: user.UserID, - }).WithAgent().Do() + for _, tc := range []struct { + name string + apiKeyScope rbac.ScopeName + }{ + { + name: "empty (backwards compat)", + apiKeyScope: "", + }, + { + name: "all", + apiKeyScope: rbac.ScopeAll, + }, + { + name: "no_user_data", + apiKeyScope: rbac.ScopeNoUserData, + }, + { + name: "application_connect", + apiKeyScope: rbac.ScopeApplicationConnect, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() - ac := agentsdk.New(client.URL) - ac.SetSessionToken(r.AgentToken) - conn, err := ac.ConnectRPC(context.Background()) - require.NoError(t, err) - defer func() { - _ = conn.Close() - }() - agentAPI := agentproto.NewDRPCAgentClient(conn) + tickCh := make(chan time.Time) + flushCh := make(chan int, 1) + client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ + WorkspaceUsageTrackerFlush: flushCh, + WorkspaceUsageTrackerTick: tickCh, + }) + user := coderdtest.CreateFirstUser(t, client) + r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OrganizationID: user.OrganizationID, + OwnerID: user.UserID, + }).WithAgent(func(agent []*proto.Agent) []*proto.Agent { + for _, a := range agent { + a.ApiKeyScope = string(tc.apiKeyScope) + } - _, err = agentAPI.UpdateStats(context.Background(), &agentproto.UpdateStatsRequest{ - Stats: &agentproto.Stats{ - ConnectionsByProto: map[string]int64{"TCP": 1}, - ConnectionCount: 1, - RxPackets: 1, - RxBytes: 1, - TxPackets: 1, - TxBytes: 1, - SessionCountVscode: 1, - SessionCountJetbrains: 0, - SessionCountReconnectingPty: 0, - SessionCountSsh: 0, - ConnectionMedianLatencyMs: 10, - }, - }) - require.NoError(t, err) + return agent + }, + ).Do() + + ac := agentsdk.New(client.URL) + ac.SetSessionToken(r.AgentToken) + conn, err := ac.ConnectRPC(context.Background()) + require.NoError(t, err) + defer func() { + _ = conn.Close() + }() + agentAPI := agentproto.NewDRPCAgentClient(conn) + + _, err = agentAPI.UpdateStats(context.Background(), &agentproto.UpdateStatsRequest{ + Stats: &agentproto.Stats{ + ConnectionsByProto: map[string]int64{"TCP": 1}, + ConnectionCount: 1, + RxPackets: 1, + RxBytes: 1, + TxPackets: 1, + TxBytes: 1, + SessionCountVscode: 1, + SessionCountJetbrains: 0, + SessionCountReconnectingPty: 0, + SessionCountSsh: 0, + ConnectionMedianLatencyMs: 10, + }, + }) + require.NoError(t, err) - tickCh <- dbtime.Now() - count := <-flushCh - require.Equal(t, 1, count, "expected one flush with one id") + tickCh <- dbtime.Now() + count := <-flushCh + require.Equal(t, 1, count, "expected one flush with one id") - newWorkspace, err := client.Workspace(context.Background(), r.Workspace.ID) - require.NoError(t, err) + newWorkspace, err := client.Workspace(context.Background(), r.Workspace.ID) + require.NoError(t, err) - assert.True(t, - newWorkspace.LastUsedAt.After(r.Workspace.LastUsedAt), - "%s is not after %s", newWorkspace.LastUsedAt, r.Workspace.LastUsedAt, - ) + assert.True(t, + newWorkspace.LastUsedAt.After(r.Workspace.LastUsedAt), + "%s is not after %s", newWorkspace.LastUsedAt, r.Workspace.LastUsedAt, + ) + }) + } } func TestAgentAPI_LargeManifest(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitLong) - client, store := coderdtest.NewWithDatabase(t, nil) - adminUser := coderdtest.CreateFirstUser(t, client) - n := 512000 - longScript := make([]byte, n) - for i := range longScript { - longScript[i] = 'q' + + for _, tc := range []struct { + name string + apiKeyScope rbac.ScopeName + }{ + { + name: "empty (backwards compat)", + apiKeyScope: "", + }, + { + name: "all", + apiKeyScope: rbac.ScopeAll, + }, + { + name: "no_user_data", + apiKeyScope: rbac.ScopeNoUserData, + }, + { + name: "application_connect", + apiKeyScope: rbac.ScopeApplicationConnect, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + client, store := coderdtest.NewWithDatabase(t, nil) + adminUser := coderdtest.CreateFirstUser(t, client) + n := 512000 + longScript := make([]byte, n) + for i := range longScript { + longScript[i] = 'q' + } + r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ + OrganizationID: adminUser.OrganizationID, + OwnerID: adminUser.UserID, + }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { + agents[0].Scripts = []*proto.Script{ + { + Script: string(longScript), + }, + } + agents[0].ApiKeyScope = string(tc.apiKeyScope) + return agents + }).Do() + ac := agentsdk.New(client.URL) + ac.SetSessionToken(r.AgentToken) + conn, err := ac.ConnectRPC(ctx) + defer func() { + _ = conn.Close() + }() + require.NoError(t, err) + agentAPI := agentproto.NewDRPCAgentClient(conn) + manifest, err := agentAPI.GetManifest(ctx, &agentproto.GetManifestRequest{}) + require.NoError(t, err) + require.Len(t, manifest.Scripts, 1) + require.Len(t, manifest.Scripts[0].Script, n) + }) } - r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ - OrganizationID: adminUser.OrganizationID, - OwnerID: adminUser.UserID, - }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { - agents[0].Scripts = []*proto.Script{ - { - Script: string(longScript), - }, - } - return agents - }).Do() - ac := agentsdk.New(client.URL) - ac.SetSessionToken(r.AgentToken) - conn, err := ac.ConnectRPC(ctx) - defer func() { - _ = conn.Close() - }() - require.NoError(t, err) - agentAPI := agentproto.NewDRPCAgentClient(conn) - manifest, err := agentAPI.GetManifest(ctx, &agentproto.GetManifestRequest{}) - require.NoError(t, err) - require.Len(t, manifest.Scripts, 1) - require.Len(t, manifest.Scripts[0].Script, n) } diff --git a/flake.nix b/flake.nix index bff207662f913..c0f36c3be6e0f 100644 --- a/flake.nix +++ b/flake.nix @@ -141,6 +141,7 @@ kubectl kubectx kubernetes-helm + lazydocker lazygit less mockgen