chore(api): add reviewer prd and deployment runbook checks#127
Closed
chore(api): add reviewer prd and deployment runbook checks#127
Conversation
Refactor Notion script logic into reusable, API-callable modules that can be invoked from APIs, tests, or other tools without CLI dependencies. Core modules: - fetchPages: Fetch all pages from Notion database - fetchPage: Fetch a single page by ID - generateMarkdown: Generate markdown files from Notion pages - generatePlaceholders: Generate placeholder content for empty pages - validateConfig: Validate Notion API configuration - getHealthStatus: Check health of Notion API service All functions return ApiResult<T> with structured error handling, execution time tracking, and consistent metadata. Includes: - Pure functions with explicit config parameters - Progress callback support for long-running operations - Type-safe interfaces for all operations - Comprehensive test coverage (21 tests) Related to: PRD.md task "Refactor Notion script logic into reusable modules callable from API"
Add comprehensive documentation test suite that verifies and documents module purity across the codebase. This establishes: 1. Purity Categories: - PURE: No side effects, output depends only on inputs - ISOLATED_IMPURE: Side effects are isolated and documented - CONFIG_DEPENDENT: Depends on environment variables 2. Module Classifications: - imageCompressor: ISOLATED_IMPURE (uses spawn for pngquant) - utils.ts: PURE (all utility functions) - notion-api/modules.ts: PURE with dependency injection - notionClient.ts: CONFIG_DEPENDENT (needs refactoring) 3. Guidelines for new modules: - Prefer pure functions with explicit configuration - Isolate external dependencies with documentation - Avoid environment variable dependencies - Use dependency injection for testability The test suite documents current architecture decisions and provides guidance for future development.
- Implement HTTP API server using Bun's native serve() - Add job tracking system with in-memory state management - Support 7 job types: notion:fetch, notion:fetch-all, notion:translate, and 4 status update workflows - Add endpoints: GET /health, GET /jobs/types, GET /jobs, POST /jobs, GET /jobs/:id - Include job progress tracking and status updates - Add comprehensive test suite with 36 passing tests - Configure npm scripts: api:server, api:server:dev, test:api-server
Add comprehensive validation tests to verify API routes match required operations and response shapes per PRD requirement. Tests validate: - All 7 required job types are supported - Correct response shapes for all endpoints (health, jobs/types, jobs) - Job status transitions (pending -> running -> completed/failed) - CORS headers configuration - Error response consistency - Request validation for job types and options - All 5 required endpoints are defined All 53 tests pass (36 existing + 17 new validation tests).
Implement a minimal job queue with: - Configurable concurrency limit to control parallel job execution - Job cancellation support for both queued and running jobs - Automatic queue processing when slots become available - Integration with existing JobTracker for state management Key features: - JobQueue class with registerExecutor, add, cancel, and getStatus methods - createJobQueue factory for pre-configured queues with all job types - AbortSignal-based cancellation for graceful job termination - Comprehensive test coverage including concurrency enforcement and cancellation Co-authored-by: Claude <claude@anthropic.com>
Add comprehensive test suite covering: - Multiple simultaneous job additions (Promise.all batching) - FIFO order preservation under concurrency constraints - Concurrency limit enforcement under rapid concurrent requests - Job additions during active queue processing - Accurate running/queued count tracking during concurrent operations - Race condition handling in processQueue - Concurrent cancellation requests - Queue integrity with mixed add/cancel operations - getStatus() thread safety during concurrent operations - Prevention of job starvation under continuous load - Concurrent getQueuedJobs/getRunningJobs calls These tests verify the queue maintains correctness and integrity when handling concurrent HTTP requests typical of API server workloads.
…rvability - Add file-based job persistence using JSON format in .jobs-data directory - Implement log capture with both file and console output - Integrate persistence into job-tracker (load on startup, save on updates) - Integrate log capture into job-executor for job execution logging - Add comprehensive tests for persistence functionality (28 tests) - Update all test files with proper cleanup for persisted data - Add .jobs-data/ to .gitignore Implements PRD requirement for job status persistence and log capture.
Add comprehensive API endpoints for Notion job lifecycle management: - Add DELETE /jobs/:id endpoint for cancelling pending/running jobs - Add query parameter filtering to GET /jobs (?status=, ?type=) - Update CORS headers to support DELETE method - Add tests for job cancellation and filtering scenarios - Update console help with new endpoints and examples The API now supports complete CRUD operations for job lifecycle: - Create: POST /jobs - Read: GET /jobs, GET /jobs/:id - Update: Job status via execution - Delete: DELETE /jobs/:id (cancel operation) Job filtering allows querying by status (pending, running, completed, failed) and job type (notion:fetch, notion:fetch-all, etc.) with optional combined filters.
Per PRD requirement: "Review: confirm endpoint list is minimal and sufficient" Adds comprehensive test suite validating: - Exactly 6 endpoints exist (no redundancy) - Complete CRUD coverage (sufficiency) - All required job lifecycle operations - Query parameter filtering (not separate endpoints) - REST conventions (GET/POST/DELETE) - No redundant purposes - Discovery endpoints (/health, /jobs/types) - HATEOAS-like response structure All 25 tests pass.
Add comprehensive input validation and error handling for all API endpoints to improve security and provide better error messages. Changes: - Add ValidationError class for typed validation errors - Add isValidJobStatus() function for status validation - Add isValidJobId() function with path traversal prevention - Enhance parseJsonBody() with Content-Type and size validation - Add request body validation for POST /jobs endpoint - Validate type field presence and type - Validate job type against allowed values - Validate options object structure and types - Add query parameter validation for GET /jobs endpoint - Validate status filter against allowed values - Validate type filter against allowed values - Add job ID validation for GET/DELETE /jobs/:id endpoints - Prevent path traversal attacks - Enforce maximum length - Add error response helper with optional details field - Add 29 comprehensive tests for validation logic Security improvements: - Path traversal prevention in job IDs - Request size limits (1MB max) - Content-Type validation for POST requests - Input sanitization for all user-provided values
Add centralized error handling system for consistent, actionable error messages across all scripts. This addresses inconsistent error reporting patterns identified during code review. Changes: - Add scripts/shared/errors.ts with base error classes (AppError, ConfigError, NetworkError, ValidationError, FileSystemError, RateLimitError) - Each error type includes default suggestions and context tracking - Add consistent logging utilities (logError, logWarning, logInfo, logSuccess) - Add withErrorHandling wrapper for async operations - Update scripts/fetchNotionData.ts to use unified error logging - Update scripts/migrate-image-cache.ts to use FileSystemError - Update scripts/notion-placeholders/index.ts to use ConfigError - Update scripts/api-server/index.ts to use extended ValidationError - Add comprehensive test coverage (32 tests in errors.test.ts) Error messages now include: - Clear description of what went wrong - Actionable suggestions for resolution - Relevant context information - Consistent formatting with chalk colors Testing: All 32 tests pass, linting clean
Implement API key authentication and comprehensive request audit logging for the Notion Jobs API server. **Authentication (auth.ts):** - API key validation via Authorization header (Bearer/Api-Key schemes) - Environment variable configuration (API_KEY_<name> format) - Graceful degradation when no keys configured (allows public access) - Key metadata tracking (name, description, active status, creation date) - Support for multiple API keys with independent management - Minimum key length validation (16 characters) **Audit Logging (audit.ts):** - Comprehensive request logging with structured JSON format - Client IP extraction from various proxy headers (X-Forwarded-For, X-Real-IP, CF-Connecting-IP) - Authentication result tracking for all requests - Response time measurement and status code logging - File-based persistence (.audit-data/audit.log) - Public endpoint detection for conditional auth **API Server Integration (index.ts):** - Public endpoints: /health, /jobs/types (no auth required) - Protected endpoints: /jobs, /jobs/:id (require valid API key) - Enhanced startup information showing auth status and configured keys - Updated CORS headers to include Authorization - Comprehensive audit logging for all requests **Tests:** - 32 new tests covering authentication and audit functionality - Tests for API key validation, header parsing, and error handling - Tests for audit entry creation, logging, and configuration - All existing tests remain passing **Usage:** - Set API_KEY_* environment variables to enable authentication - Example: API_KEY_READONLY=sk_123... API_KEY_ADMIN=sk_456... - Use: Authorization: Bearer <api-key> or Authorization: Api-Key <api-key>
…etion - Integrate reportJobCompletion into executeJobAsync's onComplete callback - Pass GitHub context, job duration, and error details to status reporter - Add github-context parameter to executeJobAsync signature - Add comprehensive tests for GitHub status integration - Add tests for github-status module (reportJobCompletion, validation)
Add comprehensive test coverage for GitHub status reporting functionality including: - Idempotency verification: demonstrates that status updates are NOT idempotent (calling same status multiple times sends multiple updates to GitHub) - Job completion reporting: tests status content validation including job type, duration, error messages, and description truncation - GitHub context handling: verifies that status is only reported when context is provided, and that context is persisted with jobs - API response handling: tests rate limiting, server errors, network errors, and proper error logging without throwing - Context and target URL: validates default context usage and custom target URL inclusion All 16 new tests pass, providing verification that the GitHub status implementation is functionally correct while documenting the lack of idempotency protection.
Add comprehensive developer tools documentation with: - API Reference: Complete REST API documentation with curl examples for all endpoints - CLI Reference: Complete CLI command reference with examples for all commands - Developer Tools category: New sidebar category for developer documentation - i18n updates: Spanish and Portuguese translations for new sections The API reference includes: - Health check endpoint - Job types listing - Job creation with options - Job status queries with filtering - Job cancellation - Authentication and CORS details The CLI reference includes: - Notion content commands (fetch, fetch-all, fetch-one) - Translation commands - Status management commands - Export and template commands - API server commands - Development and testing commands All documentation follows project patterns with proper frontmatter, keywords, tags, and cross-references between API and CLI docs.
Add a new /docs endpoint that serves an OpenAPI 3.0 specification for the API server. This provides programmatic access to API documentation and enables integration with API documentation tools like Swagger UI. Changes: - Add GET /docs endpoint (public) that returns OpenAPI 3.0 JSON spec - Include all endpoints: /health, /jobs/types, /jobs, /jobs/:id - Document request/response schemas for all endpoints - Add bearer authentication security scheme - Update 404 response to include /docs endpoint - Update server startup logging to show /docs endpoint - Add comprehensive test coverage for /docs endpoint structure The /docs endpoint returns a complete OpenAPI specification including: - API metadata (title, version, description) - Server configuration - Security schemes (bearer auth) - All path definitions with methods, parameters, responses - Reusable schema definitions for request/response bodies - API tags for grouping endpoints This completes the PRD requirement: "Add API documentation endpoints or static docs page"
Implement consistent response structures across all API endpoints to improve automation support: **New response-schemas module:** - ErrorCode enum with machine-readable error codes - Standardized error response with code, message, status, requestId, timestamp - API response envelope with data, requestId, timestamp, and optional pagination - Pagination metadata for list endpoints - Request ID generation for distributed tracing **Updated API endpoints:** - All success responses now use ApiResponse envelope structure - All error responses now use standardized ErrorResponse with error codes - X-Request-ID header added to all responses for request tracing - Field-specific validation errors with predefined error codes **Updated OpenAPI spec:** - Added ApiResponse, ErrorResponse, and PaginationMeta schemas - Documented X-Request-ID response header - Updated JobsListResponse to use 'items' instead of 'jobs' **Tests:** - 27 new tests for response schema consistency - Tests verify request ID generation, ISO 8601 timestamps, error codes - Tests ensure automation-friendly design (machine-readable codes, tracing) This ensures API responses are consistent, predictable, and designed for automation as required by the PRD.
Remove JobStatus and ListResponse interfaces from response-schemas.ts:
- JobStatus was a duplicate of Job from job-tracker.ts with slight
differences (Date vs string/null for timestamps). The Job interface
from job-tracker.ts is the single source of truth.
- ListResponse was defined but never used. List endpoints use the
ApiResponse<T> wrapper with inline { items, count } structure.
- Also remove unused ListResponse import from index.ts
This improves KISS compliance by eliminating unnecessary type
duplication and dead code.
…ogic Add comprehensive unit tests for: - Module extraction functions (extractClientIp from audit module, extractKeyFromHeader from auth module) - Core job logic (parseProgressFromOutput, JOB_COMMANDS mapping, buildArgs function) Module extraction tests cover: - IP extraction from various headers (x-forwarded-for, x-real-ip, cf-connecting-ip) - Header priority and fallback behavior - IPv6 address handling - Authorization header parsing (Bearer/Api-Key schemes) - Case-insensitive scheme matching - Invalid format detection Core job logic tests cover: - Progress pattern matching from job output - Job type configuration verification - Argument building for notion:fetch-all with all options - Edge cases (zero values, empty strings, large numbers) - Boolean flag handling and option ordering
Add comprehensive integration tests for API server components: - Job tracker integration tests covering complete job lifecycle, filtering, and concurrent operations - Response schema integration tests for API envelopes and error responses - Authentication integration tests for API key validation - Job queue integration tests with job tracker coordination - Error handling integration tests for edge cases Also add test mode support to API server: - Use random port when API_PORT=0 for testing - Skip console output in test mode - Export actualPort for test assertions 21 new tests covering integration between components.
…it wrapper Added missing test coverage for: - requireAuth() middleware function (5 tests) - withAudit() wrapper function (7 tests) The new tests verify: - API key authentication with valid/invalid keys - Authorization header parsing (Bearer/Api-Key schemes) - Missing Authorization header handling - Disabled authentication behavior - Singleton instance usage - Successful/failed request logging - Response time tracking - Auth info capture in audit entries - Query parameter capture - Multiple log entry handling All 44 tests passing (auth: 24, audit: 20)
…t entries Adds validateAuditEntry() and validateAuthResult() functions to ensure runtime validation of audit log entries and authentication results. - validateAuditEntry: Validates all audit entry fields including id format, timestamps, auth success/error consistency, status codes, and response times - validateAuthResult: Validates auth result structure including success/error mutual exclusivity, meta fields, and date types - Comprehensive test coverage for all validation scenarios These functions help catch data integrity issues early and ensure audit logs are always well-formed.
Add Dockerfile, docker-compose.yml, and .dockerignore for API service containerization. Includes comprehensive tests for Docker configuration. - Dockerfile: Multi-stage build using official Bun image, non-root user, health check on /health endpoint, production-optimized - docker-compose.yml: Service definition with environment variables, resource limits, health checks, logging rotation, and volume for job persistence - .dockerignore: Excludes node_modules, test files, generated content, and development files for smaller build context - Tests: 33 tests validating Docker configuration consistency across files Testing: All 33 Docker configuration tests pass.
Minimize image size: - Remove unnecessary builder stage (no compilation needed) - Copy only essential API server files instead of entire project - Clear bun package cache after install - Use production-only dependencies - Enhanced .dockerignore to exclude all non-essential files Add build configurability: - ARG for BUN_VERSION (default: 1) - ARG for NODE_ENV (default: production) - ARG for health check intervals (interval, timeout, start_period, retries) Add runtime configurability via environment variables: - DOCKER_IMAGE_NAME, DOCKER_IMAGE_TAG, DOCKER_CONTAINER_NAME - DOCKER_CPU_LIMIT, DOCKER_MEMORY_LIMIT - DOCKER_CPU_RESERVATION, DOCKER_MEMORY_RESERVATION - DOCKER_RESTART_POLICY - HEALTHCHECK_INTERVAL, HEALTHCHECK_TIMEOUT, etc. - DOCKER_LOG_DRIVER, DOCKER_LOG_MAX_SIZE, DOCKER_LOG_MAX_FILE - DOCKER_VOLUME_NAME, DOCKER_NETWORK, DOCKER_NETWORK_NAME - Add metadata labels for better container organization Enhanced tests: - Add Image Minimization test suite for Dockerfile - Add Build Configurability test suite for Dockerfile - Add Environment Variable Configurability test suite for docker-compose - Add Image Size Minimization test suite for .dockerignore - Update existing tests to match new configurable patterns
…ions Add new workflow that calls the API server instead of running scripts directly. The workflow supports: - Multiple job types (notion:fetch-all, notion:fetch, notion:translate, etc.) - Configurable page limits and force options - GitHub status reporting (pending, success, failure) - Automatic job polling until completion - Local mode fallback for testing when API_ENDPOINT not set - Slack notifications on job completion This enables centralized job management through the API server with proper authentication, audit logging, and GitHub integration. Co-authored-by: Claude <noreply@github.com>
Add comprehensive test suite for VPS deployment documentation validation. Tests verify: - Frontmatter structure (id, title, sidebar_position, etc.) - Content sections (prerequisites, quick start, deployment steps) - Environment variables documentation - Code examples (bash, docker compose, nginx config) - External links and references - Deployment steps coverage - Troubleshooting sections - Security best practices - Production checklist items - Container management commands The test suite includes 54 tests validating the documentation structure and content completeness for the VPS deployment guide.
Add complete inventory of all Notion-related scripts including: - Core Notion scripts (notion-fetch, notion-fetch-all, etc.) - Shared utilities (fetchNotionData, notionClient, constants) - API server integration (job-executor, job-tracker, auth, audit) - Testing infrastructure and workflow integration Provides a central reference for understanding script relationships, entry points, environment variables, and API server job mappings. Addresses the original "Inventory scripts" task from PRD.md.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3400f751c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1066
to
+1070
| // Mark job as failed with cancellation reason | ||
| tracker.updateJobStatus(jobId, "failed", { | ||
| success: false, | ||
| error: "Job cancelled by user", | ||
| }); |
There was a problem hiding this comment.
The cancel handler only updates the job status to failed, but it never stops the running job process or prevents queued work from continuing. For running jobs this means the underlying Bun child process will keep executing and can later overwrite the status to completed, so a client that issues DELETE /jobs/:id won’t actually cancel the work. If cancellation is supported, this needs to terminate the spawned process (or mark the job as canceled in a way that the executor checks before continuing) rather than only changing the status.
Useful? React with 👍 / 👎.
Contributor
🚀 Preview DeploymentYour documentation preview is ready! Preview URL: https://pr-127.comapeo-docs.pages.dev 🔄 Content: Regenerated 5 pages from Notion (script changes detected)
This preview will update automatically when you push new commits to this PR. Built with commit 0f2010c |
Contributor
Author
|
Closing to keep a single feature PR/branch. All relevant changes were consolidated into PR #126 on feat/notion-api-service. |
Contributor
🧹 Preview Deployment CleanupThe preview deployment for this PR has been cleaned up. Preview URL was: Note: Cloudflare Pages deployments follow automatic retention policies. Old previews are cleaned up automatically. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.prd/chore/api-reviewer-prd-and-deploy-docs/PRD.md.claude/command-history.logand.audit-data/to.gitignoreand stop tracking command history logcontext/workflows/api-service-deployment.mdPRD.mdand.env.exampleTesting
bunx eslint scripts/api-server/deployment-runbook.test.ts scripts/api-server/docker-smoke-tests.test.ts --fixbunx vitest run scripts/api-server/deployment-runbook.test.ts scripts/api-server/docker-smoke-tests.test.ts scripts/api-server/vps-deployment-docs.test.tsbun run test:api-server(one flaky failure observed inscripts/api-server/job-persistence.test.tstied to.jobs-data/jobs.jsonrace, thenbunx vitest run scripts/api-server/job-persistence.test.tspasses)Notes
feat/notion-api-serviceand existing PR api-driven notion operations #126