From a086957407b9128e4d2923d7bc4e32516a7d9fc3 Mon Sep 17 00:00:00 2001 From: Sarah Wooders Date: Tue, 24 Feb 2026 12:19:31 -0800 Subject: [PATCH] fix(cli): detect API key from env instead of checking repo secrets (#1116) Co-authored-by: Letta Co-authored-by: letta-code <248085862+letta-code@users.noreply.github.com> Co-authored-by: Charles Packer --- src/cli/commands/install-github-app.ts | 41 ++- src/cli/components/InstallGithubAppFlow.tsx | 156 ++--------- src/tests/cli/install-github-app.test.ts | 281 ++++++++++++++++++++ 3 files changed, 318 insertions(+), 160 deletions(-) diff --git a/src/cli/commands/install-github-app.ts b/src/cli/commands/install-github-app.ts index 6d9b6bd..892d174 100644 --- a/src/cli/commands/install-github-app.ts +++ b/src/cli/commands/install-github-app.ts @@ -50,13 +50,11 @@ export interface GhPreflightResult { export interface RepoSetupState { workflowExists: boolean; - secretExists: boolean; } export interface InstallGithubAppOptions { repo: string; workflowPath: string; - reuseExistingSecret: boolean; apiKey: string | null; agentMode: "current" | "existing" | "create"; agentId: string | null; @@ -301,8 +299,7 @@ export function getDefaultWorkflowPath(workflowExists: boolean): string { export function getRepoSetupState(repo: string): RepoSetupState { const workflowExists = checkRemoteFileExists(repo, DEFAULT_WORKFLOW_PATH); - const secretExists = hasRepositorySecret(repo, "LETTA_API_KEY"); - return { workflowExists, secretExists }; + return { workflowExists }; } export function hasRepositorySecret(repo: string, secretName: string): boolean { @@ -455,7 +452,6 @@ export async function installGithubApp( const { repo, workflowPath, - reuseExistingSecret, apiKey, agentMode, agentId: providedAgentId, @@ -467,24 +463,18 @@ export async function installGithubApp( throw new Error("Repository must be in owner/repo format."); } - if (!apiKey && (!reuseExistingSecret || agentMode === "create")) { + if (!apiKey) { throw new Error("LETTA_API_KEY is required."); } - - const secretAction: "reused" | "set" = reuseExistingSecret ? "reused" : "set"; let resolvedAgentId: string | null = providedAgentId; progress(onProgress, "Getting repository information"); ensureRepoAccess(repo); - // Create agent if needed (requires API key) + // Create agent if needed if (agentMode === "create" && agentName) { - const keyForAgent = apiKey; - if (!keyForAgent) { - throw new Error("LETTA_API_KEY is required to create an agent."); - } progress(onProgress, `Creating agent ${agentName}`); - const agent = await createLettaAgent(keyForAgent, agentName); + const agent = await createLettaAgent(apiKey, agentName); resolvedAgentId = agent.id; } @@ -502,6 +492,15 @@ export async function installGithubApp( progress(onProgress, "Creating workflow files"); const changed = writeWorkflow(repoDir, workflowPath, workflowContent); + // Always set the secret from the locally-available key + progress(onProgress, "Setting up LETTA_API_KEY secret"); + setRepositorySecret(repo, "LETTA_API_KEY", apiKey); + + if (resolvedAgentId) { + progress(onProgress, "Configuring agent"); + setRepositoryVariable(repo, "LETTA_AGENT_ID", resolvedAgentId); + } + if (!changed) { progress(onProgress, "Workflow already up to date."); return { @@ -511,7 +510,7 @@ export async function installGithubApp( pullRequestUrl: null, pullRequestCreateMode: "created", committed: false, - secretAction, + secretAction: "set", agentId: resolvedAgentId, agentUrl: resolvedAgentId ? `https://app.letta.com/agents/${resolvedAgentId}` @@ -522,16 +521,6 @@ export async function installGithubApp( runGit(["add", workflowPath], repoDir); runGit(["commit", "-m", "Add Letta Code GitHub Workflow"], repoDir); - if (!reuseExistingSecret && apiKey) { - progress(onProgress, "Setting up LETTA_API_KEY secret"); - setRepositorySecret(repo, "LETTA_API_KEY", apiKey); - } - - if (resolvedAgentId) { - progress(onProgress, "Configuring agent"); - setRepositoryVariable(repo, "LETTA_AGENT_ID", resolvedAgentId); - } - progress(onProgress, "Opening pull request page"); runGit(["push", "-u", "origin", branchName], repoDir); @@ -549,7 +538,7 @@ export async function installGithubApp( pullRequestUrl: pullRequest.url, pullRequestCreateMode: pullRequest.mode, committed: true, - secretAction, + secretAction: "set", agentId: resolvedAgentId, agentUrl: resolvedAgentId ? `https://app.letta.com/agents/${resolvedAgentId}` diff --git a/src/cli/components/InstallGithubAppFlow.tsx b/src/cli/components/InstallGithubAppFlow.tsx index 8811f28..c29707b 100644 --- a/src/cli/components/InstallGithubAppFlow.tsx +++ b/src/cli/components/InstallGithubAppFlow.tsx @@ -33,7 +33,6 @@ type Step = | "checking" | "choose-repo" | "enter-repo" - | "choose-secret" | "enter-api-key" | "choose-agent" | "enter-agent-name" @@ -55,10 +54,9 @@ interface ProgressItem { const SOLID_LINE = "─"; -function buildProgressSteps( +export function buildProgressSteps( agentMode: "current" | "existing" | "create", agentName: string | null, - reuseExistingSecret: boolean, ): { key: string; label: string }[] { const steps: { key: string; label: string }[] = []; steps.push({ @@ -76,14 +74,11 @@ function buildProgressSteps( key: "Creating workflow files", label: "Creating workflow files", }); - if (!reuseExistingSecret) { - steps.push({ - key: "Setting up LETTA_API_KEY secret", - label: "Setting up LETTA_API_KEY secret", - }); - } + steps.push({ + key: "Setting up LETTA_API_KEY secret", + label: "Setting up LETTA_API_KEY secret", + }); if (agentMode !== "create") { - // create mode sets variable inline during agent creation step steps.push({ key: "Configuring agent", label: "Configuring agent" }); } steps.push({ @@ -93,13 +88,12 @@ function buildProgressSteps( return steps; } -function buildProgress( +export function buildProgress( currentStatus: string, agentMode: "current" | "existing" | "create", agentName: string | null, - reuseExistingSecret: boolean, ): ProgressItem[] { - const steps = buildProgressSteps(agentMode, agentName, reuseExistingSecret); + const steps = buildProgressSteps(agentMode, agentName); const normalized = currentStatus.toLowerCase(); const activeIndex = steps.findIndex((step) => @@ -189,12 +183,8 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ const [repoError, setRepoError] = useState(""); // Secret state - const [secretExists, setSecretExists] = useState(false); - const [secretChoiceIndex, setSecretChoiceIndex] = useState(0); const [apiKeyInput, setApiKeyInput] = useState(""); const [envApiKey, setEnvApiKey] = useState(null); - const [reuseExistingSecret, setReuseExistingSecret] = - useState(false); // Agent state const [currentAgentId, setCurrentAgentIdState] = useState( @@ -230,23 +220,6 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ return [{ label: "Enter a repository", value: "manual" as const }]; }, [currentRepo]); - const secretChoices = useMemo( - () => - secretExists - ? [ - { - label: "Use existing LETTA_API_KEY secret (recommended)", - value: "reuse" as const, - }, - { - label: "Set or overwrite LETTA_API_KEY secret", - value: "set" as const, - }, - ] - : [{ label: "Set LETTA_API_KEY secret", value: "set" as const }], - [secretExists], - ); - const agentChoices = useMemo(() => { const choices: { label: string; @@ -281,7 +254,6 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ finalAgentMode: "current" | "existing" | "create", finalAgentId: string | null, finalAgentName: string | null, - useExistingSecret: boolean, key: string | null, ) => { if (!repo) { @@ -290,7 +262,6 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ return; } - setReuseExistingSecret(useExistingSecret); setAgentMode(finalAgentMode); setStep("creating"); setStatus("Preparing setup..."); @@ -299,7 +270,6 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ const installResult = await installGithubApp({ repo, workflowPath, - reuseExistingSecret: useExistingSecret, apiKey: key, agentMode: finalAgentMode, agentId: finalAgentId, @@ -331,21 +301,14 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ try { const setup = getRepoSetupState(trimmed); setRepo(trimmed); - setSecretExists(setup.secretExists); setWorkflowPath(getDefaultWorkflowPath(setup.workflowExists)); - setSecretChoiceIndex(0); if (envApiKey) { - // Already have API key from environment — skip entry, go to secret/agent choice - if (setup.secretExists) { - setStep("choose-secret"); - } else { - setReuseExistingSecret(false); - setAgentChoiceIndex(0); - setStep("choose-agent"); - } + // Already have API key from environment — skip to agent choice + setAgentChoiceIndex(0); + setStep("choose-agent"); } else { - // OAuth user — need to collect API key + // Need to collect API key setStep("enter-api-key"); } } catch (error) { @@ -409,15 +372,9 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ agentId: string | null, agentName: string | null, ) => { - void runInstall( - mode, - agentId, - agentName, - reuseExistingSecret, - availableApiKey, - ); + void runInstall(mode, agentId, agentName, availableApiKey); }, - [availableApiKey, reuseExistingSecret, runInstall], + [availableApiKey, runInstall], ); useInput((input, key) => { @@ -454,23 +411,8 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ } return; } - if (step === "choose-secret") { - if (envApiKey) { - // Skipped API key entry — go back to repo - if (currentRepo) { - setStep("choose-repo"); - } else { - setStep("enter-repo"); - } - } else { - setStep("enter-api-key"); - } - return; - } if (step === "choose-agent") { - if (secretExists) { - setStep("choose-secret"); - } else if (envApiKey) { + if (envApiKey) { if (currentRepo) { setStep("choose-repo"); } else { @@ -513,23 +455,6 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ return; } - if (step === "choose-secret") { - if (key.upArrow || input === "k") { - setSecretChoiceIndex((prev) => Math.max(0, prev - 1)); - } else if (key.downArrow || input === "j") { - setSecretChoiceIndex((prev) => - Math.min(secretChoices.length - 1, prev + 1), - ); - } else if (key.return) { - const selected = secretChoices[secretChoiceIndex] ?? secretChoices[0]; - if (!selected) return; - setReuseExistingSecret(selected.value === "reuse"); - setAgentChoiceIndex(0); - setStep("choose-agent"); - } - return; - } - if (step === "choose-agent") { if (key.upArrow || input === "k") { setAgentChoiceIndex((prev) => Math.max(0, prev - 1)); @@ -554,25 +479,13 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ }); // Handlers for text input steps - const handleApiKeySubmit = useCallback( - (value: string) => { - const trimmed = value.trim(); - if (!trimmed) return; - setApiKeyInput(trimmed); - - if (secretExists) { - // Ask whether to reuse or overwrite the existing secret - setSecretChoiceIndex(0); - setStep("choose-secret"); - } else { - // No existing secret — we'll set it during install - setReuseExistingSecret(false); - setAgentChoiceIndex(0); - setStep("choose-agent"); - } - }, - [secretExists], - ); + const handleApiKeySubmit = useCallback((value: string) => { + const trimmed = value.trim(); + if (!trimmed) return; + setApiKeyInput(trimmed); + setAgentChoiceIndex(0); + setStep("choose-agent"); + }, []); const handleAgentNameSubmit = useCallback( (value: string) => { @@ -651,28 +564,6 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ ); } - if (step === "choose-secret") { - return renderPanel( - solidLine, - "Install GitHub App", - "Configure LETTA_API_KEY", - <> - - Repository: - {repo} - - - Workflow: - {workflowPath} - - - - - ↑/↓ to select · Enter to continue · Esc to go back - , - ); - } - if (step === "enter-api-key") { return renderPanel( solidLine, @@ -769,7 +660,6 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ status, agentMode, agentNameInput || null, - reuseExistingSecret, ); return renderPanel( solidLine, @@ -801,9 +691,7 @@ export const InstallGithubAppFlow = memo(function InstallGithubAppFlow({ const successLines: string[] = [ "✓ GitHub Actions workflow created!", "", - reuseExistingSecret - ? "✓ Using existing LETTA_API_KEY secret" - : "✓ API key saved as LETTA_API_KEY secret", + "✓ API key saved as LETTA_API_KEY secret", ]; if (result?.agentId) { diff --git a/src/tests/cli/install-github-app.test.ts b/src/tests/cli/install-github-app.test.ts index d3b66f5..3dcc2a9 100644 --- a/src/tests/cli/install-github-app.test.ts +++ b/src/tests/cli/install-github-app.test.ts @@ -3,10 +3,19 @@ import { buildInstallPrBody, generateLettaWorkflowYaml, getDefaultWorkflowPath, + type InstallGithubAppResult, parseGitHubRepoFromRemote, parseScopesFromGhAuthStatus, validateRepoSlug, } from "../../cli/commands/install-github-app"; +import { + buildProgress, + buildProgressSteps, +} from "../../cli/components/InstallGithubAppFlow"; + +// --------------------------------------------------------------------------- +// Helper utilities +// --------------------------------------------------------------------------- describe("install-github-app helpers", () => { test("validateRepoSlug accepts owner/repo and rejects invalid forms", () => { @@ -92,3 +101,275 @@ describe("install-github-app helpers", () => { expect(body).toContain("letta-code-action"); }); }); + +// --------------------------------------------------------------------------- +// Wizard progress steps +// --------------------------------------------------------------------------- + +describe("buildProgressSteps", () => { + test("includes all standard steps for current agent mode", () => { + const steps = buildProgressSteps("current", null); + const labels = steps.map((s) => s.label); + + expect(labels).toEqual([ + "Getting repository information", + "Creating branch", + "Creating workflow files", + "Setting up LETTA_API_KEY secret", + "Configuring agent", + "Opening pull request page", + ]); + }); + + test("includes all standard steps for existing agent mode", () => { + const steps = buildProgressSteps("existing", null); + const labels = steps.map((s) => s.label); + + expect(labels).toContain("Configuring agent"); + expect(labels).toContain("Setting up LETTA_API_KEY secret"); + expect(labels).not.toContainEqual( + expect.stringContaining("Creating agent"), + ); + }); + + test("includes agent creation step in create mode", () => { + const steps = buildProgressSteps("create", "My Bot"); + const labels = steps.map((s) => s.label); + + expect(labels).toContain("Creating agent My Bot"); + }); + + test("omits 'Configuring agent' in create mode", () => { + const steps = buildProgressSteps("create", "My Bot"); + const labels = steps.map((s) => s.label); + + expect(labels).not.toContain("Configuring agent"); + }); + + test("always includes LETTA_API_KEY secret step regardless of mode", () => { + for (const mode of ["current", "existing", "create"] as const) { + const steps = buildProgressSteps(mode, mode === "create" ? "Bot" : null); + const labels = steps.map((s) => s.label); + expect(labels).toContain("Setting up LETTA_API_KEY secret"); + } + }); + + test("steps are in the correct order", () => { + const steps = buildProgressSteps("current", null); + const labels = steps.map((s) => s.label); + + const repoIdx = labels.indexOf("Getting repository information"); + const branchIdx = labels.indexOf("Creating branch"); + const workflowIdx = labels.indexOf("Creating workflow files"); + const secretIdx = labels.indexOf("Setting up LETTA_API_KEY secret"); + const agentIdx = labels.indexOf("Configuring agent"); + const prIdx = labels.indexOf("Opening pull request page"); + + expect(repoIdx).toBeLessThan(branchIdx); + expect(branchIdx).toBeLessThan(workflowIdx); + expect(workflowIdx).toBeLessThan(secretIdx); + expect(secretIdx).toBeLessThan(agentIdx); + expect(agentIdx).toBeLessThan(prIdx); + }); +}); + +// --------------------------------------------------------------------------- +// Progress state machine +// --------------------------------------------------------------------------- + +describe("buildProgress", () => { + test("marks first step as active when status matches it", () => { + const items = buildProgress( + "Getting repository information", + "current", + null, + ); + + expect(items[0]?.active).toBe(true); + expect(items[0]?.done).toBe(false); + // All subsequent steps should be inactive and not done + for (const item of items.slice(1)) { + expect(item.active).toBe(false); + expect(item.done).toBe(false); + } + }); + + test("marks prior steps as done and current step as active", () => { + const items = buildProgress("Creating workflow files", "current", null); + const labels = items.map((i) => i.label); + const workflowIdx = labels.indexOf("Creating workflow files"); + + // All steps before should be done + for (let i = 0; i < workflowIdx; i++) { + expect(items[i]?.done).toBe(true); + expect(items[i]?.active).toBe(false); + } + // Current step should be active + expect(items[workflowIdx]?.active).toBe(true); + expect(items[workflowIdx]?.done).toBe(false); + // Steps after should be neither done nor active + for (let i = workflowIdx + 1; i < items.length; i++) { + expect(items[i]?.done).toBe(false); + expect(items[i]?.active).toBe(false); + } + }); + + test("marks all prior steps done when on last step", () => { + const items = buildProgress("Opening pull request page", "current", null); + const lastIdx = items.length - 1; + + for (let i = 0; i < lastIdx; i++) { + expect(items[i]?.done).toBe(true); + } + expect(items[lastIdx]?.active).toBe(true); + expect(items[lastIdx]?.done).toBe(false); + }); + + test("no step is active when status doesn't match any step", () => { + const items = buildProgress("Preparing setup...", "current", null); + + for (const item of items) { + expect(item.active).toBe(false); + expect(item.done).toBe(false); + } + }); + + test("matches status case-insensitively", () => { + const items = buildProgress( + "SETTING UP LETTA_API_KEY SECRET", + "current", + null, + ); + const secretItem = items.find( + (i) => i.label === "Setting up LETTA_API_KEY secret", + ); + + expect(secretItem?.active).toBe(true); + }); + + test("includes agent creation step in create mode progress", () => { + const items = buildProgress("Creating agent My Bot", "create", "My Bot"); + const agentItem = items.find((i) => i.label === "Creating agent My Bot"); + + expect(agentItem).toBeDefined(); + expect(agentItem?.active).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// Success screen / InstallGithubAppResult content +// --------------------------------------------------------------------------- + +describe("success screen content", () => { + const baseResult: InstallGithubAppResult = { + repo: "letta-ai/letta-code", + workflowPath: ".github/workflows/letta.yml", + branchName: "letta/install-github-app-abc123", + pullRequestUrl: "https://github.com/letta-ai/letta-code/pull/42", + pullRequestCreateMode: "created", + committed: true, + secretAction: "set", + agentId: "agent-aaaabbbb-cccc-dddd-eeee-ffffffffffff", + agentUrl: + "https://app.letta.com/agents/agent-aaaabbbb-cccc-dddd-eeee-ffffffffffff", + }; + + test("agentUrl points to app.letta.com ADE", () => { + expect(baseResult.agentUrl).toBe( + `https://app.letta.com/agents/${baseResult.agentId}`, + ); + }); + + test("agentUrl is null when agentId is null", () => { + const noAgent: InstallGithubAppResult = { + ...baseResult, + agentId: null, + agentUrl: null, + }; + expect(noAgent.agentUrl).toBeNull(); + }); + + test("secretAction is always 'set' after the API key fix", () => { + // After removing reuseExistingSecret, the secret is always set + expect(baseResult.secretAction).toBe("set"); + }); + + test("pullRequestUrl is present when workflow was committed", () => { + expect(baseResult.committed).toBe(true); + expect(baseResult.pullRequestUrl).toContain("github.com"); + expect(baseResult.pullRequestUrl).toContain("/pull/"); + }); + + test("pullRequestUrl is null when workflow was unchanged", () => { + const unchanged: InstallGithubAppResult = { + ...baseResult, + committed: false, + branchName: null, + pullRequestUrl: null, + }; + expect(unchanged.pullRequestUrl).toBeNull(); + expect(unchanged.branchName).toBeNull(); + }); + + // Simulate the success lines built by the component to verify content + test("success lines include expected content", () => { + const result = baseResult; + const successLines: string[] = [ + "✓ GitHub Actions workflow created!", + "", + "✓ API key saved as LETTA_API_KEY secret", + ]; + + if (result.agentId) { + successLines.push(""); + successLines.push(`✓ Agent configured: ${result.agentId}`); + } + + successLines.push(""); + successLines.push("Next steps:"); + successLines.push("1. A pre-filled PR page has been created"); + successLines.push("2. Merge the PR to enable Letta Code PR assistance"); + successLines.push("3. Mention @letta-code in an issue or PR to test"); + + // Verify all expected content is present + const allText = successLines.join("\n"); + expect(allText).toContain("GitHub Actions workflow created"); + expect(allText).toContain("API key saved as LETTA_API_KEY secret"); + expect(allText).toContain(`Agent configured: ${result.agentId}`); + expect(allText).toContain("Merge the PR"); + expect(allText).toContain("@letta-code"); + }); + + test("success lines omit agent line when no agent configured", () => { + const result: InstallGithubAppResult = { + ...baseResult, + agentId: null, + agentUrl: null, + }; + const successLines: string[] = [ + "✓ GitHub Actions workflow created!", + "", + "✓ API key saved as LETTA_API_KEY secret", + ]; + + if (result.agentId) { + successLines.push(""); + successLines.push(`✓ Agent configured: ${result.agentId}`); + } + + const allText = successLines.join("\n"); + expect(allText).not.toContain("Agent configured"); + }); + + test("agent URL uses correct ADE format for any agent ID", () => { + const agentId = "agent-12345678-abcd-efgh-ijkl-123456789012"; + const expectedUrl = `https://app.letta.com/agents/${agentId}`; + + // This mirrors the logic in installGithubApp + const agentUrl = agentId ? `https://app.letta.com/agents/${agentId}` : null; + + expect(agentUrl).toBe(expectedUrl); + expect(agentUrl).toContain("app.letta.com/agents/"); + expect(agentUrl).toContain(agentId); + }); +});