Deduplicate double-scheduling in github-sync (WL-0MN834ICI00339E7)

src/commands/helpers.ts

@@ -3,6 +3,7 @@
  */
 
 import { theme } from '../theme.js';
+import { redactAuditText } from '../audit.js';
 import type { WorkItem, Comment } from '../types.js';
 import type { SyncResult } from '../sync.js';
 import type { WorklogDatabase } from '../database.js';
@@ -261,7 +262,11 @@ export function humanFormatWorkItem(item: WorkItem, db: WorklogDatabase | null,
     lines.push(`Effort: ${item.effort || '—'}`);
     if (item.assignee) lines.push(`Assignee: ${item.assignee}`);
     if (item.audit) {
-      const firstLine = String(item.audit.text || '').split(/\r?\n/, 1)[0];
+      // For human outputs, show a truncated, redacted one-line audit excerpt.
+      // Do not include the author in concise output to keep it compact.
+      const raw = String(item.audit.text || '');
+      const redacted = redactAuditText(raw);
+      const firstLine = redacted.split(/\r?\n/, 1)[0];
       lines.push(`Audit: ${firstLine}`);
     }
     if (item.tags && item.tags.length > 0) lines.push(`Tags: ${item.tags.join(', ')}`);
@@ -285,7 +290,10 @@ export function humanFormatWorkItem(item: WorkItem, db: WorklogDatabase | null,
     lines.push(`Effort: ${item.effort || '—'}`);
     if (item.assignee) lines.push(`Assignee: ${item.assignee}`);
     if (item.audit) {
-      const firstLine = String(item.audit.text || '').split(/\r?\n/, 1)[0];
+      const raw = String(item.audit.text || '');
+      const redacted = redactAuditText(raw);
+      const firstLine = redacted.split(/\r?\n/, 1)[0];
+      // Keep concise audit excerpt in normal output as well (author omitted).
       lines.push(`Audit: ${firstLine}`);
     }
     if (item.parentId) lines.push(`Parent: ${item.parentId}`);

src/commands/show.ts

@@ -4,6 +4,7 @@
 
 import type { PluginContext } from '../plugin-types.js';
 import type { ShowOptions } from '../cli-types.js';
+import type { WorkItem, Comment, ShowJsonOutput } from '../types.js';
 import { displayItemTree, displayItemTreeWithFormat, humanFormatComment, resolveFormat, humanFormatWorkItem } from './helpers.js';
 
 export default function register(ctx: PluginContext): void {
@@ -26,16 +27,25 @@ export default function register(ctx: PluginContext): void {
       }
 
       if (utils.isJsonMode()) {
-        const result: any = { success: true, workItem: item };
-        result.comments = db.getCommentsForWorkItem(normalizedId);
+        // Prepare JSON-safe copies that omit the `audit` field when absent.
+        // Keep the audit object verbatim when present so JSON consumers can
+        // rely on the structured { time, author, text } shape.
+        const stripAudit = (src: WorkItem) => {
+          const copy: any = Object.assign({}, src);
+          if (copy.audit === undefined || copy.audit === null) delete copy.audit;
+          return copy as WorkItem;
+        };
+
+        const result: ShowJsonOutput = { success: true, workItem: stripAudit(item) };
+        result.comments = db.getCommentsForWorkItem(normalizedId) as Comment[];
         if (options.children) {
-           const children = db.getDescendants(normalizedId);
-           const ancestors: typeof item[] = [];
+           const children = db.getDescendants(normalizedId).map(stripAudit);
+           const ancestors: any[] = [];
           let currentParentId = item.parentId;
           while (currentParentId) {
             const parent = db.get(currentParentId);
             if (!parent) break;
-            ancestors.push(parent);
+            ancestors.push(stripAudit(parent));
             currentParentId = parent.parentId;
           }
           result.children = children;

src/github-sync.ts

@@ -251,7 +251,10 @@ export async function upsertIssuesFromWorkItems(
   };
 
   // Concurrency: upsert issues and comments with a bounded concurrency pool
-  const upsertConcurrency = Number(process.env.WL_GITHUB_CONCURRENCY || '6');
+  // The central throttler enforces concurrency/rate limits. Do not rely on
+  // a local worker pool here; schedule GitHub API calls through `throttler`.
+  // Keep the env var available to the throttler implementation.
+  // (local upsertConcurrency removed)
 
   const truncateTitle = (title: string, maxLen = 60): string =>
     title.length <= maxLen ? title : title.slice(0, maxLen - 1) + '\u2026';
@@ -292,14 +295,16 @@ export async function upsertIssuesFromWorkItems(
       const shouldUpdateIssue = !item.githubIssueNumber
         || !item.githubIssueUpdatedAt
         || new Date(item.updatedAt).getTime() > new Date(item.githubIssueUpdatedAt).getTime();
-      if (shouldUpdateIssue) {
+    if (shouldUpdateIssue) {
         const upsertStart = Date.now();
         if (onVerboseLog) {
           onVerboseLog(`[upsert] ${item.githubIssueNumber ? 'update' : 'create'} ${item.id}`);
         }
-        if (item.githubIssueNumber) {
-          increment('api.issue.update');
-          issue = await updateGithubIssueAsync(config, item.githubIssueNumber!, payload);
+            if (item.githubIssueNumber) {
+              increment('api.issue.update');
+              // updateGithubIssueAsync already schedules via the central throttler
+              // internally (see src/github.ts). Avoid double-scheduling here.
+              issue = await updateGithubIssueAsync(config, item.githubIssueNumber!, payload);
           if (item.status === 'deleted') {
             result.closed += 1;
             result.syncedItems.push({
@@ -317,13 +322,14 @@ export async function upsertIssuesFromWorkItems(
               issueNumber: item.githubIssueNumber,
             });
           }
-        } else {
-          increment('api.issue.create');
-          issue = await createGithubIssueAsync(config, {
-            title: payload.title,
-            body: payload.body,
-            labels: payload.labels,
-          });
+            } else {
+              increment('api.issue.create');
+              // createGithubIssueAsync schedules via the central throttler itself.
+              issue = await createGithubIssueAsync(config, {
+                title: payload.title,
+                body: payload.body,
+                labels: payload.labels,
+              });
           result.created += 1;
           result.syncedItems.push({
             action: 'created',
@@ -343,14 +349,16 @@ export async function upsertIssuesFromWorkItems(
       }
 
       const shouldSyncCommentsNow = itemComments.length > 0 && (shouldSyncComments || shouldUpdateIssue);
-      if (shouldSyncCommentsNow && issueNumber) {
-        const commentListStart = Date.now();
-        increment('api.comment.list');
-        const existingComments = await listGithubIssueCommentsAsync(config, issueNumber!);
-        timing.commentListMs += Date.now() - commentListStart;
-        const commentUpsertStart = Date.now();
-        const commentSummary = await upsertGithubIssueCommentsAsync(config, issueNumber, itemComments, existingComments);
-        timing.commentUpsertMs += Date.now() - commentUpsertStart;
+        if (shouldSyncCommentsNow && issueNumber) {
+          const commentListStart = Date.now();
+          increment('api.comment.list');
+          // listGithubIssueCommentsAsync now schedules internally via the throttler
+          // (see src/github.ts). Call it directly to avoid double-scheduling.
+          const existingComments = await listGithubIssueCommentsAsync(config, issueNumber!);
+          timing.commentListMs += Date.now() - commentListStart;
+          const commentUpsertStart = Date.now();
+          const commentSummary = await upsertGithubIssueCommentsAsync(config, issueNumber, itemComments, existingComments);
+          timing.commentUpsertMs += Date.now() - commentUpsertStart;
         increment('api.comment.create', commentSummary.created || 0);
         increment('api.comment.update', commentSummary.updated || 0);
         result.commentsCreated = (result.commentsCreated || 0) + commentSummary.created;
@@ -399,12 +407,14 @@ export async function upsertIssuesFromWorkItems(
     for (const comment of sorted) {
       const body = buildGithubCommentBody(comment);
       const existing = byWorklogId.get(comment.id);
-      if (existing) {
+        if (existing) {
         // If the GH comment exists, only update if body changed OR GH's updatedAt is newer than our recorded mapping
         const bodyMatch = (existing.body || '').trim() === body.trim();
-        if (!bodyMatch) {
-          increment('api.comment.update');
-           const updatedComment = await updateGithubIssueCommentAsync(issueConfig, existing.id!, body);
+           if (!bodyMatch) {
+           increment('api.comment.update');
+             // updateGithubIssueCommentAsync now schedules internally via the throttler
+             // (see src/github.ts). Call it directly to avoid double-scheduling.
+             const updatedComment = await updateGithubIssueCommentAsync(issueConfig, existing.id!, body);
           // Persist mapping back to local comment
           comment.githubCommentId = existing.id;
           comment.githubCommentUpdatedAt = updatedComment.updatedAt;
@@ -417,9 +427,11 @@ export async function upsertIssuesFromWorkItems(
         continue;
       }
 
-      // No GH comment mapping found — create a new comment
-       increment('api.comment.create');
-          const createdComment = await createGithubIssueCommentAsync(issueConfig, issueNumber, body);
+       // No GH comment mapping found — create a new comment
+        increment('api.comment.create');
+            // createGithubIssueCommentAsync now schedules internally via the throttler
+            // (see src/github.ts). Call it directly to avoid double-scheduling.
+            const createdComment = await createGithubIssueCommentAsync(issueConfig, issueNumber, body);
       // Persist mapping back to local comment so future runs can directly reference by ID
       comment.githubCommentId = createdComment.id;
       comment.githubCommentUpdatedAt = createdComment.updatedAt;
@@ -433,23 +445,10 @@ export async function upsertIssuesFromWorkItems(
     return { created, updated, latestUpdatedAt };
   }
 
-  // simple concurrent mapper for issue upserts
-  async function mapWithConcurrencyItems(arr: WorkItem[], limit: number, fn: (v: WorkItem, i: number) => Promise<void>) {
-    const results: Promise<void>[] = [];
-    let i = 0;
-    async function worker() {
-      while (true) {
-        const idx = i++;
-        if (idx >= arr.length) return;
-        await fn(arr[idx], idx);
-      }
-    }
-    const workers = Math.min(limit, arr.length);
-    for (let w = 0; w < workers; w += 1) results.push(worker());
-    await Promise.all(results);
-  }
-
-  await mapWithConcurrencyItems(issueItems, upsertConcurrency, upsertMapper);
+  // Launch upsert mappers without a local worker pool; schedule external
+  // GitHub API calls through the central throttler. The throttler enforces
+  // WL_GITHUB_CONCURRENCY and rate limits configured in src/github-throttler.ts.
+  await Promise.all(issueItems.map((it, idx) => upsertMapper(it, idx)));
 
   result.skipped = items.length - issueItems.length + skippedUpdates;
 
@@ -554,23 +553,10 @@ export async function upsertIssuesFromWorkItems(
       }
     }
 
-  // simple concurrent mapper
-  async function mapWithConcurrency(arr: string[], limit: number, fn: (v: string, i: number) => Promise<void>) {
-    const results: Promise<void>[] = [];
-    let i = 0;
-    async function worker() {
-      while (true) {
-        const idx = i++;
-        if (idx >= arr.length) return;
-        await fn(arr[idx], idx);
-      }
-    }
-    const workers = Math.min(limit, arr.length);
-    for (let w = 0; w < workers; w += 1) results.push(worker());
-    await Promise.all(results);
-  }
-
-  await mapWithConcurrency(pairs, concurrency, mapper);
+  // Process hierarchy pairs concurrently and let the throttler limit GitHub
+  // requests. Avoid a local worker pool — schedule linking/fetch calls via
+  // the central throttler inside `mapper`.
+  await Promise.all(pairs.map((p, idx) => mapper(p, idx)));
 
   result.updated += linkedCount;
   timing.totalMs = Date.now() - startTime;

src/github.ts

@@ -934,14 +934,18 @@ export function listGithubIssueComments(config: GithubConfig, issueNumber: numbe
 export async function listGithubIssueCommentsAsync(config: GithubConfig, issueNumber: number): Promise<GithubIssueComment[]> {
   const { owner, name } = parseRepoSlug(config.repo);
   const command = `gh api repos/${owner}/${name}/issues/${issueNumber}/comments --paginate`;
-  try {
-    const data = await runGhJsonAsync(command);
-    if (!data) return [];
-    const raw = Array.isArray(data) ? data : [];
-    return raw.map(comment => normalizeGithubIssueComment(comment));
-  } catch {
-    return [];
-  }
+  // Schedule network call through central throttler to enforce concurrency
+  // and rate limits. Callers should not need to schedule this themselves.
+  return await throttler.schedule(async () => {
+    try {
+      const data = await runGhJsonAsync(command);
+      if (!data) return [];
+      const raw = Array.isArray(data) ? data : [];
+      return raw.map(comment => normalizeGithubIssueComment(comment));
+    } catch {
+      return [];
+    }
+  });
 }
 
 export function createGithubIssueComment(config: GithubConfig, issueNumber: number, body: string): GithubIssueComment {
@@ -952,10 +956,13 @@ export function createGithubIssueComment(config: GithubConfig, issueNumber: numb
 }
 
 export async function createGithubIssueCommentAsync(config: GithubConfig, issueNumber: number, body: string): Promise<GithubIssueComment> {
-  const { owner, name } = parseRepoSlug(config.repo);
-  const command = `gh api -X POST repos/${owner}/${name}/issues/${issueNumber}/comments -F body=@-`;
-  const data = await runGhJsonAsync(command, body);
-  return normalizeGithubIssueComment(data);
+  // Ensure comment creation is scheduled through the central throttler.
+  return await throttler.schedule(async () => {
+    const { owner, name } = parseRepoSlug(config.repo);
+    const command = `gh api -X POST repos/${owner}/${name}/issues/${issueNumber}/comments -F body=@-`;
+    const data = await runGhJsonAsync(command, body);
+    return normalizeGithubIssueComment(data);
+  });
 }
 
 export function updateGithubIssueComment(config: GithubConfig, commentId: number, body: string): GithubIssueComment {
@@ -966,10 +973,13 @@ export function updateGithubIssueComment(config: GithubConfig, commentId: number
 }
 
 export async function updateGithubIssueCommentAsync(config: GithubConfig, commentId: number, body: string): Promise<GithubIssueComment> {
-  const { owner, name } = parseRepoSlug(config.repo);
-  const command = `gh api -X PATCH repos/${owner}/${name}/issues/comments/${commentId} -F body=@-`;
-  const data = await runGhJsonAsync(command, body);
-  return normalizeGithubIssueComment(data);
+  // Ensure comment updates are scheduled through the central throttler.
+  return await throttler.schedule(async () => {
+    const { owner, name } = parseRepoSlug(config.repo);
+    const command = `gh api -X PATCH repos/${owner}/${name}/issues/comments/${commentId} -F body=@-`;
+    const data = await runGhJsonAsync(command, body);
+    return normalizeGithubIssueComment(data);
+  });
 }
 
 export function getGithubIssueComment(config: GithubConfig, commentId: number): GithubIssueComment {

src/types.ts

@@ -225,3 +225,18 @@ export interface NextWorkItemResult {
   workItem: WorkItem | null;
   reason: string;
 }
+
+/**
+ * JSON output shape for the `show` command when --json mode is enabled.
+ * This keeps the CLI's JSON API stable and explicitly documents the fields
+ * returned by the endpoint.
+ */
+export interface ShowJsonOutput {
+  success: true | false;
+  workItem?: WorkItem;
+  comments?: Comment[];
+  children?: WorkItem[];
+  ancestors?: WorkItem[];
+  // Optional error message used when success is false
+  error?: string;
+}

tests/cli/__snapshots__/human-show-list-audit-snapshots.test.ts.snap

@@ -0,0 +1,41 @@
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+exports[`Human snapshots: show and list outputs with audit > renders concise/list and single-item human outputs with and without audit (snapshots) > human-list-with-audit 1`] = `
+"Found 2 work item(s):
+
+
+├── Audited task TEST-1
+│   Status: Open · Stage: Undefined | Priority: medium
+│   SortIndex: 0
+│   Risk: —
+│   Effort: —
+│   Audit: Ready to close: Yes
+└── No audit TEST-2
+    Status: Open · Stage: Undefined | Priority: medium
+    SortIndex: 0
+    Risk: —
+    Effort: —
+
+"
+`;
+
+exports[`Human snapshots: show and list outputs with audit > renders concise/list and single-item human outputs with and without audit (snapshots) > human-show-with-audit 1`] = `
+"
+└── Audited task TEST-1
+    Status: Open · Stage: Undefined | Priority: medium
+    SortIndex: 0
+    Risk: —
+    Effort: —
+    Audit: Ready to close: Yes
+"
+`;
+
+exports[`Human snapshots: show and list outputs with audit > renders concise/list and single-item human outputs with and without audit (snapshots) > human-show-without-audit 1`] = `
+"
+└── No audit TEST-2
+    Status: Open · Stage: Undefined | Priority: medium
+    SortIndex: 0
+    Risk: —
+    Effort: —
+"
+`;