diff --git a/docs/tasks/code-quality-refactoring-task.md b/docs/tasks/code-quality-refactoring-task.md new file mode 100644 index 0000000..e6815dc --- /dev/null +++ b/docs/tasks/code-quality-refactoring-task.md @@ -0,0 +1,175 @@ +--- +openproject: 359 +--- +# Task: Code Quality Refactoring + +## Related Documents +- Analysis: `/streamline` command output (2026-01-12) +- OpenProject: [#359](https://openproject.rimskij.net/work_packages/359) +- Branch: `feature/359-code-quality-refactoring` (from `feature/caching`) + +## Priority +NORMAL + +## Objective +Reduce code duplication and improve type safety across the swagger-tools codebase by extracting shared utilities, consolidating schema access patterns, and adding proper type guards. This will reduce maintenance burden by ~30% and make adding new tools faster. + +## Definition of Done +- [ ] Code implemented per specification +- [ ] TypeScript compilation CLEAN +- [ ] ALL manual tests passing (parse-spec, validate-spec, query-endpoints, get-schema, generate-types) +- [ ] No regression in existing functionality +- [ ] PROOF PROVIDED (before/after comparison) + +## Scope + +### IN SCOPE +- Extract HTTP_METHODS constant to shared location +- Consolidate schema access functions (getSchemas, findSchema, getSchemaNames, getSchemaCount) +- Create tool response helpers (successResponse, errorResponse) +- Add type guards for OpenAPI spec versions +- Extract cache configuration constants + +### OUT OF SCOPE +- Adding new features +- Changing tool behavior or output format +- Restructuring directory layout +- Adding test framework + +## Sub-Tasks + +### Phase 1: Foundation - Extract Shared Constants +#### 1.1 Extract HTTP_METHODS to types.ts +- **Details**: Move the duplicated HTTP_METHODS constant from parser.ts, query.ts, and validator.ts to lib/types.ts. Export as const array and derive HttpMethod type. +- **Files**: + - `src/lib/types.ts` (add export) + - `src/lib/parser.ts` (import, remove local) + - `src/tools/query.ts` (import, remove local) + - `src/lib/validator.ts` (import, remove inline) +- **Testing**: `npm run build` must pass, all tools work unchanged + +#### 1.2 Extract cache configuration constants +- **Details**: Extract DEFAULT_CACHE_MAX_SIZE=10 and DEFAULT_CACHE_TTL_MINUTES=15 as named constants in cache.ts +- **Files**: `src/lib/cache.ts` +- **Testing**: `npm run build`, caching behavior unchanged + +### Phase 2: Schema Utilities Consolidation +#### 2.1 Create schema-utils.ts module +- **Details**: Create new module with consolidated schema access functions extracted from schema.ts, generate.ts, and parser.ts +- **Files**: + - `src/lib/schema-utils.ts` (new file) +- **Testing**: `npm run build` + +#### 2.2 Implement getSchemas function +- **Details**: Single function handling both OpenAPI 3.x (components.schemas) and Swagger 2.0 (definitions) +- **Files**: `src/lib/schema-utils.ts` +- **Testing**: Parse petstore.yaml, verify schemas returned + +#### 2.3 Implement findSchema, getSchemaNames, getSchemaCount +- **Details**: Build on getSchemas for consistent behavior +- **Files**: `src/lib/schema-utils.ts` +- **Testing**: get-schema tool works, generate-types works + +#### 2.4 Update consumers to use schema-utils +- **Details**: Replace local implementations with imports from schema-utils.ts +- **Files**: + - `src/tools/schema.ts` (replace findSchema, getAvailableSchemas) + - `src/tools/generate.ts` (replace getSchemas) + - `src/lib/parser.ts` (replace getSchemaCount) +- **Testing**: All tools produce identical output to before + +### Phase 3: Tool Response Helpers +#### 3.1 Create tool-response.ts module +- **Details**: Create response builder utilities for consistent MCP tool responses +- **Files**: + - `src/lib/tool-response.ts` (new file) +- **Testing**: `npm run build` + +#### 3.2 Implement successResponse and errorResponse +- **Details**: Helper functions that construct the MCP response format with content and structuredContent +- **Files**: `src/lib/tool-response.ts` +- **Testing**: Type-check passes + +#### 3.3 Update tool handlers to use response helpers +- **Details**: Replace try/catch boilerplate in each tool handler +- **Files**: + - `src/tools/parse.ts` + - `src/tools/validate.ts` + - `src/tools/query.ts` + - `src/tools/schema.ts` + - `src/tools/generate.ts` +- **Testing**: All tools produce identical output, error handling works + +### Phase 4: Type Guards +#### 4.1 Create spec-guards.ts module +- **Details**: Type guard functions for OpenAPI spec versions +- **Files**: + - `src/lib/spec-guards.ts` (new file) +- **Testing**: `npm run build` + +#### 4.2 Implement isOpenAPIV3 and isSwagger2 guards +- **Details**: Proper type guards that narrow types, replace `as` casts +- **Files**: `src/lib/spec-guards.ts` +- **Testing**: Type-check passes + +#### 4.3 Update consumers to use type guards +- **Details**: Replace type assertions with proper guards where beneficial +- **Files**: + - `src/lib/parser.ts` + - `src/lib/schema-utils.ts` +- **Testing**: All tools work unchanged + +## Files to Modify + +| File | Change | +|------|--------| +| `src/lib/types.ts` | Add HTTP_METHODS, HttpMethod exports | +| `src/lib/cache.ts` | Extract cache config constants | +| `src/lib/parser.ts` | Import HTTP_METHODS, use schema-utils, use type guards | +| `src/lib/validator.ts` | Import HTTP_METHODS | +| `src/lib/schema-utils.ts` | **NEW** - consolidated schema access | +| `src/lib/tool-response.ts` | **NEW** - response helpers | +| `src/lib/spec-guards.ts` | **NEW** - type guards | +| `src/tools/parse.ts` | Use tool-response helpers | +| `src/tools/validate.ts` | Use tool-response helpers | +| `src/tools/query.ts` | Import HTTP_METHODS, use tool-response helpers | +| `src/tools/schema.ts` | Use schema-utils, tool-response helpers | +| `src/tools/generate.ts` | Use schema-utils, tool-response helpers | + +## Risks & Mitigations + +| Risk | Impact | Mitigation | +|------|--------|------------| +| Breaking existing tool behavior | HIGH | Test each tool after each phase | +| Type errors from refactoring | MEDIUM | Run `npm run build` after each sub-task | +| Missing edge cases in consolidation | MEDIUM | Compare output before/after for test fixtures | + +## Testing Strategy + +**After each phase:** +1. Build: `npm run build` - must pass +2. Type check: `npm run typecheck` - must pass +3. Manual tests: + ```bash + # Test parse-spec + echo '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"parse-spec","arguments":{"path":"test/fixtures/petstore.yaml"}}}' | node dist/index.js + + # Test validate-spec + echo '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"validate-spec","arguments":{"path":"test/fixtures/petstore.yaml"}}}' | node dist/index.js + + # Test query-endpoints + echo '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"query-endpoints","arguments":{"path":"test/fixtures/petstore.yaml","method":"GET"}}}' | node dist/index.js + + # Test get-schema + echo '{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"get-schema","arguments":{"path":"test/fixtures/petstore.yaml","schemaName":"Pet"}}}' | node dist/index.js + + # Test generate-types + echo '{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"generate-types","arguments":{"path":"test/fixtures/petstore.yaml","schemas":["Pet"]}}}' | node dist/index.js + ``` + +## Implementation Notes + +- Preserve exact output format - consumers may depend on it +- Each phase should be independently committable +- Run full test suite between phases +- New files follow existing patterns (ESM imports, zod schemas, etc.)