Merge pull request #178 from maystudios/worktree-agent-a9ba34bf

fix: enforce spec compliance in GitHub module

Author: maystudios

packages/cli/src/github/comments.ts

@@ -74,12 +74,16 @@ export function parseIssueState(body: string): MaxsimIssueState | null {
   };
 }
 
+const VALID_COMMENT_TYPES = new Set(['plan', 'research', 'context', 'progress', 'verification', 'summary', 'error', 'escalation', 'handoff', 'user-intent', 'phase-complete', 'checkpoint'] as const);
+
 /** Extract comment metadata from the first line HTML comment. */
 export function parseCommentMeta(body: string): MaxsimCommentMeta | null {
   const match = body.match(COMMENT_META_REGEX);
   if (!match) return null;
 
-  const type = match[1] as MaxsimCommentMeta['type'];
+  const type = match[1];
+  if (!(VALID_COMMENT_TYPES as ReadonlySet<string>).has(type)) return null;
+
   const attrs: Record<string, unknown> = { type };
 
   // Parse space-separated key=value pairs

packages/cli/src/github/discussions.ts

@@ -162,10 +162,19 @@ export function listDiscussions(
   }
 
   const hasCategory = categoryId !== undefined;
-  const query = `
-    query($owner: String!, $repo: String!, $first: Int!${hasCategory ? ', $categoryId: ID!' : ''}) {
+
+  const allNodes: RawDiscussion[] = [];
+  let hasNextPage = true;
+  let endCursor: string | null = null;
+
+  while (hasNextPage) {
+    const afterClause = endCursor ? `, after: $after` : '';
+    const afterVar = endCursor ? ', $after: String!' : '';
+
+    const query = `
+    query($owner: String!, $repo: String!, $first: Int!${hasCategory ? ', $categoryId: ID!' : ''}${afterVar}) {
       repository(owner: $owner, name: $repo) {
-        discussions(first: $first${hasCategory ? ', categoryId: $categoryId' : ''}) {
+        discussions(first: $first${hasCategory ? ', categoryId: $categoryId' : ''}${afterClause}) {
           nodes {
             number
             id
@@ -176,31 +185,54 @@ export function listDiscussions(
             createdAt
             updatedAt
           }
+          pageInfo {
+            hasNextPage
+            endCursor
+          }
         }
       }
     }
   `.trim();
 
-  const args = [
-    'api', 'graphql',
-    '-f', `query=${query}`,
-    '-f', `owner=${owner}`,
-    '-f', `repo=${repoName}`,
-    '-F', `first=${first}`,
-  ];
+    const args = [
+      'api', 'graphql',
+      '-f', `query=${query}`,
+      '-f', `owner=${owner}`,
+      '-f', `repo=${repoName}`,
+      '-F', `first=${first}`,
+    ];
 
-  if (hasCategory) {
-    args.push('-f', `categoryId=${categoryId}`);
-  }
+    if (hasCategory) {
+      args.push('-f', `categoryId=${categoryId}`);
+    }
 
-  const result = ghJson<{
-    data: { repository: { discussions: { nodes: RawDiscussion[] } } };
-  }>(args);
+    if (endCursor) {
+      args.push('-f', `after=${endCursor}`);
+    }
 
-  if (!result.ok) return result;
+    const result = ghJson<{
+      data: {
+        repository: {
+          discussions: {
+            nodes: RawDiscussion[];
+            pageInfo: { hasNextPage: boolean; endCursor: string | null };
+          };
+        };
+      };
+    }>(args);
+
+    if (!result.ok) return result;
+
+    const discussions = result.data?.data?.repository?.discussions;
+    const nodes = discussions?.nodes ?? [];
+    allNodes.push(...nodes);
+
+    const pageInfo = discussions?.pageInfo;
+    hasNextPage = pageInfo?.hasNextPage ?? false;
+    endCursor = pageInfo?.endCursor ?? null;
+  }
 
-  const nodes = result.data?.data?.repository?.discussions?.nodes ?? [];
-  return { ok: true, data: nodes.map(mapDiscussion) };
+  return { ok: true, data: allNodes.map(mapDiscussion) };
 }
 
 export function getDiscussion(

packages/cli/src/github/projects.ts

@@ -257,10 +257,11 @@ export async function ensureProjectBoard(
     ]);
 
     if (!updateResult.ok) {
-      console.warn(
-        `Warning: Failed to add missing Status options (${missingColumns.map((c) => c.name).join(', ')}): ${updateResult.error}. ` +
-        'You may need to add them manually via the GitHub Projects UI.',
-      );
+      return {
+        ok: false,
+        error: `Failed to add missing Status options (${missingColumns.map((c) => c.name).join(', ')}): ${updateResult.error}`,
+        code: 'UNKNOWN',
+      };
     }
   }
 

packages/cli/src/github/types.ts

@@ -133,7 +133,7 @@ export interface MaxsimIssueState {
 
 /** Structured comment metadata from HTML comment markers. */
 export interface MaxsimCommentMeta {
-  type: 'plan' | 'research' | 'context' | 'progress' | 'verification' | 'summary' | 'error' | 'escalation' | 'handoff' | 'user-intent';
+  type: 'plan' | 'research' | 'context' | 'progress' | 'verification' | 'summary' | 'error' | 'escalation' | 'handoff' | 'user-intent' | 'phase-complete' | 'checkpoint';
   phase?: number;
   task?: number;
   [key: string]: unknown;

packages/cli/tests/unit/comments.test.ts

@@ -120,6 +120,10 @@ describe('parseCommentMeta', () => {
   it('returns null for non-maxsim comments', () => {
     expect(parseCommentMeta('Just a normal comment')).toBeNull();
   });
+
+  it('returns null for invalid comment types', () => {
+    expect(parseCommentMeta('<!-- maxsim:type=INVALID_TYPE -->')).toBeNull();
+  });
 });
 
 describe('formatIssueMeta', () => {

packages/cli/tests/unit/github-discussions.test.ts

@@ -119,12 +119,16 @@ function createDiscussionResponse(discussion: typeof MOCK_DISCUSSION_RAW = MOCK_
 }
 
 /** Build the GraphQL response envelope for a discussions list query. */
-function listDiscussionsResponse(discussions: typeof MOCK_DISCUSSION_RAW[] = [MOCK_DISCUSSION_RAW]): string {
+function listDiscussionsResponse(
+  discussions: typeof MOCK_DISCUSSION_RAW[] = [MOCK_DISCUSSION_RAW],
+  pageInfo: { hasNextPage: boolean; endCursor: string | null } = { hasNextPage: false, endCursor: null },
+): string {
   return JSON.stringify({
     data: {
       repository: {
         discussions: {
           nodes: discussions,
+          pageInfo,
         },
       },
     },
@@ -322,6 +326,28 @@ describe('listDiscussions', () => {
     const ghArgs = graphqlCall?.[1] as string[];
     expect(ghArgs).toContain('owner=customowner');
   });
+
+  it('paginates through multiple pages and returns all discussions', () => {
+    const disc1 = { ...MOCK_DISCUSSION_RAW, number: 1, id: 'D_1', title: 'Discussion 1' };
+    const disc2 = { ...MOCK_DISCUSSION_RAW, number: 2, id: 'D_2', title: 'Discussion 2' };
+    const disc3 = { ...MOCK_DISCUSSION_RAW, number: 3, id: 'D_3', title: 'Discussion 3' };
+
+    // First page: 2 discussions with hasNextPage=true
+    // Second page: 1 discussion with hasNextPage=false
+    setupExecMock(
+      listDiscussionsResponse([disc1, disc2], { hasNextPage: true, endCursor: 'cursor_abc' }),
+      listDiscussionsResponse([disc3], { hasNextPage: false, endCursor: null }),
+    );
+
+    const result = listDiscussions();
+
+    expect(result.ok).toBe(true);
+    if (!result.ok) return;
+    expect(result.data).toHaveLength(3);
+    expect(result.data[0].number).toBe(1);
+    expect(result.data[1].number).toBe(2);
+    expect(result.data[2].number).toBe(3);
+  });
 });
 
 // ── getDiscussion ───────────────────────────────────────────────────────────

packages/cli/tests/unit/github-projects.test.ts

@@ -656,6 +656,61 @@ describe('ensureProjectBoard', () => {
     expect(graphqlCalls.length).toBe(1);
   });
 
+  it('returns ok: false when GraphQL mutation to add missing Status options fails', async () => {
+    // Status field exists but is missing columns, and the GraphQL update fails.
+    // We need a custom mock because setupExecMock catches all `gh api` calls
+    // as getRepoInfo type-detection. Here we differentiate `gh api graphql`
+    // (the mutation) from `gh api /users/...` (the type check).
+    const partialFields = {
+      fields: [
+        {
+          id: 'PVTSSF_status',
+          name: 'Status',
+          type: 'SINGLE_SELECT',
+          options: [
+            { id: 'opt_backlog', name: 'Backlog', color: 'GRAY' },
+          ],
+        },
+      ],
+    };
+    const projectList = { projects: [MOCK_PROJECT] };
+    const ghResponses = [
+      JSON.stringify(projectList),
+      JSON.stringify(partialFields),
+    ];
+    let callIndex = 0;
+    execFileSyncMock.mockImplementation((_cmd: string, args?: readonly string[]) => {
+      const argList = args ?? [];
+
+      // git calls used by getRepoInfo
+      if (_cmd === 'git') {
+        if (argList[0] === 'remote') return REPO_REMOTE_URL;
+      }
+
+      // gh api /users/* call used to detect org/user type
+      if (_cmd === 'gh' && argList[0] === 'api' && String(argList[1]).startsWith('/users/')) {
+        return 'User';
+      }
+
+      // gh api graphql — the mutation call should fail
+      if (_cmd === 'gh' && argList[0] === 'api' && argList[1] === 'graphql') {
+        throw new Error('gh: GraphQL mutation failed');
+      }
+
+      // All other gh calls — dispatch from the queue
+      const response = ghResponses[callIndex++];
+      if (response instanceof Error) throw response;
+      return response ?? '';
+    });
+
+    const result = await ensureProjectBoard('My Board', 'testorg');
+
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.code).toBe('UNKNOWN');
+    expect(result.error).toContain('Failed to add missing Status options');
+  });
+
   it('propagates error from findProject', async () => {
     setupExecMock(new Error('gh: 401 Unauthorized'));