Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Composio login command #1009

Merged
merged 4 commits into from
Dec 14, 2024
Merged

fix: Composio login command #1009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a5cad64
fixed composio login command
plxity Dec 13, 2024
62632e9
fixed prettier
plxity Dec 13, 2024
ca083ed
feat: fixes
himanshu-dixit Dec 14, 2024
b2cbbdc
fix: review
himanshu-dixit Dec 14, 2024
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
3 changes: 2 additions & 1 deletion js/src/env/docker/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type Docker from "dockerode";
import type CliProgress from "cli-progress";
import { IWorkspaceConfig, WorkspaceConfig } from "../config";
import logger from "../../utils/logger";
import { getComposioDir } from "../../sdk/utils/fileUtils";

const ENV_COMPOSIO_DEV_MODE = "COMPOSIO_DEV_MODE";
const ENV_COMPOSIO_SWE_AGENT = "COMPOSIO_SWE_AGENT";
Expand Down Expand Up @@ -96,7 +97,7 @@ export class DockerWorkspace extends RemoteWorkspace {
const os = require("node:os");

const COMPOSIO_PATH = path.resolve(__dirname, "../../../../python/");
const COMPOSIO_CACHE = path.join(os.homedir(), ".composio");
const COMPOSIO_CACHE = getComposioDir(false);

volumeBindings.push(
...[
Comment on lines 97 to 103
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential Issue:

Potential Directory Existence Issue
The recent change in the code modifies the behavior of getComposioDir by setting createDirIfNotExists to false. This could lead to runtime errors if the directory does not exist and is expected to be created automatically.

  • Ensure that the directory is pre-created or handle the scenario where the directory might not exist.
  • Consider adding error handling to manage cases where the directory is missing.
  • Update documentation to inform users about this change in behavior.

This change could potentially affect the functionality that relies on the existence of this directory, so it's important to address this issue proactively. 🛠️

🔧 Suggested Code Diff:

- const COMPOSIO_CACHE = getComposioDir(false);
+ const COMPOSIO_CACHE = getComposioDir(true);

// Alternatively, add error handling
try {
  const COMPOSIO_CACHE = getComposioDir(false);
} catch (error) {
  console.error('Directory does not exist and could not be created:', error);
  // Handle error appropriately
}
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const os = require("node:os");
const COMPOSIO_PATH = path.resolve(__dirname, "../../../../python/");
const COMPOSIO_CACHE = path.join(os.homedir(), ".composio");
const COMPOSIO_CACHE = getComposioDir(false);
volumeBindings.push(
...[
const os = require("node:os");
const COMPOSIO_PATH = path.resolve(__dirname, "../../../../python/");
let COMPOSIO_CACHE;
try {
COMPOSIO_CACHE = getComposioDir(false);
} catch (error) {
console.error('Directory does not exist and could not be created:', error);
// Handle error appropriately, e.g., set a default path or exit process
COMPOSIO_CACHE = path.join(os.homedir(), ".composio"); // Fallback to default
}
volumeBindings.push(
...[
// other volume bindings
]
);

Expand Down
2 changes: 1 addition & 1 deletion js/src/sdk/base.toolset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const fileProcessor = ({
const fileData = toolResponse.data.response_data.file;
const { name, content } = fileData as { name: string; content: string };
const file_name_prefix = `${action}_${Date.now()}`;
const filePath = saveFile(file_name_prefix, content);
const filePath = saveFile(file_name_prefix, content, true);

// @ts-ignore
delete toolResponse.data.response_data.file;
Expand Down
2 changes: 1 addition & 1 deletion js/src/sdk/utils/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ export function setCliConfig(apiKey: string, baseUrl: string) {
userData.base_url = baseUrl;
}

saveFile(userDataPath(), userData);
saveFile(userDataPath(), JSON.stringify(userData));
}
Comment on lines 13 to 17
Copy link

Choose a reason for hiding this comment

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

🤖 Bug Fix:

Verify saveFile Function Input Type
The recent change to use JSON.stringify(userData) in the saveFile function call may introduce a logical error if saveFile is designed to handle objects rather than strings.

  • Verify the expected input type for the saveFile function.
  • If saveFile expects an object, revert the change to pass userData directly.
  • If saveFile expects a string, ensure all usages of saveFile are updated to handle stringified JSON.

This change is critical as it could lead to data corruption or loss if not handled correctly. 🛠️

🔧 Suggested Code Diff:

- saveFile(userDataPath(), JSON.stringify(userData));
+ saveFile(userDataPath(), userData);
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
userData.base_url = baseUrl;
}
saveFile(userDataPath(), userData);
saveFile(userDataPath(), JSON.stringify(userData));
}
saveFile(userDataPath(), userData);

5 changes: 3 additions & 2 deletions js/src/sdk/utils/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as fs from "fs";
import * as path from "path";
import * as os from "os";
import {
LOCAL_CACHE_DIRECTORY_NAME,
COMPOSIO_DIR,
USER_DATA_FILE_NAME,
DEFAULT_BASE_URL,
} from "./constants";
Expand All @@ -23,7 +23,8 @@ declare module "axios" {

// File path helpers
export const userDataPath = () =>
path.join(os.homedir(), LOCAL_CACHE_DIRECTORY_NAME, USER_DATA_FILE_NAME);
path.join(os.homedir(), COMPOSIO_DIR, USER_DATA_FILE_NAME);

export const getUserDataJson = () => {
try {
const data = fs.readFileSync(userDataPath(), "utf8");
Expand Down
3 changes: 2 additions & 1 deletion js/src/sdk/utils/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Constants
export const LOCAL_CACHE_DIRECTORY_NAME = ".composio";
export const COMPOSIO_DIR = ".composio";
export const USER_DATA_FILE_NAME = "user_data.json";
export const TEMP_FILES_DIRECTORY_NAME = "files";
export const DEFAULT_BASE_URL = "https://backend.composio.dev";

export const TELEMETRY_URL = "https://app.composio.dev";
Expand Down
48 changes: 39 additions & 9 deletions js/src/sdk/utils/fileUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,56 @@ import * as path from "path";
import * as os from "os";

import * as fs from "fs";
import { COMPOSIO_DIR, TEMP_FILES_DIRECTORY_NAME } from "./constants";

export const getComposioDir = () => {
const composioDir = path.join(os.homedir(), ".composio");
if (!fs.existsSync(composioDir)) {
/**
* Gets the Composio directory.
* @param createDirIfNotExists - Whether to create the directory if it doesn't exist.
* @returns The path to the Composio directory.
*/
export const getComposioDir = (createDirIfNotExists: boolean = false) => {
const composioDir = path.join(os.homedir(), COMPOSIO_DIR);
if (createDirIfNotExists && !fs.existsSync(composioDir)) {
fs.mkdirSync(composioDir, { recursive: true });
}
return composioDir;
};

export const getComposioFilesDir = () => {
const composioFilesDir = path.join(os.homedir(), ".composio", "files");
if (!fs.existsSync(composioFilesDir)) {
/**
* Gets the Composio temporary files directory.
* @param createDirIfNotExists - Whether to create the directory if it doesn't exist.
* @returns The path to the Composio temporary files directory.
*/
export const getComposioTempFilesDir = (
createDirIfNotExists: boolean = false
) => {
const composioFilesDir = path.join(
os.homedir(),
COMPOSIO_DIR,
TEMP_FILES_DIRECTORY_NAME
);
if (createDirIfNotExists && !fs.existsSync(composioFilesDir)) {
fs.mkdirSync(composioFilesDir, { recursive: true });
}
return composioFilesDir;
};

export const saveFile = (file: string, content: string) => {
const composioFilesDir = getComposioFilesDir();
const filePath = `${composioFilesDir}/${file}`;
/**
* Saves a file to the Composio directory.
* @param file - The name of the file to save.
* @param content - The content of the file to save. Should be a string.
* @param isTempFile - Whether the file is a temporary file.
* @returns The path to the saved file.
*/
export const saveFile = (
file: string,
content: string,
isTempFile: boolean = false
) => {
const composioFilesDir = isTempFile
? getComposioTempFilesDir(true)
: getComposioDir(true);
const filePath = path.join(composioFilesDir, path.basename(file));
fs.writeFileSync(filePath, content);

return filePath;
Expand Down
Loading