Add skills and fix race condition (#27)

* chore: trigger CI

* feat: add skills (#24)

* feat: add skills

* feat: remove comments

* feat: improve prompt

* Fix/webhook polling race conditions (#26)

* fix: resolve webhook/polling race conditions with timestamped claims

* refactor: dispatch/webhook/polling handlers for readability

---------

Co-authored-by: woywro <woywro@gmail.com>

Author: kasin-it

scripts/test-sandbox-skills.ts

@@ -0,0 +1,83 @@
+/**
+ * Debug script: provisions a bare sandbox, installs skills globally, and dumps the results.
+ * Uses Vercel OIDC for sandbox auth — no repo needed.
+ *
+ * Usage:
+ *   npx tsx scripts/test-sandbox-skills.ts
+ */
+
+import { Sandbox } from "@vercel/sandbox";
+
+const INJECTED_SKILLS = [
+  { repo: "https://github.com/obra/superpowers", skill: "using-superpowers" },
+  { repo: "https://github.com/obra/superpowers", skill: "requesting-code-review" },
+  { repo: "https://github.com/anthropics/skills", skill: "frontend-design" },
+];
+
+async function main() {
+  console.log("\n=== Provisioning sandbox ===\n");
+
+  const sandbox = await Sandbox.create({
+    runtime: "node24",
+    timeout: 300_000,
+  });
+
+  console.log("Sandbox created.\n");
+
+  await sandbox.runCommand("bash", ["-c", "git init && git commit --allow-empty -m 'init'"]);
+
+  console.log("=== Installing Claude Code ===");
+  const installCC = await sandbox.runCommand("npm", [
+    "install",
+    "-g",
+    "@anthropic-ai/claude-code",
+  ]);
+  console.log("stdout:", (await installCC.stdout()).slice(-200));
+  console.log("stderr:", (await installCC.stderr()).slice(-200));
+
+  for (const { repo, skill } of INJECTED_SKILLS) {
+    console.log(`\n=== Installing skill globally: ${skill} ===`);
+    const result = await sandbox.runCommand("npx", [
+      "-y", "skills", "add", repo, "--skill", skill, "--yes", "-g",
+    ]);
+    console.log("stdout:", (await result.stdout()).slice(-300) || "(empty)");
+    console.log("stderr:", (await result.stderr()).slice(-300) || "(empty)");
+  }
+
+  console.log("\n=== .claude/skills/ (project) ===");
+  const projectSkills = await sandbox.runCommand("bash", [
+    "-c",
+    "ls -laR .claude/skills/ 2>/dev/null || echo '(directory does not exist)'",
+  ]);
+  console.log(await projectSkills.stdout());
+
+  console.log("=== skills-lock.json (project) ===");
+  const lock = await sandbox.runCommand("bash", [
+    "-c",
+    "cat skills-lock.json 2>/dev/null || echo '(file does not exist)'",
+  ]);
+  console.log(await lock.stdout());
+
+  console.log("=== ~/.claude/skills/ (global) ===");
+  const globalSkills = await sandbox.runCommand("bash", [
+    "-c",
+    "ls -laR ~/.claude/skills/ 2>/dev/null || echo '(directory does not exist)'",
+  ]);
+  console.log(await globalSkills.stdout());
+
+  console.log("=== Find all SKILL.md files (everywhere) ===");
+  const skillFiles = await sandbox.runCommand("bash", [
+    "-c",
+    "find / -name 'SKILL.md' -not -path '*/node_modules/*' 2>/dev/null || echo '(none found)'",
+  ]);
+  console.log(await skillFiles.stdout());
+
+  console.log("\n=== Stopping sandbox ===");
+  await sandbox.stop();
+  console.log("Done.");
+}
+
+main().catch((err) => {
+  console.error("Fatal:", err);
+  process.exit(1);
+});

src/lib/cancel-run.test.ts

@@ -0,0 +1,63 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import type { RunRegistryAdapter } from "../adapters/run-registry/types.js";
+
+const mockGetRun = vi.fn();
+vi.mock("workflow/api", () => ({
+  getRun: (...args: any[]) => mockGetRun(...args),
+}));
+
+function makeRegistry(overrides: Partial<RunRegistryAdapter> = {}): RunRegistryAdapter {
+  return {
+    claim: vi.fn(),
+    register: vi.fn(),
+    getRunId: vi.fn(),
+    unregister: overrides.unregister ?? vi.fn().mockResolvedValue(undefined),
+    listAll: vi.fn(),
+  };
+}
+
+describe("cancelRun", () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it("cancels the run and unregisters", async () => {
+    const mockCancel = vi.fn().mockResolvedValue(undefined);
+    mockGetRun.mockReturnValue({ cancel: mockCancel });
+    const registry = makeRegistry();
+
+    const { cancelRun } = await import("./cancel-run.js");
+    const result = await cancelRun("PROJ-1", "run_abc", registry);
+
+    expect(result).toBe(true);
+    expect(mockGetRun).toHaveBeenCalledWith("run_abc");
+    expect(mockCancel).toHaveBeenCalled();
+    expect(registry.unregister).toHaveBeenCalledWith("PROJ-1");
+  });
+
+  it("returns false and still unregisters when cancel throws", async () => {
+    mockGetRun.mockReturnValue({
+      cancel: vi.fn().mockRejectedValue(new Error("run gone")),
+    });
+    const registry = makeRegistry();
+
+    const { cancelRun } = await import("./cancel-run.js");
+    const result = await cancelRun("PROJ-1", "run_abc", registry);
+
+    expect(result).toBe(false);
+    expect(registry.unregister).toHaveBeenCalledWith("PROJ-1");
+  });
+
+  it("is idempotent — second call on same ticket returns false without throwing", async () => {
+    mockGetRun.mockReturnValue({
+      cancel: vi.fn().mockRejectedValue(new Error("already cancelled")),
+    });
+    const unregister = vi.fn().mockResolvedValue(undefined);
+    const registry = makeRegistry({ unregister });
+
+    const { cancelRun } = await import("./cancel-run.js");
+    await cancelRun("PROJ-1", "run_abc", registry);
+    const result = await cancelRun("PROJ-1", "run_abc", registry);
+
+    expect(result).toBe(false);
+    expect(unregister).toHaveBeenCalledTimes(2);
+  });
+});

src/lib/cancel-run.ts

@@ -0,0 +1,29 @@
+import { getRun } from "workflow/api";
+import { logger } from "./logger.js";
+import type { RunRegistryAdapter } from "../adapters/run-registry/types.js";
+
+/**
+ * Cancel a workflow run and unregister it from the registry.
+ * Idempotent: safe to call multiple times for the same ticket.
+ * Returns true if cancel succeeded, false if it errored (still unregisters).
+ */
+export async function cancelRun(
+  ticketKey: string,
+  runId: string,
+  runRegistry: RunRegistryAdapter,
+): Promise<boolean> {
+  let cancelled = false;
+  try {
+    const run = getRun(runId);
+    await run.cancel();
+    cancelled = true;
+  } catch (err) {
+    logger.warn(
+      { ticketKey, runId, error: (err as Error).message },
+      "cancel_run_error",
+    );
+  }
+
+  await runRegistry.unregister(ticketKey).catch(() => {});
+  return cancelled;
+}

src/lib/dispatch.test.ts

@@ -2,10 +2,11 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
 import type { Adapters } from "./adapters.js";
 import type { TicketContent } from "../adapters/issue-tracker/types.js";
 
-
 const mockStart = vi.fn();
+const mockGetRun = vi.fn();
 vi.mock("workflow/api", () => ({
   start: (...args: any[]) => mockStart(...args),
+  getRun: (...args: any[]) => mockGetRun(...args),
 }));
 
 vi.mock("../workflows/implementation.js", () => ({
@@ -23,7 +24,6 @@ vi.mock("@vercel/sandbox", () => ({
   },
 }));
 
-
 function makeTicket(overrides: Partial<TicketContent> = {}): TicketContent {
   return {
     id: "ticket-001",
@@ -38,16 +38,22 @@ function makeTicket(overrides: Partial<TicketContent> = {}): TicketContent {
   };
 }
 
-function makeAdapters(overrides: Partial<{
-  claim: ReturnType<typeof vi.fn>;
-  register: ReturnType<typeof vi.fn>;
-  unregister: ReturnType<typeof vi.fn>;
-  fetchTicket: ReturnType<typeof vi.fn>;
-  findPR: ReturnType<typeof vi.fn>;
-}>= {}): Adapters {
+function makeAdapters(
+  overrides: Partial<{
+    claim: ReturnType<typeof vi.fn>;
+    register: ReturnType<typeof vi.fn>;
+    unregister: ReturnType<typeof vi.fn>;
+    getRunId: ReturnType<typeof vi.fn>;
+    fetchTicket: ReturnType<typeof vi.fn>;
+    findPR: ReturnType<typeof vi.fn>;
+  }> = {},
+): Adapters {
+  let claimedValue: string | undefined;
+
   return {
     issueTracker: {
-      fetchTicket: overrides.fetchTicket ?? vi.fn().mockResolvedValue(makeTicket()),
+      fetchTicket:
+        overrides.fetchTicket ?? vi.fn().mockResolvedValue(makeTicket()),
       moveTicket: vi.fn(),
       postComment: vi.fn(),
       searchTickets: vi.fn(),
@@ -64,16 +70,22 @@ function makeAdapters(overrides: Partial<{
       notify: vi.fn(),
     },
     runRegistry: {
-      claim: overrides.claim ?? vi.fn().mockResolvedValue(true),
+      claim:
+        overrides.claim ??
+        vi.fn().mockImplementation(async (_key: string, value: string) => {
+          claimedValue = value;
+          return true;
+        }),
       register: overrides.register ?? vi.fn().mockResolvedValue(undefined),
       unregister: overrides.unregister ?? vi.fn().mockResolvedValue(undefined),
-      getRunId: vi.fn(),
+      getRunId:
+        overrides.getRunId ??
+        vi.fn().mockImplementation(async () => claimedValue),
       listAll: vi.fn(),
     },
   };
 }
 
-
 describe("dispatchTicket", () => {
   beforeEach(() => {
     vi.clearAllMocks();
@@ -90,24 +102,44 @@ describe("dispatchTicket", () => {
     const result = await dispatchTicket("PROJ-42", adapters, 5);
 
     expect(result).toEqual({ started: true, runId: "run_123" });
-    expect(adapters.runRegistry.claim).toHaveBeenCalledWith("PROJ-42", "claiming");
+    expect(adapters.runRegistry.claim).toHaveBeenCalledWith(
+      "PROJ-42",
+      expect.stringMatching(/^claiming:\d+$/),
+    );
     expect(adapters.issueTracker.fetchTicket).toHaveBeenCalledWith("PROJ-42");
     expect(adapters.vcs.findPR).toHaveBeenCalledWith("blazebot/proj-42");
-    expect(mockStart).toHaveBeenCalledWith("implementationWorkflow_sentinel", ["ticket-001"]);
-    expect(adapters.runRegistry.register).toHaveBeenCalledWith("PROJ-42", "run_123");
+    expect(mockStart).toHaveBeenCalledWith("implementationWorkflow_sentinel", [
+      "ticket-001",
+    ]);
+    expect(adapters.runRegistry.register).toHaveBeenCalledWith(
+      "PROJ-42",
+      "run_123",
+    );
   });
 
   it("dispatches review-fix workflow when PR exists", async () => {
     const adapters = makeAdapters({
-      findPR: vi.fn().mockResolvedValue({ id: 7, url: "https://github.com/pr/7", branch: "blazebot/proj-42" }),
+      findPR: vi
+        .fn()
+        .mockResolvedValue({
+          id: 7,
+          url: "https://github.com/pr/7",
+          branch: "blazebot/proj-42",
+        }),
     });
     const { dispatchTicket } = await import("./dispatch.js");
 
     const result = await dispatchTicket("PROJ-42", adapters, 5);
 
     expect(result).toEqual({ started: true, runId: "run_123" });
-    expect(mockStart).toHaveBeenCalledWith("reviewFixWorkflow_sentinel", ["ticket-001", "blazebot/proj-42"]);
-    expect(adapters.runRegistry.register).toHaveBeenCalledWith("PROJ-42", "run_123");
+    expect(mockStart).toHaveBeenCalledWith("reviewFixWorkflow_sentinel", [
+      "ticket-001",
+      "blazebot/proj-42",
+    ]);
+    expect(adapters.runRegistry.register).toHaveBeenCalledWith(
+      "PROJ-42",
+      "run_123",
+    );
   });
 
   it("returns already_claimed when claim fails", async () => {
@@ -143,6 +175,25 @@ describe("dispatchTicket", () => {
     expect(mockStart).not.toHaveBeenCalled();
   });
 
+  it("aborts workflow if claim was removed during dispatch", async () => {
+    const mockCancel = vi.fn().mockResolvedValue(undefined);
+    mockGetRun.mockReturnValue({ cancel: mockCancel });
+
+    // getRunId returns null — claim was removed by a cancel while workflow was starting
+    const adapters = makeAdapters({
+      getRunId: vi.fn().mockResolvedValue(null),
+    });
+    const { dispatchTicket } = await import("./dispatch.js");
+
+    const result = await dispatchTicket("PROJ-42", adapters, 5);
+
+    expect(result).toEqual({ started: false, reason: "already_claimed" });
+    expect(mockStart).toHaveBeenCalled();
+    expect(mockGetRun).toHaveBeenCalledWith("run_123");
+    expect(mockCancel).toHaveBeenCalled();
+    expect(adapters.runRegistry.register).not.toHaveBeenCalled();
+  });
+
   it("unregisters claim and returns error on dispatch failure", async () => {
     const unregister = vi.fn().mockResolvedValue(undefined);
     const adapters = makeAdapters({