gh-workflow-mcp: Type design improvements

Type Design Improvements

Tracking 6 type design improvements identified in PR #432 all-hands-review (iteration 3) by Type Design Analyzer but deemed out-of-scope for immediate implementation.

Type-Specific Improvements

1. WatchResult: Use Discriminated Union

File: gh-watch.ts:12-21
Priority: MEDIUM

Current:

interface WatchResult {
  success: boolean;
  exitCode: number;
  timedOut: boolean;
  output: string;
}

Proposed:

type WatchResult = 
  | { status: 'success'; exitCode: 0; output: string }
  | { status: 'timeout'; exitCode: 124; output: string }
  | { status: 'failure'; exitCode: number; stderr: string; output: string };

Benefits:

• Impossible to have timedOut: true with exitCode: 0
• Type system enforces valid state combinations
• Better auto-completion and exhaustiveness checking

Tradeoffs:

• Requires updating all callsites to use discriminated union pattern
• More verbose type guards needed

---

2. WatchOptions: Branded Types

File: gh-watch.ts:23-28
Priority: LOW

Current:

interface WatchOptions {
  timeout: number;
  repo?: string;
}

Proposed:

type Milliseconds = number & { readonly __brand: 'Milliseconds' };
type RepositorySlug = string & { readonly __brand: 'RepositorySlug' };

interface WatchOptions {
  timeout: Milliseconds;
  repo?: RepositorySlug;
}

Benefits:

• Prevents passing seconds when milliseconds expected
• Type-level documentation of units
• Catches common unit conversion bugs

Tradeoffs:

• Requires explicit branding/casting
• More boilerplate for simple types
• May be overkill for internal API

---

3. Check: Discriminated Union for Status/Conclusion

File: gh-watch.ts:30-34
Priority: MEDIUM

Current:

interface Check {
  name: string;
  status: string;
  conclusion: string | null;
}

Proposed:

type Check = 
  | { name: string; status: 'completed'; conclusion: 'success' | 'failure' | 'cancelled' | 'skipped' | 'timed_out' }
  | { name: string; status: 'in_progress' | 'queued'; conclusion: null };

Benefits:

• Impossible to have status: 'completed' with conclusion: null
• Exhaustive handling of all valid conclusion types
• Better type safety in determineOverallStatus

Tradeoffs:

• GitHub API may add new statuses/conclusions
• Requires runtime validation at API boundary (see #539)

---

4. OverallStatus: Add totalCount and Use Union

File: gh-watch.ts:36-41
Priority: LOW

Current:

interface OverallStatus {
  status: string;
  successCount: number;
  failureCount: number;
  otherCount: number;
}

Proposed:

interface OverallStatus {
  status: 'SUCCESS' | 'FAILED' | 'MIXED';
  successCount: number;
  failureCount: number;
  otherCount: number;
  totalCount: number; // Derived but useful
}

Benefits:

• status has explicit valid values
• totalCount avoids recalculation
• Type checking for switch statements

Tradeoffs:

• Redundant totalCount (can compute from counts)

---

Cross-Cutting Improvements

5. Runtime Validation for External Data

Priority: HIGH (overlaps with #539)

All types representing GitHub API responses should have:

• Zod schemas for validation
• Parse functions that validate at runtime
• Clear error messages on validation failure

Affected types:

• WorkflowRunData (monitor-run.ts:52-62)
• JobData (monitor-run.ts:64-71)
• Check (gh-watch.ts:30-34)

Example:

import { z } from 'zod';

const CheckSchema = z.discriminatedUnion('status', [
  z.object({
    name: z.string(),
    status: z.literal('completed'),
    conclusion: z.enum(['success', 'failure', 'cancelled', 'skipped', 'timed_out']),
  }),
  z.object({
    name: z.string(),
    status: z.enum(['in_progress', 'queued']),
    conclusion: z.null(),
  }),
]);

type Check = z.infer<typeof CheckSchema>;

function parseCheck(data: unknown): Check {
  return CheckSchema.parse(data);
}

---

6. Use Union Types for String Enums

Priority: LOW

Replace string constants with union types for better type safety:

Example:

// Instead of:
const FAILURE_CONCLUSIONS = ['failure', 'timed_out', 'cancelled', 'action_required'];

// Use:
type FailureConclusion = 'failure' | 'timed_out' | 'cancelled' | 'action_required';
const FAILURE_CONCLUSIONS: readonly FailureConclusion[] = ['failure', 'timed_out', 'cancelled', 'action_required'] as const;

Benefits:

• Type checking for includes() checks
• Auto-completion for conclusion values
• Compile-time validation

---

Implementation Priority

1. High: Runtime validation (#5) - overlaps with #539
2. Medium: WatchResult discriminated union (#1) - significant safety improvement
3. Medium: Check discriminated union (#3) - prevents invalid states
4. Low: OverallStatus union type (#4) - minor improvement
5. Low: Branded types (#2) - may be overkill
6. Low: String enum unions (#6) - nice-to-have

Related Issues

• #432 - Parent PR for gh watch implementation
• #539 - Runtime validation improvements (HIGH priority overlap with #5)
• #15372 - Documentation improvements
• #15373 - Additional test coverage

Notes

• Many of these improvements are quality-of-life changes that make invalid states unrepresentable
• Runtime validation (#5) is most critical and overlaps with existing issue #539
• Consider implementing alongside #539 for consistency
• Discriminated unions (#1, #3) provide the best safety-to-effort ratio