refactor: dedupe extension and ui helpers

2026-03-12 07:20:45 +00:00 · 2026-03-02 19:48:50 +00:00
parent b1c30f0ba9
commit eb816e0551
7 changed files with 170 additions and 226 deletions
--- a/.pi/extensions/prompt-url-widget.ts
+++ b/.pi/extensions/prompt-url-widget.ts
@@ -114,6 +114,17 @@ export default function promptUrlWidgetExtension(pi: ExtensionAPI) {
    }
  };

+  const renderPromptMatch = (ctx: ExtensionContext, match: PromptMatch) => {
+    setWidget(ctx, match);
+    applySessionName(ctx, match);
+    void fetchGhMetadata(pi, match.kind, match.url).then((meta) => {
+      const title = meta?.title?.trim();
+      const authorText = formatAuthor(meta?.author);
+      setWidget(ctx, match, title, authorText);
+      applySessionName(ctx, match, title);
+    });
+  };
+
  pi.on("before_agent_start", async (event, ctx) => {
    if (!ctx.hasUI) {
      return;
@@ -123,14 +134,7 @@ export default function promptUrlWidgetExtension(pi: ExtensionAPI) {
      return;
    }

-    setWidget(ctx, match);
-    applySessionName(ctx, match);
-    void fetchGhMetadata(pi, match.kind, match.url).then((meta) => {
-      const title = meta?.title?.trim();
-      const authorText = formatAuthor(meta?.author);
-      setWidget(ctx, match, title, authorText);
-      applySessionName(ctx, match, title);
-    });
+    renderPromptMatch(ctx, match);
  });

  pi.on("session_switch", async (_event, ctx) => {
@@ -177,14 +181,7 @@ export default function promptUrlWidgetExtension(pi: ExtensionAPI) {
      return;
    }

-    setWidget(ctx, match);
-    applySessionName(ctx, match);
-    void fetchGhMetadata(pi, match.kind, match.url).then((meta) => {
-      const title = meta?.title?.trim();
-      const authorText = formatAuthor(meta?.author);
-      setWidget(ctx, match, title, authorText);
-      applySessionName(ctx, match, title);
-    });
+    renderPromptMatch(ctx, match);
  };

  pi.on("session_start", async (_event, ctx) => {
--- a/extensions/acpx/src/ensure.ts
+++ b/extensions/acpx/src/ensure.ts
@@ -76,6 +76,28 @@ function resolveVersionFromPackage(command: string, cwd: string): string | null
  }
 }

+function resolveVersionCheckResult(params: {
+  expectedVersion?: string;
+  installedVersion: string;
+  installCommand: string;
+}): AcpxVersionCheckResult {
+  if (params.expectedVersion && params.installedVersion !== params.expectedVersion) {
+    return {
+      ok: false,
+      reason: "version-mismatch",
+      message: `acpx version mismatch: found ${params.installedVersion}, expected ${params.expectedVersion}`,
+      expectedVersion: params.expectedVersion,
+      installCommand: params.installCommand,
+      installedVersion: params.installedVersion,
+    };
+  }
+  return {
+    ok: true,
+    version: params.installedVersion,
+    expectedVersion: params.expectedVersion,
+  };
+}
+
 export async function checkAcpxVersion(params: {
  command: string;
  cwd?: string;
@@ -131,21 +153,7 @@ export async function checkAcpxVersion(params: {
    if (hasExpectedVersion && isUnsupportedVersionProbe(result.stdout, result.stderr)) {
      const installedVersion = resolveVersionFromPackage(params.command, cwd);
      if (installedVersion) {
-        if (expectedVersion && installedVersion !== expectedVersion) {
-          return {
-            ok: false,
-            reason: "version-mismatch",
-            message: `acpx version mismatch: found ${installedVersion}, expected ${expectedVersion}`,
-            expectedVersion,
-            installCommand,
-            installedVersion,
-          };
-        }
-        return {
-          ok: true,
-          version: installedVersion,
-          expectedVersion,
-        };
+        return resolveVersionCheckResult({ expectedVersion, installedVersion, installCommand });
      }
    }
    const stderr = result.stderr.trim();
@@ -179,22 +187,7 @@ export async function checkAcpxVersion(params: {
    };
  }

-  if (expectedVersion && installedVersion !== expectedVersion) {
-    return {
-      ok: false,
-      reason: "version-mismatch",
-      message: `acpx version mismatch: found ${installedVersion}, expected ${expectedVersion}`,
-      expectedVersion,
-      installCommand,
-      installedVersion,
-    };
-  }
-
-  return {
-    ok: true,
-    version: installedVersion,
-    expectedVersion,
-  };
+  return resolveVersionCheckResult({ expectedVersion, installedVersion, installCommand });
 }

 let pendingEnsure: Promise<void> | null = null;
--- a/scripts/label-open-issues.ts
+++ b/scripts/label-open-issues.ts
@@ -182,6 +182,12 @@ type LoadedState = {
 };

 type LabelTarget = "issue" | "pr";
+type LabelItemBatch = {
+  batchIndex: number;
+  items: LabelItem[];
+  totalCount: number;
+  fetchedCount: number;
+};

 function parseArgs(argv: string[]): ScriptOptions {
  let limit = Number.POSITIVE_INFINITY;
@@ -408,9 +414,22 @@ function fetchPullRequestPage(repo: RepoInfo, after: string | null): PullRequest
  return pullRequests;
 }

-function* fetchOpenIssueBatches(limit: number): Generator<IssueBatch> {
+function mapNodeToLabelItem(node: IssuePage["nodes"][number]): LabelItem {
+  return {
+    number: node.number,
+    title: node.title,
+    body: node.body ?? "",
+    labels: node.labels?.nodes ?? [],
+  };
+}
+
+function* fetchOpenLabelItemBatches(params: {
+  limit: number;
+  kindPlural: "issues" | "pull requests";
+  fetchPage: (repo: RepoInfo, after: string | null) => IssuePage | PullRequestPage;
+}): Generator<LabelItemBatch> {
  const repo = resolveRepo();
-  const results: Issue[] = [];
+  const results: LabelItem[] = [];
  let page = 1;
  let after: string | null = null;
  let totalCount = 0;
@@ -419,33 +438,28 @@ function* fetchOpenIssueBatches(limit: number): Generator<IssueBatch> {

  logStep(`Repository: ${repo.owner}/${repo.name}`);

-  while (fetchedCount < limit) {
-    const pageData = fetchIssuePage(repo, after);
+  while (fetchedCount < params.limit) {
+    const pageData = params.fetchPage(repo, after);
    const nodes = pageData.nodes ?? [];
    totalCount = pageData.totalCount ?? totalCount;

    if (page === 1) {
-      logSuccess(`Found ${totalCount} open issues.`);
+      logSuccess(`Found ${totalCount} open ${params.kindPlural}.`);
    }

-    logInfo(`Fetched page ${page} (${nodes.length} issues).`);
+    logInfo(`Fetched page ${page} (${nodes.length} ${params.kindPlural}).`);

    for (const node of nodes) {
-      if (fetchedCount >= limit) {
+      if (fetchedCount >= params.limit) {
        break;
      }
-      results.push({
-        number: node.number,
-        title: node.title,
-        body: node.body ?? "",
-        labels: node.labels?.nodes ?? [],
-      });
+      results.push(mapNodeToLabelItem(node));
      fetchedCount += 1;

      if (results.length >= WORK_BATCH_SIZE) {
        yield {
          batchIndex,
-          issues: results.splice(0, results.length),
+          items: results.splice(0, results.length),
          totalCount,
          fetchedCount,
        };
@@ -464,72 +478,39 @@ function* fetchOpenIssueBatches(limit: number): Generator<IssueBatch> {
  if (results.length) {
    yield {
      batchIndex,
-      issues: results,
+      items: results,
      totalCount,
      fetchedCount,
    };
  }
 }

-function* fetchOpenPullRequestBatches(limit: number): Generator<PullRequestBatch> {
-  const repo = resolveRepo();
-  const results: PullRequest[] = [];
-  let page = 1;
-  let after: string | null = null;
-  let totalCount = 0;
-  let fetchedCount = 0;
-  let batchIndex = 1;
-
-  logStep(`Repository: ${repo.owner}/${repo.name}`);
-
-  while (fetchedCount < limit) {
-    const pageData = fetchPullRequestPage(repo, after);
-    const nodes = pageData.nodes ?? [];
-    totalCount = pageData.totalCount ?? totalCount;
-
-    if (page === 1) {
-      logSuccess(`Found ${totalCount} open pull requests.`);
-    }
-
-    logInfo(`Fetched page ${page} (${nodes.length} pull requests).`);
-
-    for (const node of nodes) {
-      if (fetchedCount >= limit) {
-        break;
-      }
-      results.push({
-        number: node.number,
-        title: node.title,
-        body: node.body ?? "",
-        labels: node.labels?.nodes ?? [],
-      });
-      fetchedCount += 1;
-
-      if (results.length >= WORK_BATCH_SIZE) {
-        yield {
-          batchIndex,
-          pullRequests: results.splice(0, results.length),
-          totalCount,
-          fetchedCount,
-        };
-        batchIndex += 1;
-      }
-    }
-
-    if (!pageData.pageInfo.hasNextPage) {
-      break;
-    }
-
-    after = pageData.pageInfo.endCursor ?? null;
-    page += 1;
-  }
-
-  if (results.length) {
+function* fetchOpenIssueBatches(limit: number): Generator<IssueBatch> {
+  for (const batch of fetchOpenLabelItemBatches({
+    limit,
+    kindPlural: "issues",
+    fetchPage: fetchIssuePage,
+  })) {
    yield {
-      batchIndex,
-      pullRequests: results,
-      totalCount,
-      fetchedCount,
+      batchIndex: batch.batchIndex,
+      issues: batch.items,
+      totalCount: batch.totalCount,
+      fetchedCount: batch.fetchedCount,
+    };
+  }
+}
+
+function* fetchOpenPullRequestBatches(limit: number): Generator<PullRequestBatch> {
+  for (const batch of fetchOpenLabelItemBatches({
+    limit,
+    kindPlural: "pull requests",
+    fetchPage: fetchPullRequestPage,
+  })) {
+    yield {
+      batchIndex: batch.batchIndex,
+      pullRequests: batch.items,
+      totalCount: batch.totalCount,
+      fetchedCount: batch.fetchedCount,
    };
  }
 }
--- a/ui/src/ui/controllers/usage.node.test.ts
+++ b/ui/src/ui/controllers/usage.node.test.ts
@@ -26,6 +26,23 @@ function createState(request: RequestFn, overrides: Partial<UsageState> = {}): U
  };
 }

+function expectSpecificTimezoneCalls(request: ReturnType<typeof vi.fn>, startCall: number): void {
+  expect(request).toHaveBeenNthCalledWith(startCall, "sessions.usage", {
+    startDate: "2026-02-16",
+    endDate: "2026-02-16",
+    mode: "specific",
+    utcOffset: "UTC+5:30",
+    limit: 1000,
+    includeContextWeight: true,
+  });
+  expect(request).toHaveBeenNthCalledWith(startCall + 1, "usage.cost", {
+    startDate: "2026-02-16",
+    endDate: "2026-02-16",
+    mode: "specific",
+    utcOffset: "UTC+5:30",
+  });
+}
+
 describe("usage controller date interpretation params", () => {
  beforeEach(() => {
    __test.resetLegacyUsageDateParamsCache();
@@ -48,20 +65,7 @@ describe("usage controller date interpretation params", () => {

    await loadUsage(state);

-    expect(request).toHaveBeenNthCalledWith(1, "sessions.usage", {
-      startDate: "2026-02-16",
-      endDate: "2026-02-16",
-      mode: "specific",
-      utcOffset: "UTC+5:30",
-      limit: 1000,
-      includeContextWeight: true,
-    });
-    expect(request).toHaveBeenNthCalledWith(2, "usage.cost", {
-      startDate: "2026-02-16",
-      endDate: "2026-02-16",
-      mode: "specific",
-      utcOffset: "UTC+5:30",
-    });
+    expectSpecificTimezoneCalls(request, 1);
  });

  it("sends utc mode without offset when usage timezone is utc", async () => {
@@ -124,20 +128,7 @@ describe("usage controller date interpretation params", () => {

    await loadUsage(state);

-    expect(request).toHaveBeenNthCalledWith(1, "sessions.usage", {
-      startDate: "2026-02-16",
-      endDate: "2026-02-16",
-      mode: "specific",
-      utcOffset: "UTC+5:30",
-      limit: 1000,
-      includeContextWeight: true,
-    });
-    expect(request).toHaveBeenNthCalledWith(2, "usage.cost", {
-      startDate: "2026-02-16",
-      endDate: "2026-02-16",
-      mode: "specific",
-      utcOffset: "UTC+5:30",
-    });
+    expectSpecificTimezoneCalls(request, 1);
    expect(request).toHaveBeenNthCalledWith(3, "sessions.usage", {
      startDate: "2026-02-16",
      endDate: "2026-02-16",
--- a/ui/src/ui/tool-display.ts
+++ b/ui/src/ui/tool-display.ts
@@ -3,8 +3,7 @@ import {
  defaultTitle,
  formatToolDetailText,
  normalizeToolName,
-  resolveActionArg,
-  resolveToolVerbAndDetail,
+  resolveToolVerbAndDetailForArgs,
  type ToolDisplaySpec as ToolDisplaySpecBase,
 } from "../../../src/agents/tool-display-common.js";
 import type { IconName } from "./icons.ts";
@@ -126,12 +125,10 @@ export function resolveToolDisplay(params: {
  const icon = (spec?.icon ?? FALLBACK.icon ?? "puzzle") as IconName;
  const title = spec?.title ?? defaultTitle(name);
  const label = spec?.label ?? title;
-  const action = resolveActionArg(params.args);
-  let { verb, detail } = resolveToolVerbAndDetail({
+  let { verb, detail } = resolveToolVerbAndDetailForArgs({
    toolKey: key,
    args: params.args,
    meta: params.meta,
-    action,
    spec,
    fallbackDetailKeys: FALLBACK.detailKeys,
    detailMode: "first",
--- a/ui/src/ui/types.ts
+++ b/ui/src/ui/types.ts
@@ -1,4 +1,5 @@
 export type UpdateAvailable = import("../../../src/infra/update-startup.js").UpdateAvailable;
+import type { CronJobBase } from "../../../src/cron/types-shared.js";
 import type { ConfigUiHints } from "../../../src/shared/config-ui-hints-types.js";
 import type {
  GatewayAgentRow as SharedGatewayAgentRow,
@@ -492,22 +493,14 @@ export type CronJobState = {
  lastFailureAlertAtMs?: number;
 };

-export type CronJob = {
-  id: string;
-  agentId?: string;
-  sessionKey?: string;
-  name: string;
-  description?: string;
-  enabled: boolean;
-  deleteAfterRun?: boolean;
-  createdAtMs: number;
-  updatedAtMs: number;
-  schedule: CronSchedule;
-  sessionTarget: CronSessionTarget;
-  wakeMode: CronWakeMode;
-  payload: CronPayload;
-  delivery?: CronDelivery;
-  failureAlert?: CronFailureAlert | false;
+export type CronJob = CronJobBase<
+  CronSchedule,
+  CronSessionTarget,
+  CronWakeMode,
+  CronPayload,
+  CronDelivery,
+  CronFailureAlert | false
+> & {
  state?: CronJobState;
 };

--- a/ui/src/ui/views/usage.ts
+++ b/ui/src/ui/views/usage.ts
@@ -42,6 +42,52 @@ import {

 export type { UsageColumnId, SessionLogEntry, SessionLogRole };

+function createEmptyUsageTotals(): UsageTotals {
+  return {
+    input: 0,
+    output: 0,
+    cacheRead: 0,
+    cacheWrite: 0,
+    totalTokens: 0,
+    totalCost: 0,
+    inputCost: 0,
+    outputCost: 0,
+    cacheReadCost: 0,
+    cacheWriteCost: 0,
+    missingCostEntries: 0,
+  };
+}
+
+function addUsageTotals(
+  acc: UsageTotals,
+  usage: {
+    input: number;
+    output: number;
+    cacheRead: number;
+    cacheWrite: number;
+    totalTokens: number;
+    totalCost: number;
+    inputCost?: number;
+    outputCost?: number;
+    cacheReadCost?: number;
+    cacheWriteCost?: number;
+    missingCostEntries?: number;
+  },
+): UsageTotals {
+  acc.input += usage.input;
+  acc.output += usage.output;
+  acc.cacheRead += usage.cacheRead;
+  acc.cacheWrite += usage.cacheWrite;
+  acc.totalTokens += usage.totalTokens;
+  acc.totalCost += usage.totalCost;
+  acc.inputCost += usage.inputCost ?? 0;
+  acc.outputCost += usage.outputCost ?? 0;
+  acc.cacheReadCost += usage.cacheReadCost ?? 0;
+  acc.cacheWriteCost += usage.cacheWriteCost ?? 0;
+  acc.missingCostEntries += usage.missingCostEntries ?? 0;
+  return acc;
+}
+
 export function renderUsage(props: UsageProps) {
  // Show loading skeleton if loading and no data yet
  if (props.loading && !props.totals) {
@@ -206,69 +252,15 @@ export function renderUsage(props: UsageProps) {
  // Compute totals from sessions
  const computeSessionTotals = (sessions: UsageSessionEntry[]): UsageTotals => {
    return sessions.reduce(
-      (acc, s) => {
-        if (s.usage) {
-          acc.input += s.usage.input;
-          acc.output += s.usage.output;
-          acc.cacheRead += s.usage.cacheRead;
-          acc.cacheWrite += s.usage.cacheWrite;
-          acc.totalTokens += s.usage.totalTokens;
-          acc.totalCost += s.usage.totalCost;
-          acc.inputCost += s.usage.inputCost ?? 0;
-          acc.outputCost += s.usage.outputCost ?? 0;
-          acc.cacheReadCost += s.usage.cacheReadCost ?? 0;
-          acc.cacheWriteCost += s.usage.cacheWriteCost ?? 0;
-          acc.missingCostEntries += s.usage.missingCostEntries ?? 0;
-        }
-        return acc;
-      },
-      {
-        input: 0,
-        output: 0,
-        cacheRead: 0,
-        cacheWrite: 0,
-        totalTokens: 0,
-        totalCost: 0,
-        inputCost: 0,
-        outputCost: 0,
-        cacheReadCost: 0,
-        cacheWriteCost: 0,
-        missingCostEntries: 0,
-      },
+      (acc, s) => (s.usage ? addUsageTotals(acc, s.usage) : acc),
+      createEmptyUsageTotals(),
    );
  };

  // Compute totals from daily data for selected days (more accurate than session totals)
  const computeDailyTotals = (days: string[]): UsageTotals => {
    const matchingDays = props.costDaily.filter((d) => days.includes(d.date));
-    return matchingDays.reduce(
-      (acc, d) => {
-        acc.input += d.input;
-        acc.output += d.output;
-        acc.cacheRead += d.cacheRead;
-        acc.cacheWrite += d.cacheWrite;
-        acc.totalTokens += d.totalTokens;
-        acc.totalCost += d.totalCost;
-        acc.inputCost += d.inputCost ?? 0;
-        acc.outputCost += d.outputCost ?? 0;
-        acc.cacheReadCost += d.cacheReadCost ?? 0;
-        acc.cacheWriteCost += d.cacheWriteCost ?? 0;
-        return acc;
-      },
-      {
-        input: 0,
-        output: 0,
-        cacheRead: 0,
-        cacheWrite: 0,
-        totalTokens: 0,
-        totalCost: 0,
-        inputCost: 0,
-        outputCost: 0,
-        cacheReadCost: 0,
-        cacheWriteCost: 0,
-        missingCostEntries: 0,
-      },
-    );
+    return matchingDays.reduce((acc, day) => addUsageTotals(acc, day), createEmptyUsageTotals());
  };

  // Compute display totals and count based on filters