VIBE-209 Blob Ingestion - #136
Conversation
Created specification and implementation plan for OAuth-protected API endpoint with JSON schema validation and location matching. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Created specification document with requirements and acceptance criteria - Created tasks document with implementation checklist - Includes database changes, API implementation, logging, and testing tasks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Specification document with JSON schema and OAuth requirements - Technical implementation plan with 12-phase approach - Detailed task breakdown with 32 actionable tasks - Estimated effort: 13 hours 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- specification.md: System architecture and OAuth 2.0 design - plan.md: 9-phase implementation plan (20 days) - tasks.md: 200+ detailed tasks with checklist - New @hmcts/api-publication and @hmcts/api-auth modules 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Created comprehensive technical implementation plan (plan.md) for blob ingestion API: - Database schema changes (no_match column, ingestion_log table) - New @hmcts/blob-ingestion module architecture - OAuth 2.0 authentication and JSON schema validation - Integration with existing publication logic - 5-week phased implementation plan 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds a Blob Ingestion feature: new libs/api package (validation, service, file storage, OAuth middleware, routes), Prisma migration for ingestion_log and artefact.no_match, Helm/CI/env updates, monorepo path fixes, extensive tests (unit, integration, E2E), and supporting docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuth as OAuth Middleware
participant Validator as Validation Layer
participant Service as Blob Ingestion Service
participant Artefact as Artefact Repo
participant Storage as File Storage
participant Logs as Ingestion Logs
participant DB as Database
Client->>OAuth: POST /v1/publication (Bearer + JSON)
OAuth->>OAuth: Extract & verify token (JWKS/Azure)
alt Token invalid
OAuth-->>Client: 401
else Missing role
OAuth-->>Client: 403
else Token valid
OAuth->>Validator: Pass request
Validator->>Validator: Size, required fields, dates, enums
Validator->>Validator: Validate hearing_list schema
alt Validation fails
Validator->>Logs: Create VALIDATION_ERROR
Logs->>DB: Insert ingestion_log
Validator-->>Client: 400 (errors)
else Validation succeeds
Validator->>Service: Validated request
Service->>Artefact: createArtefact(noMatch)
Artefact->>DB: Insert artefact
Service->>Storage: saveUploadedFile(hearing_list)
Storage->>Storage: Write file
Service->>Logs: Create SUCCESS
Logs->>DB: Insert ingestion_log
alt noMatch true
Service-->>Client: 200 (no_match)
else
Service-->>Client: 201 (artefact_id)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Key areas requiring focused review:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-11-27T09:50:32.707ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/app.test.ts (1)
44-67: Replace placeholder test assertions with actual verification.Multiple tests contain placeholder comments without actual assertions. These tests don't verify the middleware is actually configured.
Consider implementing these tests properly or removing them. For example:
it("should have JSON body parser middleware", () => { - expect(app).toBeDefined(); - // Express app should be configured with json parser + // Make a request to verify JSON parsing works + const req = { body: { test: "data" } }; + // Or inspect app._router.stack for the middleware + expect(app).toBeDefined(); });Alternatively, if these middleware checks are already covered by integration tests, consider removing these placeholder tests.
libs/admin-pages/src/manual-upload/file-storage.ts (1)
1-23: Extract duplicated file storage logic to a shared module.This file has identical implementation to
libs/blob-ingestion/src/blob-ingestion/file-storage.ts, violating the DRY principle. Both modules implement the samesaveUploadedFilefunction with identical logic.Consider creating a shared file storage utility:
Create
libs/shared-utils/src/file-storage.ts:import fs from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); // Navigate to monorepo root (from libs/shared-utils/src/) const MONOREPO_ROOT = path.join(__dirname, "..", "..", ".."); const TEMP_STORAGE_BASE = path.join(MONOREPO_ROOT, "storage", "temp", "uploads"); export async function saveUploadedFile(artefactId: string, originalFileName: string, fileBuffer: Buffer): Promise<void> { const fileExtension = path.extname(originalFileName); const newFileName = `${artefactId}${fileExtension}`; await fs.mkdir(TEMP_STORAGE_BASE, { recursive: true }); const filePath = path.join(TEMP_STORAGE_BASE, newFileName); await fs.writeFile(filePath, fileBuffer); }Then import from both locations:
import { saveUploadedFile } from "@hmcts/shared-utils";
♻️ Duplicate comments (1)
libs/blob-ingestion/src/blob-ingestion/file-storage.ts (1)
1-23: Duplicate file storage implementation detected.This file is identical to
libs/admin-pages/src/manual-upload/file-storage.ts. The samesaveUploadedFilefunction with identical logic is duplicated across modules.Refer to the review comment on
libs/admin-pages/src/manual-upload/file-storage.ts(lines 1-23) for the suggested refactoring to extract this shared logic into a common module like@hmcts/shared-utils.
🧹 Nitpick comments (26)
.ai/plans/VIBE-209/specification.md (3)
23-24: Clarify downstream behavior for no_match flag.The specification states (line 24) that blobs with unmatched courts should be "ingested anyway but flag with
no_match=true", and the success response example (lines 231-244) shows this case. However, the specification does not define downstream consequences:
- Is no_match content excluded from public UI views?
- Are there admin-only visibility rules?
- Should source systems be notified of no_match for manual intervention?
- Can content stay in no_match state indefinitely, or is re-reconciliation expected?
Add a "Post-Ingestion Handling" section defining:
- UI visibility and filtering rules based on no_match status
- Remediation workflow (manual court creation, re-matching, etc.)
- Alerting to source systems or admins
- Retention and cleanup policy for unmatched artefacts
Also applies to: 231-244
407-411: Clarify PII handling and audit log data protection.Lines 409-411 specify that sensitive data (e.g., defendant names) must be masked in logs with hashing. However, the IngestionAudit schema (line 72) includes an unmasked
error_detailsJSON field that may capture sensitive information from validation or processing errors.Clarify:
- What constitutes "sensitive data" in audit context (e.g., case names, defendant names, judge names)?
- Should
error_detailsbe sanitized to exclude PII before storage?- Is the 90-day retention (line 411) enforced via database TTL, application job, or manual process?
Add implementation guidance:
- Use consistent hashing or tokenization for PII in audit logs - Implement sanitization function for error messages before storing in error_details - Configure database TTL for ingestion_audit table (TTL: 90 days)Also applies to: 71-72
437-458: Validate performance and scalability claims against design.Non-functional requirements state (lines 439-441):
- API response time: < 2 seconds p95
- Support 100 requests/minute per source system
- Optimized queries with proper indexes
However, the specification does not detail:
- Caching strategy for location lookups (will frequently-queried courts be cached?).
- Query execution plans or index design for the ingestion_audit table (line 81-84 defines indexes, but are they sufficient?).
- Load testing or capacity plan to validate the 100 req/min claim.
- Whether the external modules (
@hmcts/publication,@hmcts/location) have documented performance characteristics.Recommend:
- Add a "Performance & Caching" section detailing:
- Location reference data caching (in-memory, Redis, or query-time cache-aside).
- Indexes on frequently filtered columns (source_system, createdAt, validationResult).
- Database connection pool sizing for 100 req/min.
- Document baseline performance testing expectations (e.g., "load test with 120 req/min for 10 minutes").
- Confirm external modules meet performance SLAs.
libs/admin-pages/src/manual-upload/validation.test.ts (1)
5-41: Consider reusing shared locale definitions or types formockTranslationsThis large inline translations object duplicates a lot of UI copy that likely already exists in the shared locale files. That can drift over time and makes copy changes noisier for tests.
If possible, consider importing the manual-upload English locale (or its type) and either:
- building this mock from the real locale (e.g.
const mockTranslations: ManualUploadTranslations = realEnManualUpload;with overrides if needed), or- at least typing
mockTranslationsto the sameManualUploadTranslationsinterface so key changes are caught in one place.This keeps tests aligned with production text and reduces maintenance friction.
Based on learnings, shared/common content should live in locale files, with tests ideally reusing those definitions rather than re-encoding strings here.
apps/api/src/app.ts (1)
43-45: Replace console.error with structured logging.Using
console.errorfor error logging doesn't provide structured logging capabilities needed for production monitoring.Consider using a proper logging library or Application Insights:
- app.use((err: Error, _req: express.Request, res: express.Response, _next: express.NextFunction) => { - console.error(err.stack); + app.use((err: Error, req: express.Request, res: express.Response, _next: express.NextFunction) => { + // Log with structured data for monitoring + // logger.error("Unhandled error", { error: err.message, stack: err.stack, path: req.path }); + console.error("Unhandled error:", err.stack); res.status(500).json({ error: "Internal server error" }); });libs/blob-ingestion/src/routes/v1/publication.test.ts (1)
59-59: Avoid brittle array indexing for handler access.Accessing
handlers[1]directly assumes the route handler is always at index 1. If middleware order changes, tests will break or test the wrong handler.Consider making the handler access more explicit:
- const handlers = POST; - const handler = handlers[1] as (req: Request, res: Response) => Promise<void>; + const handlers = POST; + // Skip OAuth middleware (index 0), get actual route handler + const handler = handlers.find(h => h.length === 2) as (req: Request, res: Response) => Promise<void>; + if (!handler) throw new Error("Route handler not found");Or extract and export the handler separately for testing:
// In publication.ts export const publicationHandler = async (req: Request, res: Response) => { ... }; export const POST = [authenticateApi(), publicationHandler]; // In tests import { publicationHandler } from "./publication.js"; await publicationHandler(mockRequest as Request, mockResponse as Response);Also applies to: 81-81, 102-102, 119-119, 141-141, 159-159
libs/blob-ingestion/src/routes/v1/publication.ts (3)
11-12: Raw body size calculation is inaccurate and inefficient.
JSON.stringify(req.body).lengthre-serializes the already-parsed body, which may differ from the original payload size due to formatting differences. Additionally, this counts characters, not bytes, which underestimates size for non-ASCII content.Consider using
req.get('Content-Length')or Express raw body middleware to capture actual bytes.- const request = req.body as BlobIngestionRequest; - const rawBodySize = JSON.stringify(req.body).length; + const request = req.body as BlobIngestionRequest; + const contentLength = req.get('Content-Length'); + const rawBodySize = contentLength ? parseInt(contentLength, 10) : Buffer.byteLength(JSON.stringify(req.body));
16-21: Fragile error categorization via string comparison.Comparing
result.message === "Validation failed"is brittle—any message change breaks this logic. Consider using a discriminated union with anerrorTypefield or an error code property for reliable status determination.- if (!result.success) { - // Determine appropriate status code - if (result.message === "Validation failed") { - return res.status(400).json(result); - } - return res.status(500).json(result); - } + if (!result.success) { + const statusCode = result.errorType === 'VALIDATION_ERROR' ? 400 : 500; + return res.status(statusCode).json(result); + }This requires updating
BlobIngestionResponseto include anerrorTypediscriminator.
26-31: Consider structured logging instead of console.error.Using
console.errorloses structured context (correlation IDs, request metadata). If a logging framework is available in this codebase, use it for better observability.libs/blob-ingestion/src/blob-ingestion/validation.test.ts (1)
15-25: Missing test forvalidateListTypeJsonreturning validation errors.The mock always returns
{ isValid: true, errors: [], schemaVersion: "1.0" }. Consider adding a test case wherevalidateListTypeJsonreturns errors to verify the validation function correctly aggregates schema validation failures into the result.it("should include schema validation errors from hearing_list", async () => { const { validateListTypeJson } = await import("@hmcts/list-types-common"); vi.mocked(validateListTypeJson).mockResolvedValueOnce({ isValid: false, errors: [{ path: "/courtLists/0", message: "Invalid structure" }], schemaVersion: "1.0" }); const result = await validateBlobRequest(validRequest, 1000); expect(result.isValid).toBe(false); expect(result.errors).toContainEqual(expect.objectContaining({ field: "hearing_list" })); });libs/publication/src/provenance.ts (1)
8-13: Consider stricter typing forPROVENANCE_LABELS.Using
Record<Provenance, string>instead ofRecord<string, string>would enforce that all enum values have corresponding labels and catch missing entries at compile time.-export const PROVENANCE_LABELS: Record<string, string> = { +export const PROVENANCE_LABELS: Record<Provenance, string> = { [Provenance.MANUAL_UPLOAD]: "Manual Upload", [Provenance.XHIBIT]: "XHIBIT", [Provenance.SNL]: "SNL", [Provenance.COMMON_PLATFORM]: "Common Platform" };VIBE-209-specification.md (2)
39-39: Add language specifier to fenced code block.Per static analysis, this code block should have a language specified for proper syntax highlighting.
-``` +```text Authorization: Bearer <token>
144-158: Add language specifier to processing logic code block.The processing logic steps would benefit from a language identifier for consistency.
-``` +```text 1. Authenticate request (OAuth token validation)libs/blob-ingestion/src/middleware/oauth-middleware.ts (3)
36-42: Consider adding a typed interface forapiUserinstead of usingany.Using
(req as any).apiUserloses type safety. Consider extending the Express Request type.Add a type declaration at the bottom of the file (per coding guidelines, interfaces/types go at bottom):
declare global { namespace Express { interface Request { apiUser?: { appId: string; roles: string[]; }; } } }Then update the assignment:
- (req as any).apiUser = { + req.apiUser = { appId: claims.appid || claims.azp, roles: claims.roles || [] };
54-54: Add proper return type instead ofPromise<any>.The function should have a typed return for the JWT claims to improve type safety.
-async function validateToken(token: string): Promise<any> { +interface JwtClaims { + appid?: string; + azp?: string; + roles?: string[]; + iss?: string; + aud?: string; + exp?: number; +} + +async function validateToken(token: string): Promise<JwtClaims> {
72-78: Consider caching the JWKS client instance.The JWKS client is created on every
validateTokencall. While internal caching is enabled, instantiating the client per request adds unnecessary overhead.Move client creation outside the function:
let jwksClientInstance: ReturnType<typeof jwksClient> | null = null; function getJwksClient(tenantId: string): ReturnType<typeof jwksClient> { if (!jwksClientInstance) { jwksClientInstance = jwksClient({ jwksUri: `https://login.microsoftonline.com/${tenantId}/discovery/v2.0/keys`, cache: true, cacheMaxAge: 86400000, rateLimit: true }); } return jwksClientInstance; }Note: This assumes tenant ID is static per deployment. If multiple tenants are supported, use a Map keyed by tenant ID.
docs/tickets/VIBE-209/plan.md (2)
690-693: Reconsider body size calculation approach.Using
JSON.stringify(req.body).lengthrecalculates the body size after parsing, which is inefficient and may differ from the actual request size. Consider usingreq.get('content-length')or tracking raw body size via middleware.In the actual route implementation, prefer:
const rawBodySize = parseInt(req.get('content-length') || '0', 10);Or use
express.json({ verify: (req, res, buf) => { req.rawBodySize = buf.length; } })to capture the actual size.
123-144: Add language specifier to module structure code block.-``` +```text libs/blob-ingestion/libs/blob-ingestion/src/blob-ingestion/queries.ts (1)
31-39: Consider extracting a shared mapper function to reduce duplication.The mapping logic converting Prisma results to
IngestionLogis duplicated betweengetIngestionLogsByDateRangeandgetRecentErrorLogs. Additionally, the type assertion onstatus(lines 36, 60) bypasses type safety—if the database contains an unexpected value, this will silently pass through.+function mapToIngestionLog(log: { + id: string; + timestamp: Date; + sourceSystem: string; + courtId: string; + status: string; + errorMessage: string | null; + artefactId: string | null; +}): IngestionLog { + const validStatuses = ["SUCCESS", "VALIDATION_ERROR", "SYSTEM_ERROR"] as const; + const status = validStatuses.includes(log.status as any) + ? (log.status as IngestionLog["status"]) + : "SYSTEM_ERROR"; // fallback for unexpected values + return { + id: log.id, + timestamp: log.timestamp, + sourceSystem: log.sourceSystem, + courtId: log.courtId, + status, + errorMessage: log.errorMessage || undefined, + artefactId: log.artefactId || undefined + }; +}Then use
return logs.map(mapToIngestionLog);in both functions.Also applies to: 55-63
libs/blob-ingestion/src/middleware/oauth-middleware.test.ts (2)
40-40: Type assertion onmockNextis misleading.
mockNextis aMock<[], void>from Vitest, not aNextFunction. The assertion hides this, and the variable isn't reset between tests (unlike the response mock). Consider:- const mockNext = vi.fn() as NextFunction; + let mockNext: ReturnType<typeof vi.fn>; + + beforeEach(() => { + vi.clearAllMocks(); + mockNext = vi.fn(); + });
202-259: Environment variable cleanup may not run if test fails.Setting
process.envvalues in the test body (lines 213-214) and deleting them later (lines 257-258) risks test pollution if assertions fail before cleanup. UseafterEachorvi.stubEnvfor safer isolation.it("should use environment variables when config throws error", async () => { + vi.stubEnv("AZURE_TENANT_ID", "env-tenant-id"); + vi.stubEnv("AZURE_CLIENT_ID", "env-client-id"); + const config = await import("config"); // ... rest of test setup ... - // Set environment variables - process.env.AZURE_TENANT_ID = "env-tenant-id"; - process.env.AZURE_CLIENT_ID = "env-client-id"; // ... assertions ... - - // Clean up environment variables - delete process.env.AZURE_TENANT_ID; - delete process.env.AZURE_CLIENT_ID; });Note:
vi.stubEnvautomatically restores values after the test.libs/blob-ingestion/src/blob-ingestion/validation.ts (2)
48-58: Unnecessary string-to-number conversion forlistTypeId.
listTypeIdis declared asstring, assigned fromlistType.id.toString(), then converted back toNumber.parseInt(listTypeId, 10)on line 152. SincelistType.idis already a number, this round-trip is unnecessary.- let listTypeId: string | undefined; + let listTypeId: number | undefined; if (request.list_type) { const listType = mockListTypes.find((lt) => lt.name === request.list_type); if (!listType) { errors.push({ field: "list_type", message: `Invalid list type. Allowed values: ${mockListTypes.map((lt) => lt.name).join(", ")}` }); } else { - listTypeId = listType.id.toString(); + listTypeId = listType.id; } }Then on line 130, pass
listTypeId.toString()tovalidateListTypeJsonif it requires a string, and remove the conversion on line 152.
140-145: Swallowed error loses diagnostic information.The caught error is discarded (
_error), making it harder to debug schema validation failures. Consider logging the error details (without sensitive data) for troubleshooting.} catch (_error) { + console.error("Schema validation error:", _error instanceof Error ? _error.message : _error); errors.push({ field: "hearing_list", message: "Failed to validate hearing_list against schema" }); }libs/blob-ingestion/src/blob-ingestion/service.ts (2)
24-25: Use nullish coalescing for more precise fallback handling.The
||operator treats empty strings as falsy, which could lead to "UNKNOWN" being used when an empty string is intentionally provided. Consider using the nullish coalescing operator (??) to only fallback when the value isnullorundefined.Apply this diff:
- sourceSystem: request.provenance || "UNKNOWN", - courtId: request.court_id || "UNKNOWN", + sourceSystem: request.provenance ?? "UNKNOWN", + courtId: request.court_id ?? "UNKNOWN",
41-43: Improve error message clarity for debugging.The error message "List type ID not found after validation" could be more descriptive to help developers understand this represents an internal validation bug rather than a user error.
Apply this diff:
if (!validation.listTypeId) { - throw new Error("List type ID not found after validation"); + throw new Error("Internal error: List type ID missing after successful validation. This indicates a bug in the validation logic."); }libs/blob-ingestion/src/blob-ingestion/model.ts (1)
8-18: Consider more specific typing for hearing_list if structure is known.The
hearing_listfield is typed asunknown, which provides maximum flexibility but sacrifices type safety. If the hearing list has a known structure (even a general one likeRecord<string, unknown>or an array type), consider using a more specific type to catch errors at compile time.However, if the hearing list structure varies significantly by source system or is intentionally opaque at this layer, the current
unknowntype is appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (51)
.ai/plans/VIBE-209/plan.md(1 hunks).ai/plans/VIBE-209/specification.md(1 hunks).ai/plans/VIBE-209/tasks.md(1 hunks).gitignore(1 hunks)VIBE-209-plan.md(1 hunks)VIBE-209-specification.md(1 hunks)apps/api/helm/values.dev.yaml(1 hunks)apps/api/helm/values.yaml(1 hunks)apps/api/src/app.test.ts(1 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/server.ts(1 hunks)apps/postgres/prisma.config.ts(0 hunks)apps/postgres/prisma/migrations/20251126100649_blob_ingestion/migration.sql(1 hunks)apps/postgres/prisma/schema.prisma(1 hunks)apps/web/src/server.test.ts(1 hunks)docs/VIBE-209/plan.md(1 hunks)docs/VIBE-209/specification.md(1 hunks)docs/VIBE-209/tasks.md(1 hunks)docs/tickets/VIBE-209/plan.md(1 hunks)docs/tickets/VIBE-209/specification.md(1 hunks)docs/tickets/VIBE-209/tasks.md(1 hunks)libs/admin-pages/src/manual-upload/file-storage.ts(1 hunks)libs/admin-pages/src/manual-upload/validation.test.ts(1 hunks)libs/admin-pages/src/pages/admin-dashboard/index.test.ts(3 hunks)libs/admin-pages/src/pages/manual-upload-summary/index.test.ts(6 hunks)libs/admin-pages/src/pages/manual-upload-summary/index.ts(1 hunks)libs/admin-pages/src/pages/manual-upload/index.test.ts(18 hunks)libs/blob-ingestion/package.json(1 hunks)libs/blob-ingestion/src/blob-ingestion/file-storage.test.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/file-storage.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/model.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/queries.test.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/queries.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/service.test.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/service.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/validation.test.ts(1 hunks)libs/blob-ingestion/src/blob-ingestion/validation.ts(1 hunks)libs/blob-ingestion/src/config.ts(1 hunks)libs/blob-ingestion/src/index.ts(1 hunks)libs/blob-ingestion/src/middleware/oauth-middleware.test.ts(1 hunks)libs/blob-ingestion/src/middleware/oauth-middleware.ts(1 hunks)libs/blob-ingestion/src/routes/v1/publication.test.ts(1 hunks)libs/blob-ingestion/src/routes/v1/publication.ts(1 hunks)libs/blob-ingestion/tsconfig.json(1 hunks)libs/list-types/civil-and-family-daily-cause-list/src/pages/index.ts(2 hunks)libs/location/src/routes/locations.test.ts(1 hunks)libs/publication/src/provenance.ts(1 hunks)libs/publication/src/repository/model.ts(1 hunks)libs/publication/src/repository/queries.ts(4 hunks)package.json(2 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/postgres/prisma.config.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and Interfaces must use PascalCase (e.g.,UserService,CaseRepository). Do NOT useIprefix for interfaces (useUserRepositorynotIUserRepository).
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
TypeScript must use strict mode enabled with noanywithout justification. Use workspace aliases (@hmcts/*) for imports.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Only export functions that are intended to be used outside the module. Don't export functions solely for testing purposes.
Only add comments when they are meaningful. Explain why something is done, not what is done.
Favor functional style with simple functional approaches. Don't use a class unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Files:
libs/publication/src/repository/model.tslibs/admin-pages/src/pages/manual-upload/index.test.tslibs/admin-pages/src/pages/manual-upload-summary/index.test.tslibs/blob-ingestion/src/config.tslibs/blob-ingestion/src/blob-ingestion/file-storage.tslibs/admin-pages/src/pages/admin-dashboard/index.test.tsapps/api/src/server.tslibs/blob-ingestion/src/middleware/oauth-middleware.test.tslibs/blob-ingestion/src/blob-ingestion/queries.tslibs/blob-ingestion/src/routes/v1/publication.test.tslibs/blob-ingestion/src/blob-ingestion/service.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tsapps/api/src/app.test.tslibs/blob-ingestion/src/blob-ingestion/validation.test.tslibs/blob-ingestion/src/blob-ingestion/service.tslibs/blob-ingestion/src/index.tslibs/blob-ingestion/src/blob-ingestion/queries.test.tslibs/blob-ingestion/src/blob-ingestion/file-storage.test.tsapps/web/src/server.test.tslibs/admin-pages/src/manual-upload/validation.test.tslibs/blob-ingestion/src/routes/v1/publication.tslibs/blob-ingestion/src/blob-ingestion/model.tslibs/blob-ingestion/src/blob-ingestion/validation.tsapps/api/src/app.tslibs/admin-pages/src/manual-upload/file-storage.tslibs/location/src/routes/locations.test.tslibs/blob-ingestion/src/middleware/oauth-middleware.tslibs/admin-pages/src/pages/manual-upload-summary/index.tslibs/publication/src/provenance.tslibs/publication/src/repository/queries.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
API endpoints must use plural for collections (
/api/cases,/api/users), singular for specific resources (/api/case/:id), and singular for creation (POST /api/case).
Files:
libs/publication/src/repository/model.tslibs/admin-pages/src/pages/manual-upload/index.test.tslibs/admin-pages/src/pages/manual-upload-summary/index.test.tslibs/blob-ingestion/src/config.tslibs/blob-ingestion/src/blob-ingestion/file-storage.tslibs/admin-pages/src/pages/admin-dashboard/index.test.tsapps/api/src/server.tslibs/blob-ingestion/src/middleware/oauth-middleware.test.tslibs/blob-ingestion/src/blob-ingestion/queries.tslibs/blob-ingestion/src/routes/v1/publication.test.tslibs/blob-ingestion/src/blob-ingestion/service.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tsapps/api/src/app.test.tslibs/blob-ingestion/src/blob-ingestion/validation.test.tslibs/blob-ingestion/src/blob-ingestion/service.tslibs/blob-ingestion/src/index.tslibs/blob-ingestion/src/blob-ingestion/queries.test.tslibs/blob-ingestion/src/blob-ingestion/file-storage.test.tsapps/web/src/server.test.tslibs/admin-pages/src/manual-upload/validation.test.tslibs/blob-ingestion/src/routes/v1/publication.tslibs/blob-ingestion/src/blob-ingestion/model.tslibs/blob-ingestion/src/blob-ingestion/validation.tsapps/api/src/app.tslibs/admin-pages/src/manual-upload/file-storage.tslibs/location/src/routes/locations.test.tslibs/blob-ingestion/src/middleware/oauth-middleware.tslibs/admin-pages/src/pages/manual-upload-summary/index.tslibs/publication/src/provenance.tslibs/publication/src/repository/queries.ts
**/{pages,locales}/**/*.{ts,njk}
📄 CodeRabbit inference engine (CLAUDE.md)
Every page must support both English and Welsh by providing
enandcyobjects in controllers and maintaining matching structure in locale files (libs/[module]/src/locales/en.ts and cy.ts).
Files:
libs/admin-pages/src/pages/manual-upload/index.test.tslibs/admin-pages/src/pages/manual-upload-summary/index.test.tslibs/admin-pages/src/pages/admin-dashboard/index.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/admin-pages/src/pages/manual-upload-summary/index.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All packages must use
"test": "vitest run"as the test script. Unit and integration tests must be co-located with source code as*.test.tsfiles.
Files:
libs/admin-pages/src/pages/manual-upload/index.test.tslibs/admin-pages/src/pages/manual-upload-summary/index.test.tslibs/admin-pages/src/pages/admin-dashboard/index.test.tslibs/blob-ingestion/src/middleware/oauth-middleware.test.tslibs/blob-ingestion/src/routes/v1/publication.test.tslibs/blob-ingestion/src/blob-ingestion/service.test.tsapps/api/src/app.test.tslibs/blob-ingestion/src/blob-ingestion/validation.test.tslibs/blob-ingestion/src/blob-ingestion/queries.test.tslibs/blob-ingestion/src/blob-ingestion/file-storage.test.tsapps/web/src/server.test.tslibs/admin-pages/src/manual-upload/validation.test.tslibs/location/src/routes/locations.test.ts
**/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Page routes are created based on file names within the
pages/directory. Nested routes are created using subdirectories (e.g.,pages/admin/my-page.tsbecomes/admin/my-page).
Files:
libs/admin-pages/src/pages/manual-upload/index.test.tslibs/admin-pages/src/pages/manual-upload-summary/index.test.tslibs/admin-pages/src/pages/admin-dashboard/index.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/admin-pages/src/pages/manual-upload-summary/index.ts
**/{locales,pages}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Shared/common content (button text, phase banner, service name, common errors) should be in locale files (libs/[module]/src/locales/en.ts and cy.ts). Page-specific content should be in controllers.
Files:
libs/admin-pages/src/pages/manual-upload/index.test.tslibs/admin-pages/src/pages/manual-upload-summary/index.test.tslibs/admin-pages/src/pages/admin-dashboard/index.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/admin-pages/src/pages/manual-upload-summary/index.ts
**/config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Config exports (pageRoutes, apiRoutes, prismaSchemas, assets) must be in a separate
config.tsfile to avoid circular dependencies during Prisma client generation. Apps must import config using the/configpath (e.g.,@hmcts/my-feature/config).
Files:
libs/blob-ingestion/src/config.ts
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Package names must use @hmcts scope (e.g.,@hmcts/auth,@hmcts/case-management).
All package.json files must use"type": "module"to enforce ES modules. Never use CommonJSrequire()ormodule.exports. Useimport/exportonly.
Express version 5.x only must be used ("express": "5.1.0"). Pin all dependencies to specific versions except peer dependencies.
Build scripts must includebuild:nunjucksif the module contains Nunjucks templates in thepages/directory to copy .njk files to dist.
Files:
libs/blob-ingestion/package.jsonpackage.json
**/pages/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Page controllers must export GET and/or POST functions with signature
(req: Request, res: Response) => Promise<void>. Content (titles, descriptions) should be organized asenandcyobjects.
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All modules must have
src/index.tsfor business logic exports separate fromsrc/config.ts.
Files:
libs/blob-ingestion/src/index.ts
**/*-middleware.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable middleware must be placed in a dedicated
libs/[module]/src/[middleware-name]-middleware.tsfile and exported as a function.
Files:
libs/blob-ingestion/src/middleware/oauth-middleware.ts
**/*.prisma
📄 CodeRabbit inference engine (CLAUDE.md)
Database tables and fields MUST be singular and snake_case (e.g.,
user,case,created_at). Use Prisma@@mapand@mapfor aliases.
Files:
apps/postgres/prisma/schema.prisma
🧠 Learnings (16)
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.test.ts : All packages must use `"test": "vitest run"` as the test script. Unit and integration tests must be co-located with source code as `*.test.ts` files.
Applied to files:
libs/admin-pages/src/pages/manual-upload/index.test.tslibs/admin-pages/src/pages/manual-upload-summary/index.test.tslibs/admin-pages/src/pages/admin-dashboard/index.test.tslibs/blob-ingestion/package.jsonlibs/blob-ingestion/src/blob-ingestion/service.test.tslibs/blob-ingestion/src/blob-ingestion/file-storage.test.tslibs/blob-ingestion/tsconfig.jsondocs/tickets/VIBE-209/plan.mdlibs/location/src/routes/locations.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/config.ts : Config exports (pageRoutes, apiRoutes, prismaSchemas, assets) must be in a separate `config.ts` file to avoid circular dependencies during Prisma client generation. Apps must import config using the `/config` path (e.g., `hmcts/my-feature/config`).
Applied to files:
libs/blob-ingestion/src/config.tsapps/api/src/server.tsdocs/VIBE-209/specification.mdtsconfig.jsonlibs/blob-ingestion/tsconfig.jsonapps/api/src/app.tslibs/location/src/routes/locations.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to libs/*/src/index.ts : All modules must have `src/index.ts` for business logic exports separate from `src/config.ts`.
Applied to files:
libs/blob-ingestion/src/config.tslibs/blob-ingestion/package.jsondocs/VIBE-209/specification.mdlibs/blob-ingestion/src/index.tstsconfig.jsonlibs/blob-ingestion/tsconfig.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : Always add `.js` extension to relative imports (e.g., `import { foo } from "./bar.js"`). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
Applied to files:
libs/blob-ingestion/src/config.tsapps/api/src/server.tslibs/blob-ingestion/tsconfig.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Applied to files:
apps/api/src/server.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tsapps/api/src/app.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/package.json : All package.json files must use `"type": "module"` to enforce ES modules. Never use CommonJS `require()` or `module.exports`. Use `import`/`export` only.
Applied to files:
libs/blob-ingestion/package.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
Applied to files:
libs/blob-ingestion/package.jsonlibs/blob-ingestion/tsconfig.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Applied to files:
docs/VIBE-209/specification.mdlibs/blob-ingestion/src/blob-ingestion/queries.tslibs/blob-ingestion/src/blob-ingestion/queries.test.tsdocs/tickets/VIBE-209/plan.md
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/pages/*.ts : Page controllers must export GET and/or POST functions with signature `(req: Request, res: Response) => Promise<void>`. Content (titles, descriptions) should be organized as `en` and `cy` objects.
Applied to files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/blob-ingestion/src/routes/v1/publication.tslibs/location/src/routes/locations.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/{locales,pages}/**/*.ts : Shared/common content (button text, phase banner, service name, common errors) should be in locale files (libs/[module]/src/locales/en.ts and cy.ts). Page-specific content should be in controllers.
Applied to files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/admin-pages/src/manual-upload/validation.test.tslibs/location/src/routes/locations.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*-middleware.ts : Reusable middleware must be placed in a dedicated `libs/[module]/src/[middleware-name]-middleware.ts` file and exported as a function.
Applied to files:
libs/blob-ingestion/src/index.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : TypeScript must use strict mode enabled with no `any` without justification. Use workspace aliases (`hmcts/*`) for imports.
Applied to files:
tsconfig.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/package.json : Package names must use hmcts scope (e.g., `hmcts/auth`, `hmcts/case-management`).
Applied to files:
tsconfig.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : Only export functions that are intended to be used outside the module. Don't export functions solely for testing purposes.
Applied to files:
apps/web/src/server.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/types.ts : Don't create types.ts files. Colocate types with the appropriate code.
Applied to files:
libs/blob-ingestion/tsconfig.json
📚 Learning: 2025-11-25T06:40:35.587Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: docs/AGENT.md:0-0
Timestamp: 2025-11-25T06:40:35.587Z
Learning: Applies to docs/**/CLAUDE.md : Use # at start of input as a memory shortcut to add to CLAUDE.md
Applied to files:
.gitignore
🧬 Code graph analysis (12)
libs/admin-pages/src/pages/manual-upload-summary/index.test.ts (1)
libs/admin-pages/src/manual-upload/file-storage.ts (1)
saveUploadedFile(12-23)
libs/blob-ingestion/src/blob-ingestion/file-storage.ts (1)
libs/admin-pages/src/manual-upload/file-storage.ts (1)
saveUploadedFile(12-23)
libs/blob-ingestion/src/middleware/oauth-middleware.test.ts (2)
libs/blob-ingestion/src/middleware/oauth-middleware.ts (1)
authenticateApi(12-52)libs/blob-ingestion/src/index.ts (1)
authenticateApi(4-4)
libs/blob-ingestion/src/blob-ingestion/queries.ts (1)
libs/blob-ingestion/src/blob-ingestion/model.ts (1)
IngestionLog(40-48)
libs/blob-ingestion/src/routes/v1/publication.test.ts (2)
libs/blob-ingestion/src/blob-ingestion/service.ts (1)
processBlobIngestion(15-97)libs/blob-ingestion/src/blob-ingestion/model.ts (1)
BlobIngestionRequest(8-18)
libs/blob-ingestion/src/blob-ingestion/service.test.ts (6)
libs/blob-ingestion/src/blob-ingestion/model.ts (1)
BlobIngestionRequest(8-18)libs/blob-ingestion/src/blob-ingestion/validation.ts (1)
validateBlobRequest(9-154)libs/publication/src/repository/queries.ts (1)
createArtefact(4-52)libs/blob-ingestion/src/blob-ingestion/service.ts (1)
processBlobIngestion(15-97)libs/blob-ingestion/src/blob-ingestion/file-storage.ts (1)
saveUploadedFile(12-23)libs/blob-ingestion/src/blob-ingestion/queries.ts (1)
createIngestionLog(4-16)
libs/blob-ingestion/src/blob-ingestion/validation.test.ts (2)
libs/blob-ingestion/src/blob-ingestion/model.ts (1)
BlobIngestionRequest(8-18)libs/blob-ingestion/src/blob-ingestion/validation.ts (1)
validateBlobRequest(9-154)
libs/blob-ingestion/src/blob-ingestion/queries.test.ts (2)
libs/blob-ingestion/src/blob-ingestion/model.ts (1)
IngestionLog(40-48)libs/blob-ingestion/src/blob-ingestion/queries.ts (3)
createIngestionLog(4-16)getIngestionLogsByDateRange(18-40)getRecentErrorLogs(42-64)
libs/blob-ingestion/src/blob-ingestion/validation.ts (2)
libs/blob-ingestion/src/blob-ingestion/model.ts (3)
BlobIngestionRequest(8-18)BlobValidationResult(33-38)ValidationError(28-31)libs/publication/src/index.ts (2)
Sensitivity(7-7)Language(2-2)
libs/blob-ingestion/src/middleware/oauth-middleware.ts (1)
libs/blob-ingestion/src/index.ts (1)
authenticateApi(4-4)
libs/admin-pages/src/pages/manual-upload-summary/index.ts (1)
libs/publication/src/index.ts (1)
Provenance(4-4)
libs/publication/src/provenance.ts (1)
libs/publication/src/index.ts (2)
PROVENANCE_LABELS(4-4)Provenance(4-4)
🪛 LanguageTool
docs/VIBE-209/specification.md
[grammar] ~1038-~1038: Ensure spelling is correct
Context: ...ations 1. Response Time Target - < 500ms for valid requests 2. **Throughput Targ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/VIBE-209/tasks.md
[style] ~209-~209: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...g time - [ ] Return IngestionResult with success - [ ] Catch exceptions and return e...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[grammar] ~711-~711: Ensure spelling is correct
Context: ... [ ] Confirm notification mechanism for noMatch incidents - [ ] Confirm schema versioni...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~711-~711: Ensure spelling is correct
Context: ...fication mechanism for noMatch incidents - [ ] Confirm schema versioning strategy -...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/VIBE-209/plan.md
[uncategorized] ~243-~243: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ole("api.publisher.user")`) - [ ] Apply rate limiting middleware - [ ] Add request size limit...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~316-~316: Ensure spelling is correct
Context: ...mance Testing - [ ] Test response time (<500ms target) - [ ] Test throughput (1000 req...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~476-~476: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...lag allows ingestion, alerts admins | | High volume ingestion impacts other services | High...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~565-~565: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...imize validation logic if needed - Tune rate limiting - Review and optimize audit log retenti...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/tickets/VIBE-209/plan.md
[uncategorized] ~1312-~1312: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-azure-ad or jsonwebtoken + jwks-rsa) - Rate limiting library (express-rate-limit) ## 18. Su...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
docs/VIBE-209/specification.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
296-296: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
408-408: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
537-537: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
874-874: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
VIBE-209-specification.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
144-144: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
176-176: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/tickets/VIBE-209/plan.md
123-123: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
990-990: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🎭 Playwright E2E Test Results419 tests 419 ✅ 34m 15s ⏱️ Results for commit e658848. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (4)
libs/api/src/routes/v1/publication.test.ts (1)
149-169: The no_match test still expects incorrect status code.This test expects a 500 status code for the no_match scenario, but as noted in the previous review, per the VIBE-209 specification (line 989), a no_match scenario should return a successful status (200 or 201) with
noMatch: truein the response body, not a server error.The issue remains unresolved:
- The handler in publication.ts (lines 16-21) treats
success: falseas a 500 error- The service should return
success: truewithno_match: truefor this scenario- This test should verify status 201 (or 200) with the no_match flag in the response
This aligns with the previous review feedback. Both the handler logic and this test need correction to treat no_match as a successful business outcome rather than an error.
libs/api/src/blob-ingestion/validation.ts (1)
156-163: Date validation functions are too permissive.This issue was previously identified.
isValidISODateonly verifies the string starts with a date pattern but doesn't validate actual date values.isValidISODateTimeonly checks for presence of "T". Invalid dates like"2025-13-45"or malformed datetimes like"2025-01-01Txyz"will pass validation.Apply stricter validation:
function isValidISODate(dateString: string): boolean { - const date = new Date(dateString); - return !Number.isNaN(date.getTime()) && /^\d{4}-\d{2}-\d{2}/.test(dateString); + const regex = /^\d{4}-\d{2}-\d{2}$/; + if (!regex.test(dateString)) return false; + const date = new Date(dateString + "T00:00:00Z"); + if (Number.isNaN(date.getTime())) return false; + return date.toISOString().startsWith(dateString); } function isValidISODateTime(dateString: string): boolean { - const date = new Date(dateString); - return !Number.isNaN(date.getTime()) && dateString.includes("T"); + const regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|[+-]\d{2}:\d{2})?$/; + if (!regex.test(dateString)) return false; + const date = new Date(dateString); + return !Number.isNaN(date.getTime()); }libs/api/src/middleware/oauth-middleware.ts (1)
44-50: Avoid logging potentially sensitive authentication error details.The error object may contain token fragments or other sensitive authentication data. Per coding guidelines, sensitive data should never be included in logs.
} catch (error) { - console.error("API authentication error:", error); + console.error("API authentication error:", error instanceof Error ? error.message : "Unknown error"); return res.status(401).json({ success: false, message: "Invalid or expired token" }); }libs/api/src/middleware/oauth-middleware.test.ts (1)
264-264: Unused import_jwksClient.The imported
_jwksClientis not used in this test case.- const _jwksClient = await import("jwks-rsa");
🧹 Nitpick comments (19)
libs/auth/src/pages/sso-rejected/index.test.ts (2)
40-41: Good improvement, but consider applying consistently.The use of
vi.mocked()wrapper and optional chaining improves type safety and defensive programming. However, the first test (lines 15-28) directly accessesres.renderwithout this pattern, creating inconsistency.Consider applying the same pattern in the first test for consistency, or if the first test's simpler assertion approach is sufficient (using
toHaveBeenCalledWithwith matchers), you could stick with that pattern throughout.Also applies to: 57-58
41-41: Consider typing the content instead of usingas any.The
as anytype assertion reduces the type safety benefits of usingvi.mocked(). You could define an interface for the page content structure:+interface SsoRejectedContent { + en: { + title: string; + header: string; + paragraph1: string; + linkText: string; + }; + cy: { + title: string; + header: string; + paragraph1: string; + linkText: string; + }; +} + const renderCall = vi.mocked(res.render).mock.calls[0]; -const content = renderCall?.[1] as any; +const content = renderCall?.[1] as SsoRejectedContent;This maintains strict typing while still being flexible for test assertions.
Also applies to: 58-58
libs/web-core/src/middleware/helmet/helmet-middleware.test.ts (1)
111-111: Avoidas anytype casts—properly type the helmet mock instead.The use of
as anyappears 16 times to access helmet'scontentSecurityPolicyconfiguration, defeating TypeScript's type safety without justification.As per coding guidelines, TypeScript must not use
anywithout justification. Consider these alternatives:
- Type the helmet mock call result:
import type { HelmetOptions } from 'helmet'; const helmetCall = vi.mocked(helmet).mock.calls[0][0] as HelmetOptions; const directives = helmetCall?.contentSecurityPolicy?.directives;
- Create a typed helper:
function getHelmetCallDirectives() { const helmetCall = vi.mocked(helmet).mock.calls[0][0] as HelmetOptions; return helmetCall?.contentSecurityPolicy?.directives; }This would restore type safety and enable IntelliSense for CSP directives.
Also applies to: 123-123, 136-136, 150-150, 166-166, 183-183, 200-200, 211-211, 222-222, 234-234, 247-247, 254-254, 263-263, 272-272, 281-281, 291-291
libs/web-core/src/middleware/session-stores/postgres-store.test.ts (1)
60-60: LGTM! Test data now matches express-session cookie structure.The addition of
originalMaxAgealongsidemaxAgein cookie objects correctly reflects the structure used by express-session. The changes are applied consistently across all test cases.Optional: Consider defining a proper type for test session data.
Instead of casting as
any, you could define a test helper type to improve type safety:type TestSessionData = { cookie: { maxAge: number; originalMaxAge: number }; userId?: string; };Then use it in tests:
-const sessionData = { cookie: { maxAge: 3600000, originalMaxAge: 3600000 }, userId: "123" } as any; +const sessionData: TestSessionData = { cookie: { maxAge: 3600000, originalMaxAge: 3600000 }, userId: "123" };Optional: Consider adding edge case tests.
All current tests use matching values for
maxAgeandoriginalMaxAge. You might want to add tests for:
- Sessions where
originalMaxAgediffers frommaxAge(e.g., after a touch operation)- Sessions where
originalMaxAgeis undefined (legacy sessions)This would ensure the store handles these scenarios correctly.
Also applies to: 106-106, 124-124, 148-148, 170-170
libs/web-core/src/middleware/i18n/locale-middleware.test.ts (1)
436-459: Align test name with updated “undefined options” behaviorThe test now explicitly calls
res.render("test-view", undefined)and the inline comment mentions “undefined options”, but the test name still says "should handle null options". This can be slightly confusing.Consider renaming the test to match the behavior:
- it("should handle null options", () => { + it("should handle undefined options", () => {(or “no options” if you want to stay neutral on null vs undefined).
libs/admin-pages/src/manual-upload/file-storage.test.ts (1)
3-16: Path resolution change aligns tests with implementationUsing
fileURLToPath(import.meta.url)and derivingMONOREPO_ROOT/TEST_STORAGE_BASEfrom__dirnamematches the implementation’s logic and avoidsprocess.cwd()flakiness. This should make these tests more robust across runners. If more modules start computing this root path, consider a small shared helper to avoid divergence, but it’s fine as‑is here.libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts (1)
72-86: Reduce duplication in artefact mocks and tighten typesThe
mockArtefactobject with the extended fields (locationId,sensitivity,lastReceivedDate,isFlatFile,provenance,supersededCount,noMatch) is repeated almost verbatim across multiple tests, with only small differences (primarilyprovenance). This makes future schema changes harder to apply consistently.Consider extracting a small factory/helper, e.g.
buildMockArtefact(overrides?: Partial<ArtefactLike>), and (optionally) typing it against the actual Artefact type from@hmcts/postgres/Prisma to avoid the repeatedas anycasts in this file.Also applies to: 99-113, 133-146, 192-205, 243-256, 302-316, 353-366
libs/api/src/blob-ingestion/repository/model.ts (1)
8-48: Blob ingestion models are clear; consider tightening a few string fields laterThe request/response and internal result models read cleanly and match the blob‑ingestion flow described in the PR (
no_match, validation errors, ingestion log, etc.), and usingunknownforhearing_listis preferable toanywhile the shape is still evolving.As the API stabilises, you may want to:
- Narrow
provenance,sensitivity,language, andstatusto string‑literal unions (or shared enums) so they stay aligned with Prisma/OpenAPI.- Replace
hearing_list: unknownwith a concrete type once the hearing list schema is finalised.These can be deferred but will strengthen type safety across the ingestion pipeline.
e2e-tests/tests/api/blob-ingestion.spec.ts (1)
1-271: Consider adding positive test cases with valid authentication.The current E2E test suite primarily validates authentication failures and input validation errors using invalid tokens. While these negative tests are valuable, the suite lacks positive test cases that verify the complete ingestion flow with valid authentication and valid payloads.
Consider adding test cases that:
- Successfully authenticate with a valid test token
- Submit a valid payload and verify 201 response with artefact_id
- Test the no_match scenario with valid auth
- Verify the artefact is created in the database
- Verify the ingestion log is recorded
This would provide end-to-end validation of the happy path, complementing the existing negative tests.
libs/api/src/blob-ingestion/repository/service.ts (1)
81-90: Potential sensitive data in error logs.Per coding guidelines, sensitive data should never be included in logs. The
error.messagemay contain internal details. Consider logging a sanitized version.await createIngestionLog({ id: randomUUID(), timestamp: new Date(), sourceSystem: request.provenance, courtId: request.court_id, status: "SYSTEM_ERROR", - errorMessage: error instanceof Error ? error.message : "Unknown error" + errorMessage: "Ingestion processing failed" });If detailed error tracking is needed, consider a separate internal logging mechanism that doesn't persist to the database.
libs/api/src/blob-ingestion/repository/queries.ts (2)
31-39: Consider extracting duplicate log mapping logic.The same mapping logic is repeated in both
getIngestionLogsByDateRangeandgetRecentErrorLogs. Extract to a helper function for DRY compliance.function mapPrismaLogToIngestionLog(log: PrismaIngestionLog): IngestionLog { return { id: log.id, timestamp: log.timestamp, sourceSystem: log.sourceSystem, courtId: log.courtId, status: log.status as IngestionLog["status"], errorMessage: log.errorMessage || undefined, artefactId: log.artefactId || undefined }; }Then use:
return logs.map(mapPrismaLogToIngestionLog);Also applies to: 55-63
18-40: Consider validating date range inputs.
getIngestionLogsByDateRangeaccepts Date objects directly without validation. If callers pass invalid dates, the query behavior may be undefined.export async function getIngestionLogsByDateRange(startDate: Date, endDate: Date): Promise<IngestionLog[]> { + if (!(startDate instanceof Date) || Number.isNaN(startDate.getTime()) || + !(endDate instanceof Date) || Number.isNaN(endDate.getTime())) { + throw new Error("Invalid date range provided"); + } const logs = await prisma.ingestionLog.findMany({libs/api/src/middleware/oauth-middleware.ts (3)
37-41: Extend Express Request type instead of usinganycast.The
(req as any).apiUserpattern loses type safety. Define a proper interface extension.Add at the bottom of the file (per module ordering guidelines):
export interface ApiUser { appId: string; roles: string[]; } declare global { namespace Express { interface Request { apiUser?: ApiUser; } } }Then update the assignment:
- (req as any).apiUser = { + req.apiUser = { appId: claims.appid || claims.azp, roles: claims.roles || [] };
54-54: Add proper return type forvalidateToken.
Promise<any>loses type safety. Define an interface for the expected claims.+interface TokenClaims { + appid?: string; + azp?: string; + roles?: string[]; + iss?: string; + aud?: string; +} -async function validateToken(token: string): Promise<any> { +async function validateToken(token: string): Promise<TokenClaims> {
72-78: Consider caching JWKS client instance.A new JWKS client is created on every request. While the client itself caches keys, instantiating it repeatedly adds overhead. Consider module-level caching keyed by tenant.
const jwksClients = new Map<string, ReturnType<typeof jwksClient>>(); function getJwksClient(tenantId: string) { if (!jwksClients.has(tenantId)) { jwksClients.set(tenantId, jwksClient({ jwksUri: `https://login.microsoftonline.com/${tenantId}/discovery/v2.0/keys`, cache: true, cacheMaxAge: 86400000, rateLimit: true })); } return jwksClients.get(tenantId)!; }libs/api/src/middleware/oauth-middleware.test.ts (2)
212-258: Usevi.stubEnvfor environment variable tests.Direct manipulation of
process.envcan cause test pollution across parallel test runs. Vitest providesvi.stubEnvfor safer isolation.- // Set environment variables - process.env.AZURE_TENANT_ID = "env-tenant-id"; - process.env.AZURE_CLIENT_ID = "env-client-id"; + // Set environment variables using vi.stubEnv for isolation + vi.stubEnv("AZURE_TENANT_ID", "env-tenant-id"); + vi.stubEnv("AZURE_CLIENT_ID", "env-client-id"); // ... test code ... - // Clean up environment variables - delete process.env.AZURE_TENANT_ID; - delete process.env.AZURE_CLIENT_ID; + // vi.stubEnv is automatically restored by vi.clearAllMocks in beforeEachNote: You may need to call
vi.unstubAllEnvs()inbeforeEachor usevi.restoreAllMocks()to ensure cleanup.
40-40: Type assertion formockNextis incorrect.
mockNextis declared asNextFunctionbut assigned avi.fn(). This works at runtime but the type assertion is misleading.- const mockNext = vi.fn() as NextFunction; + const mockNext: NextFunction = vi.fn();Or more explicitly:
const mockNext = vi.fn() as unknown as NextFunction;libs/publication/src/repository/queries.test.ts (2)
54-54: Avoidas anycasts oncreateArtefactfixtures by updating them to the new shapeThe repeated
createArtefact(artefactData as any)calls are only needed because the fixtures don’t include the newnoMatchfield introduced onArtefact. This sidesteps strict typing rather than keeping tests aligned with the domain model.Consider adding
noMatch: falseto eachartefactDataobject used withcreateArtefactand typing them appropriately (e.g. importing theArtefacttype from./model.js), so the calls can be made withoutas any. That will keep tests honest with respect to the currentArtefactshape and your “noanyin TS” guideline.Also applies to: 130-130, 190-190, 230-230, 295-295, 340-340
375-396: Optionally assertnoMatch/supersededCountin retrieval testsThe
getArtefactsByLocationandgetArtefactsByIdsmocks now includelastReceivedDate,isFlatFile,provenance,supersededCount, andnoMatch, but the assertions only check IDs and array length. That means any regression in mapping these new fields out of Prisma rows would go unnoticed.If you want coverage for the new fields with minimal noise, you could add expectations like:
expect(result[0]).toMatchObject({ noMatch: false });(and similarly for
supersededCount) in at least one of the retrieval tests.Also applies to: 426-442, 460-491
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
.github/workflows/e2e.yml(1 hunks).gitignore(1 hunks)e2e-tests/tests/api/blob-ingestion.spec.ts(1 hunks)libs/admin-pages/src/manual-upload/file-storage.test.ts(1 hunks)libs/admin-pages/src/pages/admin-dashboard/index.test.ts(3 hunks)libs/admin-pages/src/pages/manual-upload-summary/index.test.ts(8 hunks)libs/admin-pages/src/pages/manual-upload/index.test.ts(20 hunks)libs/api/package.json(1 hunks)libs/api/src/blob-ingestion/file-storage.test.ts(1 hunks)libs/api/src/blob-ingestion/file-storage.ts(1 hunks)libs/api/src/blob-ingestion/repository/model.ts(1 hunks)libs/api/src/blob-ingestion/repository/queries.test.ts(1 hunks)libs/api/src/blob-ingestion/repository/queries.ts(1 hunks)libs/api/src/blob-ingestion/repository/service.test.ts(1 hunks)libs/api/src/blob-ingestion/repository/service.ts(1 hunks)libs/api/src/blob-ingestion/validation.test.ts(1 hunks)libs/api/src/blob-ingestion/validation.ts(1 hunks)libs/api/src/config.test.ts(1 hunks)libs/api/src/config.ts(1 hunks)libs/api/src/index.ts(1 hunks)libs/api/src/middleware/oauth-middleware.test.ts(1 hunks)libs/api/src/middleware/oauth-middleware.ts(1 hunks)libs/api/src/routes/v1/publication.test.ts(1 hunks)libs/api/src/routes/v1/publication.ts(1 hunks)libs/api/tsconfig.json(1 hunks)libs/auth/src/pages/sso-rejected/index.test.ts(2 hunks)libs/cloud-native-platform/src/properties-volume/properties.test.ts(11 hunks)libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts(12 hunks)libs/publication/src/repository/queries.test.ts(12 hunks)libs/web-core/src/middleware/helmet/helmet-middleware.test.ts(15 hunks)libs/web-core/src/middleware/i18n/locale-middleware.test.ts(5 hunks)libs/web-core/src/middleware/session-stores/postgres-store.test.ts(5 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- libs/api/src/config.test.ts
- libs/cloud-native-platform/src/properties-volume/properties.test.ts
- libs/api/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/admin-pages/src/pages/admin-dashboard/index.test.ts
- libs/admin-pages/src/pages/manual-upload/index.test.ts
- libs/admin-pages/src/pages/manual-upload-summary/index.test.ts
- .gitignore
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and Interfaces must use PascalCase (e.g.,UserService,CaseRepository). Do NOT useIprefix for interfaces (useUserRepositorynotIUserRepository).
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
TypeScript must use strict mode enabled with noanywithout justification. Use workspace aliases (@hmcts/*) for imports.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Only export functions that are intended to be used outside the module. Don't export functions solely for testing purposes.
Only add comments when they are meaningful. Explain why something is done, not what is done.
Favor functional style with simple functional approaches. Don't use a class unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Files:
libs/web-core/src/middleware/session-stores/postgres-store.test.tslibs/api/src/index.tslibs/api/src/blob-ingestion/file-storage.tslibs/api/src/blob-ingestion/validation.test.tslibs/api/src/middleware/oauth-middleware.tse2e-tests/tests/api/blob-ingestion.spec.tslibs/api/src/routes/v1/publication.tslibs/api/src/config.tslibs/api/src/blob-ingestion/validation.tslibs/api/src/blob-ingestion/repository/queries.test.tslibs/api/src/blob-ingestion/repository/queries.tslibs/api/src/blob-ingestion/repository/service.tslibs/api/src/middleware/oauth-middleware.test.tslibs/api/src/blob-ingestion/file-storage.test.tslibs/auth/src/pages/sso-rejected/index.test.tslibs/web-core/src/middleware/helmet/helmet-middleware.test.tslibs/admin-pages/src/manual-upload/file-storage.test.tslibs/publication/src/repository/queries.test.tslibs/api/src/blob-ingestion/repository/model.tslibs/web-core/src/middleware/i18n/locale-middleware.test.tslibs/api/src/routes/v1/publication.test.tslibs/api/src/blob-ingestion/repository/service.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
API endpoints must use plural for collections (
/api/cases,/api/users), singular for specific resources (/api/case/:id), and singular for creation (POST /api/case).
Files:
libs/web-core/src/middleware/session-stores/postgres-store.test.tslibs/api/src/index.tslibs/api/src/blob-ingestion/file-storage.tslibs/api/src/blob-ingestion/validation.test.tslibs/api/src/middleware/oauth-middleware.tse2e-tests/tests/api/blob-ingestion.spec.tslibs/api/src/routes/v1/publication.tslibs/api/src/config.tslibs/api/src/blob-ingestion/validation.tslibs/api/src/blob-ingestion/repository/queries.test.tslibs/api/src/blob-ingestion/repository/queries.tslibs/api/src/blob-ingestion/repository/service.tslibs/api/src/middleware/oauth-middleware.test.tslibs/api/src/blob-ingestion/file-storage.test.tslibs/auth/src/pages/sso-rejected/index.test.tslibs/web-core/src/middleware/helmet/helmet-middleware.test.tslibs/admin-pages/src/manual-upload/file-storage.test.tslibs/publication/src/repository/queries.test.tslibs/api/src/blob-ingestion/repository/model.tslibs/web-core/src/middleware/i18n/locale-middleware.test.tslibs/api/src/routes/v1/publication.test.tslibs/api/src/blob-ingestion/repository/service.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All packages must use
"test": "vitest run"as the test script. Unit and integration tests must be co-located with source code as*.test.tsfiles.
Files:
libs/web-core/src/middleware/session-stores/postgres-store.test.tslibs/api/src/blob-ingestion/validation.test.tslibs/api/src/blob-ingestion/repository/queries.test.tslibs/api/src/middleware/oauth-middleware.test.tslibs/api/src/blob-ingestion/file-storage.test.tslibs/auth/src/pages/sso-rejected/index.test.tslibs/web-core/src/middleware/helmet/helmet-middleware.test.tslibs/admin-pages/src/manual-upload/file-storage.test.tslibs/publication/src/repository/queries.test.tslibs/web-core/src/middleware/i18n/locale-middleware.test.tslibs/api/src/routes/v1/publication.test.tslibs/api/src/blob-ingestion/repository/service.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All modules must have
src/index.tsfor business logic exports separate fromsrc/config.ts.
Files:
libs/api/src/index.ts
**/*-middleware.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable middleware must be placed in a dedicated
libs/[module]/src/[middleware-name]-middleware.tsfile and exported as a function.
Files:
libs/api/src/middleware/oauth-middleware.ts
**/config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Config exports (pageRoutes, apiRoutes, prismaSchemas, assets) must be in a separate
config.tsfile to avoid circular dependencies during Prisma client generation. Apps must import config using the/configpath (e.g.,@hmcts/my-feature/config).
Files:
libs/api/src/config.ts
**/{pages,locales}/**/*.{ts,njk}
📄 CodeRabbit inference engine (CLAUDE.md)
Every page must support both English and Welsh by providing
enandcyobjects in controllers and maintaining matching structure in locale files (libs/[module]/src/locales/en.ts and cy.ts).
Files:
libs/auth/src/pages/sso-rejected/index.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Page routes are created based on file names within the
pages/directory. Nested routes are created using subdirectories (e.g.,pages/admin/my-page.tsbecomes/admin/my-page).
Files:
libs/auth/src/pages/sso-rejected/index.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/{locales,pages}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Shared/common content (button text, phase banner, service name, common errors) should be in locale files (libs/[module]/src/locales/en.ts and cy.ts). Page-specific content should be in controllers.
Files:
libs/auth/src/pages/sso-rejected/index.test.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Package names must use @hmcts scope (e.g.,@hmcts/auth,@hmcts/case-management).
All package.json files must use"type": "module"to enforce ES modules. Never use CommonJSrequire()ormodule.exports. Useimport/exportonly.
Express version 5.x only must be used ("express": "5.1.0"). Pin all dependencies to specific versions except peer dependencies.
Build scripts must includebuild:nunjucksif the module contains Nunjucks templates in thepages/directory to copy .njk files to dist.
Files:
libs/api/package.json
**/pages/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Page controllers must export GET and/or POST functions with signature
(req: Request, res: Response) => Promise<void>. Content (titles, descriptions) should be organized asenandcyobjects.
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
🧠 Learnings (12)
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to libs/*/src/index.ts : All modules must have `src/index.ts` for business logic exports separate from `src/config.ts`.
Applied to files:
libs/api/src/index.tslibs/api/src/config.tstsconfig.jsonlibs/api/package.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*-middleware.ts : Reusable middleware must be placed in a dedicated `libs/[module]/src/[middleware-name]-middleware.ts` file and exported as a function.
Applied to files:
libs/api/src/index.tslibs/api/src/middleware/oauth-middleware.tslibs/api/src/middleware/oauth-middleware.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/pages/*.ts : Page controllers must export GET and/or POST functions with signature `(req: Request, res: Response) => Promise<void>`. Content (titles, descriptions) should be organized as `en` and `cy` objects.
Applied to files:
libs/api/src/routes/v1/publication.tslibs/auth/src/pages/sso-rejected/index.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/config.ts : Config exports (pageRoutes, apiRoutes, prismaSchemas, assets) must be in a separate `config.ts` file to avoid circular dependencies during Prisma client generation. Apps must import config using the `/config` path (e.g., `hmcts/my-feature/config`).
Applied to files:
libs/api/src/config.tstsconfig.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/pages/**/*.ts : Page routes are created based on file names within the `pages/` directory. Nested routes are created using subdirectories (e.g., `pages/admin/my-page.ts` becomes `/admin/my-page`).
Applied to files:
libs/api/src/config.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : TypeScript must use strict mode enabled with no `any` without justification. Use workspace aliases (`hmcts/*`) for imports.
Applied to files:
tsconfig.jsonlibs/web-core/src/middleware/helmet/helmet-middleware.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
Applied to files:
tsconfig.jsonlibs/api/package.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Applied to files:
libs/api/src/blob-ingestion/repository/queries.test.tslibs/api/src/blob-ingestion/repository/queries.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.test.ts : All packages must use `"test": "vitest run"` as the test script. Unit and integration tests must be co-located with source code as `*.test.ts` files.
Applied to files:
libs/api/src/blob-ingestion/file-storage.test.tslibs/auth/src/pages/sso-rejected/index.test.tslibs/api/package.jsonlibs/admin-pages/src/manual-upload/file-storage.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/package.json : All package.json files must use `"type": "module"` to enforce ES modules. Never use CommonJS `require()` or `module.exports`. Use `import`/`export` only.
Applied to files:
libs/api/package.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/package.json : Package names must use hmcts scope (e.g., `hmcts/auth`, `hmcts/case-management`).
Applied to files:
libs/api/package.json
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/{locales,pages}/**/*.ts : Shared/common content (button text, phase banner, service name, common errors) should be in locale files (libs/[module]/src/locales/en.ts and cy.ts). Page-specific content should be in controllers.
Applied to files:
libs/web-core/src/middleware/i18n/locale-middleware.test.ts
🧬 Code graph analysis (10)
libs/api/src/blob-ingestion/validation.test.ts (2)
libs/api/src/blob-ingestion/repository/model.ts (1)
BlobIngestionRequest(8-18)libs/api/src/blob-ingestion/validation.ts (1)
validateBlobRequest(9-154)
libs/api/src/middleware/oauth-middleware.ts (2)
libs/api/src/index.ts (1)
authenticateApi(4-4)e2e-tests/run-with-credentials.js (1)
client(31-31)
libs/api/src/blob-ingestion/repository/queries.test.ts (2)
libs/api/src/blob-ingestion/repository/model.ts (1)
IngestionLog(40-48)libs/api/src/blob-ingestion/repository/queries.ts (3)
createIngestionLog(4-16)getIngestionLogsByDateRange(18-40)getRecentErrorLogs(42-64)
libs/api/src/blob-ingestion/repository/queries.ts (1)
libs/api/src/blob-ingestion/repository/model.ts (1)
IngestionLog(40-48)
libs/api/src/middleware/oauth-middleware.test.ts (2)
libs/api/src/index.ts (1)
authenticateApi(4-4)libs/api/src/middleware/oauth-middleware.ts (1)
authenticateApi(12-52)
libs/api/src/blob-ingestion/file-storage.test.ts (1)
libs/api/src/blob-ingestion/file-storage.ts (1)
saveUploadedFile(12-23)
libs/publication/src/repository/queries.test.ts (2)
libs/publication/src/repository/queries.ts (1)
createArtefact(4-52)libs/publication/src/index.ts (1)
createArtefact(6-6)
libs/api/src/routes/v1/publication.test.ts (3)
libs/api/src/blob-ingestion/repository/service.ts (1)
processBlobIngestion(15-97)libs/api/src/routes/v1/publication.ts (1)
POST(7-34)libs/api/src/blob-ingestion/repository/model.ts (1)
BlobIngestionRequest(8-18)
libs/api/src/blob-ingestion/repository/service.test.ts (5)
libs/api/src/blob-ingestion/repository/model.ts (1)
BlobIngestionRequest(8-18)libs/api/src/blob-ingestion/validation.ts (1)
validateBlobRequest(9-154)libs/api/src/blob-ingestion/repository/service.ts (1)
processBlobIngestion(15-97)libs/api/src/blob-ingestion/file-storage.ts (1)
saveUploadedFile(12-23)libs/api/src/blob-ingestion/repository/queries.ts (1)
createIngestionLog(4-16)
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts (1)
libs/list-types/civil-and-family-daily-cause-list/src/rendering/renderer.ts (1)
renderCauseListData(170-220)
🪛 GitHub Actions: Test
libs/api/src/middleware/oauth-middleware.ts
[error] 69-69: API authentication error: Error: Azure AD configuration not found. Ensure AZURE_TENANT_ID and AZURE_CLIENT_ID are set in Key Vault or environment variables.
[error] 85-85: API authentication error: Error: Invalid token format
[error] 90-90: API authentication error: Error: Token missing kid (key ID) in header
[error] 96-96: API authentication error: Error: Failed to fetch signing key: Failed to fetch key from JWKS endpoint
[error] 102-102: API authentication error: Error: Token verification failed: Token signature verification failed
[error] 112-112: API authentication error: Error: Token verification failed: Token signature verification failed
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
[error] 1-1: yarn run test --coverage failed with exit code 1 in libs/list-types/civil-and-family-daily-cause-list
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (22)
libs/web-core/src/middleware/i18n/locale-middleware.test.ts (4)
129-135: MockResponsecasting pattern looks goodSwitching
restoas unknown as Responsehere is consistent with the rest of the file and avoids structural typing noise for this minimal mock.
146-152: Welsh translations test mock typing is consistentSame
as unknown as Responsepattern here keeps the mock lightweight while satisfying TS; no issues.
163-169: Graceful missing-locale test uses appropriateResponsecastingThe mock
resshape plusas unknown as Responseis appropriate for this test; assertions remain clear and focused on behavior.
178-184: Request/response mocks aligned with file-wide typing approachUsing
as unknown as Requestforreqandas unknown as Responseforresbrings this test in line with the rest of the suite and avoids over-specifying the mock shape..github/workflows/e2e.yml (1)
91-94: Confirm blob-ingestion OAuth env var names match application/Helm configThe new vars look sensible, but the naming mixes
AZURE_*withAPP_PIP_DATA_MANAGEMENT_SCOPE. Please double‑check that these exact names are what the blob‑ingestion OAuth middleware and Helm values expect; otherwise CI tests may run without a scope or with mismatched config.libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts (1)
160-168: Ensure renderCauseListData mock shape matches real renderer/controller contractIn the success-path tests you mock
renderCauseListDatato resolve to{ document, venue, courtLists } as any, while the controller assertions still expectres.renderto receiveheader,openJustice, andlistDatafields.Please verify that:
- The real
renderCauseListDataimplementation returns the same structure you’re mocking here, and- The controller transforms that structure into the
header/openJustice/listDataprops expected by the template.If the renderer still returns
{ header, openJustice, listData }, the current mocks may cause test breakage (and could be related to the failingyarn run test --coveragein this package). Align either the mocks or the implementation/controller to the agreed contract.Also applies to: 223-227, 274-278, 334-338, 385-388
libs/api/package.json (1)
1-39: New @hmcts/blob-ingestion package configuration looks consistentThe module is correctly set up as ESM with separate
./configexport, usesvitest runfor tests, and wires internal dependencies viaworkspace:*. This matches the documented package.json patterns in this repo.libs/api/src/config.ts (1)
1-7: apiRoutes config matches expected patternDeriving
apiRoutes.pathfrom__dirnameusingfileURLToPath(import.meta.url)is consistent with other modules and with the separateconfig.tspattern required to avoid Prisma circular dependencies. This should plug cleanly into@hmcts/blob-ingestion/configvia the new package export.tsconfig.json (1)
26-28: TS path aliases correctly wired for blob-ingestion moduleThe new
@hmcts/blob-ingestionand@hmcts/blob-ingestion/configaliases point tolibs/api/srcand itsconfigsubpath, which matches the new package.json exports and will keep imports consistent across the monorepo.libs/api/src/index.ts (1)
1-4: LGTM!The barrel export structure follows the monorepo conventions, with proper ESM imports using
.jsextensions. The module correctly exposes the blob-ingestion components and OAuth middleware.libs/api/src/blob-ingestion/validation.test.ts (1)
1-226: LGTM!The test suite provides comprehensive coverage of the validation logic:
- Required field validation
- Format validation (ISO dates/datetimes)
- Allowed value constraints (provenance, list_type, sensitivity, language)
- Logical constraints (display_to after display_from)
- Size limits
- Location existence handling
The mocking strategy appropriately isolates external dependencies, and all test scenarios align with the validation requirements.
libs/api/src/blob-ingestion/file-storage.test.ts (1)
1-119: LGTM!The test suite thoroughly validates the file storage functionality:
- Artefact-based filename construction with extension preservation
- Handling of files without extensions and multiple extensions (e.g., .tar.gz)
- Directory creation with recursive option
- Correct path structure validation
- Error propagation for mkdir and writeFile failures
The tests ensure the storage logic is robust and handles edge cases appropriately.
libs/api/src/blob-ingestion/repository/queries.test.ts (1)
1-206: LGTM!The test suite provides thorough coverage of the ingestion log query functions:
createIngestionLog: Validates payload structure for both success logs and error logs with optional fieldsgetIngestionLogsByDateRange: Verifies date range filtering, ordering, and result mappinggetRecentErrorLogs: Tests default and custom limits, status filtering, and orderingThe tests appropriately mock Prisma and verify the correct query parameters are passed, ensuring the query logic is robust.
libs/api/src/blob-ingestion/repository/service.test.ts (1)
1-203: LGTM!The test suite comprehensively validates the blob ingestion service:
- Successful ingestion with existing location (lines 50-75)
- No-match scenario with appropriate flag and message (lines 77-98)
- Validation error handling without artefact creation (lines 100-121)
- System error handling with proper logging (lines 123-143)
- Provenance mapping verification (lines 145-162)
- Date field parsing (lines 164-183)
- isFlatFile flag for JSON blobs (lines 185-202)
The tests ensure all critical paths are covered, including success, validation failures, and system errors, with appropriate assertions on artefact creation, file storage, and audit logging.
libs/api/src/blob-ingestion/validation.ts (3)
9-154: LGTM on overall validation structure.The validation function is well-organized with clear separation of concerns: size validation, required field checks, format validation, cross-field validation, and schema validation. The async pattern and error accumulation approach are appropriate.
1-1: I'll verify the concern aboutmockListTypesbeing used in production. Let me investigate the codebase.
<function_calls>
#!/bin/bashSearch for mockListTypes definition and usage
echo "=== Looking for mockListTypes definition ==="
rg -n "mockListTypes" --type=ts | head -20echo -e "\n=== Checking if there's a non-mock listTypes ==="
rg -n "listTypes" --type=ts -g '!mock' -g '!*.test.ts' | grep -E "(export|const|let)" | head -20echo -e "\n=== Examining the validation.ts file ==="
wc -l libs/api/src/blob-ingestion/validation.ts
</function_calls>
<function_calls>
#!/bin/bashRead the validation.ts file to see how mockListTypes is used
echo "=== Reading validation.ts ==="
cat -n libs/api/src/blob-ingestion/validation.ts | head -100
</function_calls>
7-7: I'll verify the allowed provenances to determine if there's a genuine mismatch between requirements and implementation. Let me start by exploring the codebase.
<function_calls>
#!/bin/bashSearch for Provenance type definitions in the @hmcts/publication module
rg -n "Provenance" --type=ts -C3 | head -80
</function_calls>Let me also search more specifically for the module:
<function_calls>
#!/bin/bashFind the @hmcts/publication module location
fd -t d "publication" | head -20
</function_calls>Let me check the validation.ts file context and look for related constants or configurations:
<function_calls>
#!/bin/bashRead the validation.ts file to understand the context
cat -n libs/api/src/blob-ingestion/validation.ts | head -40
</function_calls>Let me search for provenance references more broadly:
<function_calls>
#!/bin/bashSearch for LIBRA, SJP, CPP references to see if they appear anywhere in the codebase
rg -in "LIBRA|SJP|CPP" --type=ts -C2 | head -60
</function_calls>Let me search for SNL, XHIBIT, MANUAL_UPLOAD references:
<function_calls>
#!/bin/bashSearch for the provenances that are currently in ALLOWED_PROVENANCES
rg -in "SNL|MANUAL_UPLOAD|COMMON_PLATFORM" --type=ts -C2
</function_calls>Let me check for any documentation or PR context in common locations:
<function_calls>
#!/bin/bashLook for README or documentation files related to blob ingestion
find . -type f ( -name "README*" -o -name "*.md" ) | xargs grep -l -i "provenance|blob.*ingestion" 2>/dev/null | head -10
</function_calls>Let me check the broader context around the validation file:
<function_calls>
#!/bin/bashList files in the blob-ingestion directory
ls -la libs/api/src/blob-ingestion/ 2>/dev/null || find . -type d -name "blob-ingestion" | head -5
</function_calls>Let me check git history or PR context:
<function_calls>
#!/bin/bashCheck git log for recent changes to validation.ts
git log --oneline -n 20 -- libs/api/src/blob-ingestion/validation.ts 2>/dev/null || echo "Git history not available"
</function_calls>Let me search for type definitions and imports in the validation file:
<function_calls>
#!/bin/bashGet the full content of validation.ts
wc -l libs/api/src/blob-ingestion/validation.ts 2>/dev/null && echo "---" && cat -n libs/api/src/blob-ingestion/validation.ts
</function_calls>libs/api/src/blob-ingestion/repository/service.ts (2)
8-13: PROVENANCE_MAP aligns with validation allowed values.The map correctly covers the provenances defined in
ALLOWED_PROVENANCESfrom the validation module. The fallback at line 57 handles any edge cases gracefully.
15-35: Validation failure handling is well-structured.The flow correctly logs validation errors with appropriate context and returns a structured error response without exposing internal details.
libs/api/src/blob-ingestion/repository/queries.ts (1)
4-16: LGTM on parameterized query usage.The Prisma queries correctly use parameterized inputs, following the coding guidelines for secure database access. Based on learnings, this approach prevents SQL injection.
libs/api/src/middleware/oauth-middleware.ts (1)
12-52: Middleware structure follows conventions.The middleware is correctly exported as a function and placed in a dedicated middleware file as per coding guidelines. The authentication flow is logically structured with appropriate HTTP status codes (401 for auth failures, 403 for insufficient permissions).
libs/api/src/middleware/oauth-middleware.test.ts (1)
25-460: Comprehensive test coverage for authentication scenarios.The test suite covers all critical paths: header validation, token format, Azure config handling, role verification, JWKS failures, and edge cases. This provides good confidence in the middleware behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
48-48: `` Outdated dependency versions already flagged.Prior review flagged that
jwks-rsaand@types/jsonwebtokenversions are behind the latest available releases:
@types/jsonwebtoken: 9.0.7 → latest 9.0.10jwks-rsa: 3.1.0 → latest 3.2.0Update these to the latest secure releases for currency and any bug fixes.
- "@types/jsonwebtoken": "9.0.7", + "@types/jsonwebtoken": "9.0.10",- "jwks-rsa": "3.1.0", + "jwks-rsa": "3.2.0",Also applies to: 73-74
🧹 Nitpick comments (2)
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts (2)
72-86: Deduplicate repeatedmockArtefactconstruction and avoid scatteredas anycastsThe artefact shape (including new fields like
sensitivity,lastReceivedDate,isFlatFile,provenance,supersededCount,noMatch, andlocationIdas a string) is repeated verbatim across many tests, each ending withas any. That’s a lot of duplication and makes future schema changes easy to miss in one of the blocks.Consider introducing a small typed factory/helper at the top of the file and using overrides per test, e.g.:
+type MockArtefact = { + artefactId: string; + locationId: string; + listTypeId: number; + contentDate: Date; + sensitivity: string; + language: string; + displayFrom: Date; + displayTo: Date; + lastReceivedDate: Date; + isFlatFile: boolean; + provenance: string; + supersededCount: number; + noMatch: boolean; +}; + +const createMockArtefact = (overrides: Partial<MockArtefact> = {}): MockArtefact => ({ + artefactId: "test-id", + locationId: "1", + listTypeId: 8, + contentDate: new Date("2025-01-13"), + sensitivity: "PUBLIC", + language: "ENGLISH", + displayFrom: new Date("2025-01-13"), + displayTo: new Date("2025-01-20"), + lastReceivedDate: new Date("2025-01-13"), + isFlatFile: false, + provenance: "MANUAL_UPLOAD", + supersededCount: 0, + noMatch: false, + ...overrides +});Then each test can use:
-const mockArtefact = { ... } as any; +const mockArtefact = createMockArtefact({ provenance: "UNKNOWN_PROVENANCE" });This removes repeated literals, centralises the new fields, and gets rid of a lot of
as anynoise in one go.Also applies to: 100-113, 132-146, 194-207, 244-258, 304-318, 354-368
116-119: Avoidas anywhen mockingvalidateCivilFamilyCauseListresultsThe validation mocks are currently cast to
any, which works but bypasses the strict typing you get fromvalidateCivilFamilyCauseListitself.You can keep these strongly typed with minimal code, e.g.:
-vi.mocked(validateCivilFamilyCauseList).mockReturnValue({ - isValid: false, - errors: ["Validation error"] -} as any); +vi.mocked(validateCivilFamilyCauseList).mockReturnValue({ + isValid: false, + errors: ["Validation error"] +} as ReturnType<typeof validateCivilFamilyCauseList>);and similarly for the
isValid: truecases. This keeps the tests aligned with the real return type and avoidsanywhile still being concise.Also applies to: 160-163, 221-224, 272-275, 332-335, 382-385
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts(11 hunks)package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Package names must use @hmcts scope (e.g.,@hmcts/auth,@hmcts/case-management).
All package.json files must use"type": "module"to enforce ES modules. Never use CommonJSrequire()ormodule.exports. Useimport/exportonly.
Express version 5.x only must be used ("express": "5.1.0"). Pin all dependencies to specific versions except peer dependencies.
Build scripts must includebuild:nunjucksif the module contains Nunjucks templates in thepages/directory to copy .njk files to dist.
Files:
package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and Interfaces must use PascalCase (e.g.,UserService,CaseRepository). Do NOT useIprefix for interfaces (useUserRepositorynotIUserRepository).
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
TypeScript must use strict mode enabled with noanywithout justification. Use workspace aliases (@hmcts/*) for imports.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Only export functions that are intended to be used outside the module. Don't export functions solely for testing purposes.
Only add comments when they are meaningful. Explain why something is done, not what is done.
Favor functional style with simple functional approaches. Don't use a class unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
API endpoints must use plural for collections (
/api/cases,/api/users), singular for specific resources (/api/case/:id), and singular for creation (POST /api/case).
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/pages/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Page controllers must export GET and/or POST functions with signature
(req: Request, res: Response) => Promise<void>. Content (titles, descriptions) should be organized asenandcyobjects.
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/{pages,locales}/**/*.{ts,njk}
📄 CodeRabbit inference engine (CLAUDE.md)
Every page must support both English and Welsh by providing
enandcyobjects in controllers and maintaining matching structure in locale files (libs/[module]/src/locales/en.ts and cy.ts).
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All packages must use
"test": "vitest run"as the test script. Unit and integration tests must be co-located with source code as*.test.tsfiles.
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Page routes are created based on file names within the
pages/directory. Nested routes are created using subdirectories (e.g.,pages/admin/my-page.tsbecomes/admin/my-page).
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
**/{locales,pages}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Shared/common content (button text, phase banner, service name, common errors) should be in locale files (libs/[module]/src/locales/en.ts and cy.ts). Page-specific content should be in controllers.
Files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts
🧬 Code graph analysis (1)
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts (2)
libs/list-types/civil-and-family-daily-cause-list/src/rendering/renderer.ts (1)
renderCauseListData(170-220)libs/list-types/civil-and-family-daily-cause-list/src/pages/index.ts (1)
GET(19-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test Changed Packages
- GitHub Check: E2E Tests
🔇 Additional comments (2)
package.json (1)
66-67: Clarify resolutions forbody-parserandnode-forge.These entries were added to resolutions but lack context. Confirm:
- Is
body-parser@2.2.1pinned to resolve a transitive dependency conflict with Express 5.x or other packages?- Is
node-forge@1.3.2pinned to ensure consistent crypto behavior for JWT verification in the OAuth middleware?libs/list-types/civil-and-family-daily-cause-list/src/pages/index.test.ts (1)
172-188: Good coverage of render parameters, locale handling, and data source behaviourThe additional expectations around:
renderCauseListDatabeing called withlocationIdas a string and the correctlocale, and- asserting that
res.renderreceivesen,cy,header,openJustice,listData,t, and the expecteddataSourcegive solid coverage of the controller’s happy paths for English, Welsh, and the default-locale case, as well as the provenance labelling logic.
Once the
renderCauseListDatamock shape is aligned (see other comment), these assertions should provide good protection against regressions in the controller without further changes needed here.Also applies to: 233-239, 284-288
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
libs/api/src/blob-ingestion/file-storage.ts (1)
8-8: Update comment to reflect correct file path.The comment states
libs/blob-ingestion/src/blob-ingestion/but this file is located atlibs/api/src/blob-ingestion/. The path calculation itself is correct (4 levels up), but the comment should be updated for clarity.Apply this diff:
-// Navigate to monorepo root (from libs/blob-ingestion/src/blob-ingestion/) +// Navigate to monorepo root (from libs/api/src/blob-ingestion/)
🧹 Nitpick comments (1)
apps/api/src/server.test.ts (1)
16-38: Restore originalprocess.exit,console.log, andconsole.errorafter tests.These globals are overwritten but never restored, which can cause test pollution and interfere with debugging in subsequent tests or parallel runs.
+const originalExit = process.exit; +const originalConsoleLog = console.log; +const originalConsoleError = console.error; + beforeEach(() => { vi.clearAllMocks(); // Mock process methods process.exit = mockExit as any; console.log = mockConsoleLog; console.error = mockConsoleError; // Setup mock server mockServer = { listen: vi.fn(), close: vi.fn(), on: vi.fn() }; // Setup mock createApp mockCreateApp = vi.fn(); }); afterEach(() => { vi.resetModules(); vi.clearAllMocks(); + process.exit = originalExit; + console.log = originalConsoleLog; + console.error = originalConsoleError; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api/src/server.test.ts(1 hunks)libs/api/src/blob-ingestion/file-storage.ts(1 hunks)libs/api/src/blob-ingestion/repository/service.ts(1 hunks)libs/api/src/blob-ingestion/validation.ts(1 hunks)libs/api/src/middleware/oauth-middleware.ts(1 hunks)libs/api/src/routes/v1/publication.test.ts(1 hunks)libs/api/src/routes/v1/publication.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/api/src/routes/v1/publication.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and Interfaces must use PascalCase (e.g.,UserService,CaseRepository). Do NOT useIprefix for interfaces (useUserRepositorynotIUserRepository).
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
TypeScript must use strict mode enabled with noanywithout justification. Use workspace aliases (@hmcts/*) for imports.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Only export functions that are intended to be used outside the module. Don't export functions solely for testing purposes.
Only add comments when they are meaningful. Explain why something is done, not what is done.
Favor functional style with simple functional approaches. Don't use a class unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Files:
libs/api/src/blob-ingestion/validation.tslibs/api/src/routes/v1/publication.test.tslibs/api/src/blob-ingestion/repository/service.tsapps/api/src/server.test.tslibs/api/src/middleware/oauth-middleware.tslibs/api/src/blob-ingestion/file-storage.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
API endpoints must use plural for collections (
/api/cases,/api/users), singular for specific resources (/api/case/:id), and singular for creation (POST /api/case).
Files:
libs/api/src/blob-ingestion/validation.tslibs/api/src/routes/v1/publication.test.tslibs/api/src/blob-ingestion/repository/service.tsapps/api/src/server.test.tslibs/api/src/middleware/oauth-middleware.tslibs/api/src/blob-ingestion/file-storage.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All packages must use
"test": "vitest run"as the test script. Unit and integration tests must be co-located with source code as*.test.tsfiles.
Files:
libs/api/src/routes/v1/publication.test.tsapps/api/src/server.test.ts
**/*-middleware.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable middleware must be placed in a dedicated
libs/[module]/src/[middleware-name]-middleware.tsfile and exported as a function.
Files:
libs/api/src/middleware/oauth-middleware.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: VIBE-209-specification.md:67-76
Timestamp: 2025-11-27T09:50:32.692Z
Learning: In the CaTH blob ingestion API (VIBE-209), when a court_id/location_id is not found in the Court Master Reference Data, the API returns 200 OK with no_match=true rather than a 404 error. This allows ingestion to proceed and enables later admin mapping of the location.
📚 Learning: 2025-11-27T09:48:12.999Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: libs/api/src/blob-ingestion/validation.ts:156-163
Timestamp: 2025-11-27T09:48:12.999Z
Learning: In libs/api/src/blob-ingestion/validation.ts, the permissive date validation in isValidISODate and isValidISODateTime functions is expected behavior and should not be flagged for stricter validation.
Applied to files:
libs/api/src/blob-ingestion/validation.tslibs/api/src/routes/v1/publication.test.tslibs/api/src/blob-ingestion/repository/service.ts
📚 Learning: 2025-11-27T09:50:32.692Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: VIBE-209-specification.md:67-76
Timestamp: 2025-11-27T09:50:32.692Z
Learning: In the CaTH blob ingestion API (VIBE-209), when a court_id/location_id is not found in the Court Master Reference Data, the API returns 200 OK with no_match=true rather than a 404 error. This allows ingestion to proceed and enables later admin mapping of the location.
Applied to files:
libs/api/src/routes/v1/publication.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*.{ts,tsx} : Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Applied to files:
apps/api/src/server.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/*-middleware.ts : Reusable middleware must be placed in a dedicated `libs/[module]/src/[middleware-name]-middleware.ts` file and exported as a function.
Applied to files:
libs/api/src/middleware/oauth-middleware.ts
🧬 Code graph analysis (3)
libs/api/src/blob-ingestion/validation.ts (2)
libs/api/src/blob-ingestion/repository/model.ts (3)
BlobIngestionRequest(8-18)BlobValidationResult(33-38)ValidationError(28-31)libs/publication/src/index.ts (2)
Sensitivity(7-7)Language(2-2)
libs/api/src/routes/v1/publication.test.ts (3)
libs/api/src/blob-ingestion/repository/service.ts (1)
processBlobIngestion(15-115)libs/api/src/routes/v1/publication.ts (1)
POST(26-72)libs/api/src/blob-ingestion/repository/model.ts (1)
BlobIngestionRequest(8-18)
libs/api/src/middleware/oauth-middleware.ts (2)
libs/api/src/index.ts (1)
authenticateApi(4-4)e2e-tests/run-with-credentials.js (1)
client(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
🔇 Additional comments (11)
apps/api/src/server.test.ts (2)
1-14: LGTM!Clean imports with type-only import for Server and proper vitest function imports. Module-level mock declarations work well with the beforeEach reassignment pattern.
165-273: LGTM!Signal handler tests appropriately capture and invoke handlers while preserving original process.on behavior. Good coverage of graceful shutdown flow.
libs/api/src/routes/v1/publication.test.ts (1)
1-180: Excellent test coverage for the publication endpoint.The test suite comprehensively covers all scenarios including successful ingestion (201), validation failures (400), system errors (500), and the no_match scenario (200). The no_match test correctly expects HTTP 200 per the specification, treating it as a successful business outcome rather than an error.
Based on learnings, the no_match scenario returns 200 OK with
no_match=trueto allow ingestion to proceed for later admin mapping.libs/api/src/middleware/oauth-middleware.ts (2)
12-52: LGTM! OAuth middleware properly secured.The middleware correctly validates Bearer tokens, checks required roles, and attaches minimal user info to the request. The error logging at line 45 has been fixed to avoid logging sensitive authentication data, addressing the previous security concern.
As per coding guidelines, sensitive data should never be included in logs.
54-120: Solid JWT validation implementation.The token validation properly fetches Azure AD signing keys, verifies signatures using RS256, validates issuer and audience claims, and includes appropriate caching (24 hours) for performance. The configuration fallback from Key Vault to environment variables provides good flexibility.
libs/api/src/blob-ingestion/file-storage.ts (1)
12-23: File storage implementation is secure and well-structured.The function safely constructs filenames using the artefactId as the base and only the extension from the original filename, preventing path traversal attacks. The use of
path.joinand recursive directory creation ensures reliable storage.libs/api/src/blob-ingestion/validation.ts (2)
9-158: Comprehensive multi-layer validation implementation.The validation function properly implements the multi-layer strategy: size checks, required fields, format validation, business rules, location lookup, and JSON schema validation. The location validation correctly sets the
locationExistsflag without failing, enabling the no_match workflow where ingestion proceeds even when a location is not found.Based on learnings, the no_match scenario allows ingestion to proceed for later admin mapping.
160-168: Date validation helpers work as intended.The permissive date validation in these functions is expected behavior per the retrieved learnings. The functions rely on JavaScript's Date constructor, which provides the level of validation required for this use case.
Based on learnings, the permissive date validation is intentional.
libs/api/src/blob-ingestion/repository/service.ts (3)
40-61: Excellent fix for missing listTypeId handling.The previous throw has been replaced with proper defensive error handling: logging detailed context, recording the SYSTEM_ERROR in the ingestion log, and returning a consistent failure response. This ensures the error is gracefully handled without propagating unhandled exceptions to the caller.
63-98: Clean and well-structured success path.The success flow properly creates the artefact with the computed
noMatchflag, persists the JSON to temporary storage, logs the ingestion, and returns a response with contextual messaging. The provenance mapping and date parsing are handled correctly, and theno_matchflag is prominently returned to inform the caller.
99-114: Robust error handling with proper logging.The catch block correctly logs system errors to the ingestion log with context while returning a generic error message to the caller. This prevents exposure of internal details while maintaining an audit trail for debugging.
- Removed duplicate Subscription model from postgres app schema - Subscription model now only defined in libs/subscriptions - Maintained User.subscriptions relation for proper foreign key references This resolves the Prisma validation error: "The model 'Subscription' cannot be defined because a model with that name already exists" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Removed duplicate subscription table creation from blob_ingestion migration - Subscription table is already created by add_email_subscriptions migration - Kept only ingestion_log table and artefact column addition - Deleted redundant apps/api/helm/values.dev.yaml - File contained identical values to values.yaml - No local development overrides needed This fixes the GitHub pipeline error: "ERROR: relation 'subscription' already exists" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/postgres/prisma/migrations/20251126100649_blob_ingestion/migration.sql(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: VIBE-209-specification.md:67-76
Timestamp: 2025-11-27T09:50:32.707Z
Learning: In the CaTH blob ingestion API (VIBE-209), when a court_id/location_id is not found in the Court Master Reference Data, the API returns 200 OK with no_match=true rather than a 404 error. This allows ingestion to proceed and enables later admin mapping of the location.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: E2E Tests
- GitHub Check: Test Changed Packages
🔇 Additional comments (1)
apps/postgres/prisma/migrations/20251126100649_blob_ingestion/migration.sql (1)
18-24: Index strategy is appropriate and aligns with actual query patterns.Verification confirms the three single-column indexes are well-suited for the ingestion service's query patterns:
timestampindex: Used ingetIngestionLogsByDateRange()for range filtering (WHERE timestamp BETWEEN) and sorting (ORDER BY timestamp DESC)statusindex: Used ingetRecentErrorLogs()for filtering errors (WHERE status IN [...])sourceSystemindex: Defined in schema but not used in any WHERE clausesBoth
getIngestionLogsByDateRange()andgetRecentErrorLogs()query only a single column in their WHERE conditions and sort by timestamp. Composite indexes (e.g.,status + timestamp) are not required since the queries never combine multiple columns in filtering. The current single-column indexes are sufficient and efficient.
- Removed duplicate subscription table creation from blob_ingestion migration - Subscription table is already created by add_email_subscriptions migration - Kept only ingestion_log table and artefact column addition - Restored apps/api/helm/values.dev.yaml for local development - System automatically loads values.dev.yaml in non-Azure environments - Excludes production 'cath' vault, only connects to pip-ss-kv-stg - Required for Azure AD OAuth credentials from staging Key Vault This fixes: - GitHub pipeline error: "ERROR: relation 'subscription' already exists" - Local API authentication error when connecting to Key Vault 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…hmcts/cath-service into feature/VIBE-209-blob-ingestion
|
3 participants
Jira link
https://tools.hmcts.net/jira/browse/VIBE-209
Change description
Blob Ingestion using API
Summary by CodeRabbit
New Features
Validation & Logging
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.