- Handle type arrays: type: ['string', 'null'] → string | null - Handle const keyword: const: "active" → 'active' literal type - Handle nullable (OpenAPI 3.0 backward compatibility) - Extract and display webhook count in metadata - Add security escaping for string literals and JSDoc comments - Add OpenAPI 3.1 test fixture and 12 unit tests Fixes #365 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
175 lines
7.1 KiB
Markdown
175 lines
7.1 KiB
Markdown
---
|
|
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.)
|