From 8dade49172333cb6a7a71b043f95cb1716d4d06d Mon Sep 17 00:00:00 2001 From: Daniel Draper Date: Fri, 16 Jan 2026 13:04:17 +0100 Subject: [PATCH 1/3] feat: add confirmation prompt to push command - Shows test target name before pushing - Adds -y/--yes flag to skip confirmation - Prevents accidental pushes to wrong test targets --- .DS_Store | Bin 0 -> 6148 bytes src/cli.ts | 1 + src/helpers.ts | 5 +++ src/tools/test-targets.ts | 24 +++++++++++-- tests/tools/test-targets.spec.ts | 59 ++++++++++++++++++++++++++++++- 5 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..388900b8ca12543ff84adddeb4272550439ca4dd GIT binary patch literal 6148 zcmeHKQA+|r5S~@b6A2^=d_3q?>BT}T-R3)>Gaqu#Vz**TR->PJa$q!OaAhaoptQ4-2&TTYU2 zta3d)V3jPZTdB@w$IV8~K5|;~nmubBI%qeV^SNd1?(LtPcOT4EiTLu^K zg2s>b`s9t0SSEdpU(B_bgOM3v2AF|$V8GmMR(TyZ!+T%`n1NqqfbItymC!YqX;fDS zHuU>Q;{`$zwCOEDXd845W*Tt>Md(ySohr-|L+Et$+a}I6m}%7MAk@k@k6Brm7m84; zqu*BHAY6^yG6T%OG6Q+ttx)|x`~LmEoWwn5fEidT21KFbb=tTkQ(IRyN43^Qy+b9T oxJ=_m3L2^uV=R^8DykCn+hicR1~ZN5LE#?(MFTg?z@IYk4uuR;EdT%j literal 0 HcmV?d00001 diff --git a/src/cli.ts b/src/cli.ts index eb22c4c..c451950 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -405,6 +405,7 @@ export const buildCmd = (): CompletableCommand => { .description("Push local YAML test cases to the test target") .helpGroup("test-cases") .addOption(testTargetIdOption) + .option("-y, --yes", "Skip confirmation prompt") .action(addTestTargetWrapper(pushTestTarget)); // noinspection RequiredAttributes diff --git a/src/helpers.ts b/src/helpers.ts index 11b7b80..f3f4388 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -17,6 +17,11 @@ export function promptUser(question: string): Promise { }); } +export async function confirmAction(message: string): Promise { + const answer = await promptUser(`${message} (y/N): `); + return answer.toLowerCase() === "y" || answer.toLowerCase() === "yes"; +} + export const resolveTestTargetId = async ( providedTestTargetId?: string, ): Promise => { diff --git a/src/tools/test-targets.ts b/src/tools/test-targets.ts index 12fe8a1..70cdac9 100644 --- a/src/tools/test-targets.ts +++ b/src/tools/test-targets.ts @@ -3,7 +3,7 @@ import path from "path"; import ora from "ora"; import { OCTOMIND_FOLDER_NAME } from "../constants"; -import { findOctomindFolder } from "../helpers"; +import { confirmAction, findOctomindFolder } from "../helpers"; import { getUrl } from "../url"; import { client, handleError, ListOptions, logJson } from "./client"; import { push } from "./sync/push"; @@ -21,6 +21,13 @@ export const getTestTargets = async () => { return data; }; +export const getTestTargetById = async ( + testTargetId: string, +): Promise<{ id: string; app: string } | undefined> => { + const testTargets = await getTestTargets(); + return testTargets.find((t) => t.id === testTargetId); +}; + export const listTestTargets = async (options: ListOptions): Promise => { const testTargets = await getTestTargets(); if (options.json) { @@ -81,7 +88,7 @@ export const pullTestTarget = async ( }; export const pushTestTarget = async ( - options: { testTargetId: string } & ListOptions, + options: { testTargetId: string; yes?: boolean } & ListOptions, ): Promise => { const sourceDir = await findOctomindFolder(); if (!sourceDir) { @@ -91,6 +98,19 @@ export const pushTestTarget = async ( } const throbber = ora("Pushing test cases").start(); + const testTarget = await getTestTargetById(options.testTargetId); + const testTargetName = testTarget?.app ?? options.testTargetId; + + if (!options.yes) { + const confirmed = await confirmAction( + `Push local changes to test target "${testTargetName}"?`, + ); + if (!confirmed) { + console.log("Push cancelled."); + return; + } + } + const data = await push({ ...options, sourceDir, diff --git a/tests/tools/test-targets.spec.ts b/tests/tools/test-targets.spec.ts index c44b312..5bc5d76 100644 --- a/tests/tools/test-targets.spec.ts +++ b/tests/tools/test-targets.spec.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { mock } from "vitest-mock-extended"; -import { findOctomindFolder } from "../../src/helpers"; +import { confirmAction, findOctomindFolder } from "../../src/helpers"; import { pushTestTarget } from "../../src/tools"; import { client } from "../../src/tools/client"; import { getGitContext } from "../../src/tools/sync/git"; @@ -15,6 +15,7 @@ vi.mock("../../src/tools/client"); describe("push", () => { beforeEach(() => { vi.mocked(findOctomindFolder).mockResolvedValue("/project/.octomind"); + vi.mocked(confirmAction).mockResolvedValue(true); vi.mocked(getGitContext).mockResolvedValue({ defaultBranch: "refs/heads/main", ref: "refs/heads/main", @@ -24,6 +25,11 @@ describe("push", () => { }); vi.mocked(readTestCasesFromDir).mockReturnValue([]); + vi.mocked(client).GET.mockResolvedValue({ + data: [{ id: "someId", app: "My Test App" }], + error: undefined, + response: mock(), + }); vi.mocked(client).POST.mockResolvedValue({ data: undefined, error: undefined, @@ -43,6 +49,7 @@ describe("push", () => { await pushTestTarget({ testTargetId: "someId", + yes: true, }); expect(client.POST).toHaveBeenCalledWith( @@ -62,6 +69,7 @@ describe("push", () => { await pushTestTarget({ testTargetId: "someId", + yes: true, }); expect(client.POST).toHaveBeenCalledWith( @@ -69,4 +77,53 @@ describe("push", () => { expect.anything(), ); }); + + describe("confirmation", () => { + it("prompts for confirmation with test target name", async () => { + await pushTestTarget({ + testTargetId: "someId", + }); + + expect(confirmAction).toHaveBeenCalledWith( + 'Push local changes to test target "My Test App"?', + ); + }); + + it("skips confirmation when --yes flag is provided", async () => { + await pushTestTarget({ + testTargetId: "someId", + yes: true, + }); + + expect(confirmAction).not.toHaveBeenCalled(); + expect(client.POST).toHaveBeenCalled(); + }); + + it("does not push when user declines confirmation", async () => { + vi.mocked(confirmAction).mockResolvedValue(false); + + await pushTestTarget({ + testTargetId: "someId", + }); + + expect(client.POST).not.toHaveBeenCalled(); + expect(console.log).toHaveBeenCalledWith("Push cancelled."); + }); + + it("falls back to test target ID if name not found", async () => { + vi.mocked(client).GET.mockResolvedValue({ + data: [], + error: undefined, + response: mock(), + }); + + await pushTestTarget({ + testTargetId: "unknownId", + }); + + expect(confirmAction).toHaveBeenCalledWith( + 'Push local changes to test target "unknownId"?', + ); + }); + }); }); From 8c5dc5a86f3bc6d4a4f61c664de6b255a96e28b9 Mon Sep 17 00:00:00 2001 From: Daniel Draper Date: Fri, 16 Jan 2026 13:09:17 +0100 Subject: [PATCH 2/3] fix wrong commit --- .DS_Store | Bin 6148 -> 0 bytes .gitignore | 1 + 2 files changed, 1 insertion(+) delete mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index 388900b8ca12543ff84adddeb4272550439ca4dd..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKQA+|r5S~@b6A2^=d_3q?>BT}T-R3)>Gaqu#Vz**TR->PJa$q!OaAhaoptQ4-2&TTYU2 zta3d)V3jPZTdB@w$IV8~K5|;~nmubBI%qeV^SNd1?(LtPcOT4EiTLu^K zg2s>b`s9t0SSEdpU(B_bgOM3v2AF|$V8GmMR(TyZ!+T%`n1NqqfbItymC!YqX;fDS zHuU>Q;{`$zwCOEDXd845W*Tt>Md(ySohr-|L+Et$+a}I6m}%7MAk@k@k6Brm7m84; zqu*BHAY6^yG6T%OG6Q+ttx)|x`~LmEoWwn5fEidT21KFbb=tTkQ(IRyN43^Qy+b9T oxJ=_m3L2^uV=R^8DykCn+hicR1~ZN5LE#?(MFTg?z@IYk4uuR;EdT%j diff --git a/.gitignore b/.gitignore index 6d80c54..d8211cd 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ octomind-cli-debug/ vhs/ .octomind src/schemas +**/.DS_Store From ecb6970c9e3c09eb28e59d5b30f342e131996aa5 Mon Sep 17 00:00:00 2001 From: Daniel Draper Date: Fri, 16 Jan 2026 13:22:34 +0100 Subject: [PATCH 3/3] 2 spinners and correct call to get test target --- src/helpers.ts | 4 ++-- src/tools/test-targets.ts | 38 +++++++++++++++++++++++--------- tests/helpers.spec.ts | 2 +- tests/tools/test-targets.spec.ts | 27 ++++++++--------------- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/helpers.ts b/src/helpers.ts index f3f4388..c8bb65d 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -17,10 +17,10 @@ export function promptUser(question: string): Promise { }); } -export async function confirmAction(message: string): Promise { +export const confirmAction = async (message: string): Promise => { const answer = await promptUser(`${message} (y/N): `); return answer.toLowerCase() === "y" || answer.toLowerCase() === "yes"; -} +}; export const resolveTestTargetId = async ( providedTestTargetId?: string, diff --git a/src/tools/test-targets.ts b/src/tools/test-targets.ts index 70cdac9..614a807 100644 --- a/src/tools/test-targets.ts +++ b/src/tools/test-targets.ts @@ -7,7 +7,7 @@ import { confirmAction, findOctomindFolder } from "../helpers"; import { getUrl } from "../url"; import { client, handleError, ListOptions, logJson } from "./client"; import { push } from "./sync/push"; -import { readTestCasesFromDir, writeYaml } from "./sync/yaml"; +import { writeYaml } from "./sync/yaml"; export const getTestTargets = async () => { const { data, error } = await client.GET("/apiKey/v3/test-targets"); @@ -21,11 +21,25 @@ export const getTestTargets = async () => { return data; }; -export const getTestTargetById = async ( - testTargetId: string, -): Promise<{ id: string; app: string } | undefined> => { - const testTargets = await getTestTargets(); - return testTargets.find((t) => t.id === testTargetId); +export const getTestTarget = async (id: string) => { + const { data, error } = await client.GET( + "/apiKey/v3/test-targets/{testTargetId}", + { + params: { + path: { + testTargetId: id, + }, + }, + }, + ); + + handleError(error); + + if (!data) { + throw Error(`No test target with id ${id} found`); + } + + return data; }; export const listTestTargets = async (options: ListOptions): Promise => { @@ -90,20 +104,21 @@ export const pullTestTarget = async ( export const pushTestTarget = async ( options: { testTargetId: string; yes?: boolean } & ListOptions, ): Promise => { + const localThrobber = ora("Reading local test cases").start(); const sourceDir = await findOctomindFolder(); if (!sourceDir) { throw new Error( `No ${OCTOMIND_FOLDER_NAME} folder found, please pull first.`, ); } - const throbber = ora("Pushing test cases").start(); - const testTarget = await getTestTargetById(options.testTargetId); - const testTargetName = testTarget?.app ?? options.testTargetId; + const testTarget = await getTestTarget(options.testTargetId); + + localThrobber.succeed("Local test cases read successfully"); if (!options.yes) { const confirmed = await confirmAction( - `Push local changes to test target "${testTargetName}"?`, + `Push local changes to test target "${testTarget.app}" with id "${testTarget.id}"?`, ); if (!confirmed) { console.log("Push cancelled."); @@ -111,6 +126,7 @@ export const pushTestTarget = async ( } } + const pushThrobber = ora("Pushing test cases").start(); const data = await push({ ...options, sourceDir, @@ -123,5 +139,5 @@ export const pushTestTarget = async ( logJson(data); } - throbber.succeed("Test cases pushed successfully"); + pushThrobber.succeed("Test cases pushed successfully"); }; diff --git a/tests/helpers.spec.ts b/tests/helpers.spec.ts index cd1c92a..65c8077 100644 --- a/tests/helpers.spec.ts +++ b/tests/helpers.spec.ts @@ -2,7 +2,7 @@ import fsPromises from "fs/promises"; import os from "os"; import path from "path"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { OCTOMIND_FOLDER_NAME } from "../src/constants"; import { diff --git a/tests/tools/test-targets.spec.ts b/tests/tools/test-targets.spec.ts index 5bc5d76..92e0bcb 100644 --- a/tests/tools/test-targets.spec.ts +++ b/tests/tools/test-targets.spec.ts @@ -26,7 +26,7 @@ describe("push", () => { vi.mocked(readTestCasesFromDir).mockReturnValue([]); vi.mocked(client).GET.mockResolvedValue({ - data: [{ id: "someId", app: "My Test App" }], + data: { id: "someId", app: "My Test App" }, error: undefined, response: mock(), }); @@ -80,12 +80,19 @@ describe("push", () => { describe("confirmation", () => { it("prompts for confirmation with test target name", async () => { + const id = "someId"; + const name = "My Test App"; + vi.mocked(client).GET.mockResolvedValue({ + data: { id, app: name }, + error: undefined, + response: mock(), + }); await pushTestTarget({ testTargetId: "someId", }); expect(confirmAction).toHaveBeenCalledWith( - 'Push local changes to test target "My Test App"?', + `Push local changes to test target "${name}" with id "${id}"?`, ); }); @@ -109,21 +116,5 @@ describe("push", () => { expect(client.POST).not.toHaveBeenCalled(); expect(console.log).toHaveBeenCalledWith("Push cancelled."); }); - - it("falls back to test target ID if name not found", async () => { - vi.mocked(client).GET.mockResolvedValue({ - data: [], - error: undefined, - response: mock(), - }); - - await pushTestTarget({ - testTargetId: "unknownId", - }); - - expect(confirmAction).toHaveBeenCalledWith( - 'Push local changes to test target "unknownId"?', - ); - }); }); });