feat(#4043): add ingestion health backend API, data model, and error classification - #4250
feat(#4043): add ingestion health backend API, data model, and error classification#4250fullsend-ai-coder[bot] wants to merge 3 commits into
Conversation
…classification Implement the ingestion health backend (issue 5 of 29, RHIDP-15335 / RHIDP-15337) per the implementation gate scope: Data model & storage: - boost_sync_attempts table with Knex migration, indexed on (connector_id, timestamp DESC) - SyncAttemptsRepository with insertSyncAttempt, getLatestAttempts, cleanupOldAttempts, and batch getLatestAttemptsForAll - Retention policy via boost.ingestion.healthRetention config (default 100 per connector) with daily scheduled cleanup Health status API: - GET /api/boost/ingestion-health returns per-connector health - ConnectorHealthStatus type in boost-common with connectorId, connectorType, enabled, status, timestamps, errorSummary, metrics - HealthStatusService derives status from last 3 sync attempts: healthy (all success), degraded (mixed), failing (all failure), unknown (zero attempts) - ?includeDisabled=true query parameter support - ConnectorConfigReader seam for config-based connector discovery with dual enabled flags (startup vs runtime), ready for #4044 RuntimeConfigResolver handoff - Structured logging via LoggerService Error classification: - ErrorClassifier utility with classify(error, options?) method - Auth patterns (401/403, invalid token, OAuth expiry) - Network patterns (ECONNREFUSED, ETIMEDOUT, DNS, TLS) - Schema patterns (JSON parse, unexpected field, GraphQL) - Rate limit patterns (429, X-RateLimit, secondary limits) - Connector-specific matchers (GitHub, Jira, GitLab) - Unknown error fallback with diagnostic guidance Explicitly out of scope per gate: RBAC gating (deferred to #4064), admin UI, force-sync, Neo4j panel, connector HealthTracker hooks, full audit event emitters. Closes #4043
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4250 +/- ##
==========================================
+ Coverage 58.77% 58.81% +0.03%
==========================================
Files 2450 2455 +5
Lines 97862 98170 +308
Branches 27310 27416 +106
==========================================
+ Hits 57521 57735 +214
- Misses 40061 40155 +94
Partials 280 280
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
/fs-fix add changeset and fix the CI (api-reports) |
|
🤖 Finished Fix · ✅ Success · Started 12:27 PM UTC · Completed 12:40 PM UTC Commit: |
Add changeset for boost-backend and boost-common minor version bumps. Regenerate API report files to include new ingestion health exports (ErrorClassifier, SyncAttemptsRepository, ConnectorConfigReader, HealthStatusService, and related types). Addresses review feedback on #4250
🔧 Fix agent — iteration 1 (human-triggered)Added changeset for minor version bumps and regenerated API reports for boost-backend and boost-common to include all new ingestion health public exports. Both CI gates (changeset and api-reports) should now pass. Fixed (2):
Tests: passed Updated by fullsend fix agent |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 1:30 PM UTC · Completed 1:48 PM UTC Commit: |
ReviewFindingsHigh
Medium
Low
Labels: PR modifies files exclusively under workspaces/boost/ and adds a new feature (ingestion health backend API) Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| ): Router { | ||
| const { healthService, httpAuth, logger } = options; | ||
| const router = Router(); | ||
|
|
There was a problem hiding this comment.
[high] rbac-violation
The GET /ingestion-health endpoint has no permission check. It calls httpAuth.credentials() only for logging, never calls permissions.authorize(). Every other admin-facing route enforces permissions. IngestionHealthRoutesOptions does not accept PermissionsService.
Suggested fix: Add PermissionsService to IngestionHealthRoutesOptions and enforce permissions.authorize([{ permission: boostAdminPermission }], { credentials }) before returning health data.
| | { userEntityRef?: string } | ||
| | undefined; | ||
| const userRef = principal?.userEntityRef ?? 'unknown'; | ||
|
|
There was a problem hiding this comment.
[medium] data-exposure
The endpoint returns raw error messages from sync failures via errorSummary.errorMessage. These can contain internal hostnames, IPs, DNS names, and API endpoint URLs.
Suggested fix: Return only errorType and diagnosticGuidance to non-admin callers; omit or redact raw errorMessage.
| test: msg => | ||
| /cannot\s+query\s+field/i.test(msg) || | ||
| /field\s+'[^']+'\s+doesn'?t\s+exist\s+on\s+type/i.test(msg) || | ||
| /graphql/i.test(msg), |
There was a problem hiding this comment.
[medium] logic-error
The first NETWORK_PATTERNS entry includes a bare /certificate/i regex that matches any error containing 'certificate', misclassifying mTLS auth failures as network errors.
Suggested fix: Remove the bare /certificate/i catch-all or narrow it to TLS/SSL verification contexts.
| error_message: attempt.errorMessage ?? null, | ||
| assets_added: attempt.assetsAdded ?? 0, | ||
| assets_updated: attempt.assetsUpdated ?? 0, | ||
| assets_removed: attempt.assetsRemoved ?? 0, |
There was a problem hiding this comment.
[medium] edge-case
getLatestAttemptsForAll fetches ALL rows matching connector IDs with no LIMIT clause, returning up to 100N rows when only 3N are needed.
Suggested fix: Add .limit(connectorIds.length * limit) to the query.
| connectorId: row.connector_id, | ||
| timestamp: row.timestamp, | ||
| outcome: row.outcome, | ||
| errorType: row.error_type, |
There was a problem hiding this comment.
[low] nil-deref
In insertSyncAttempt, after insert+re-read, code uses non-null assertion (created!) without a guard.
Suggested fix: Add explicit null check: if (!created) throw new Error('Failed to read back inserted sync attempt').
| try { | ||
| // Extract credentials for structured logging (task 2.6) | ||
| const credentials = await httpAuth.credentials(req); | ||
| const principal = credentials.principal as |
There was a problem hiding this comment.
[low] log-injection
userRef interpolated via template literal; use structured logging instead.
Suggested fix: Use structured logging: logger.info('Ingestion health request', { userRef, includeDisabled }).
fullsend-ai-review
Bot
added
workspace/boost
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 1:51 PM UTC · Completed 2:12 PM UTC Commit: |
- Add PermissionsService and boostAdminPermission enforcement to ingestion health routes, matching the pattern used by kagenti, skills, and other admin routes - Narrow ErrorClassifier certificate regex to TLS/SSL contexts, preventing mTLS auth errors from being misclassified as network - Add LIMIT clause to getLatestAttemptsForAll query to prevent unbounded row fetches - Replace non-null assertion with explicit null check in insertSyncAttempt - Rename SyncAttemptsRepository → SyncAttemptsStore to match codebase naming convention (ConversationStore, McpServerStore, etc.) - Rename ensureTables → ensureTable (single table) - Switch route handler logging from info to debug with structured fields (matches read-handler convention) - Add tests for cleanupOldAttempts and getLatestAttemptsForAll - Add test for certificate regex narrowing - Add permission denial test for routes - Document new config keys in example config Addresses review feedback on #4250
🔧 Fix agent — iteration 2 (human-triggered)Addressed 10 of 11 review findings. Fixed: RBAC enforcement (high), certificate regex logic error, unbounded query, null-safety, missing tests, naming convention, log level, structured logging, method naming, config documentation. Disagreed with 1 (data-exposure): raw error messages are appropriate for admin-only access now that RBAC is enforced. Fixed (10):
Disagreed (1):
Tests: passed Updated by fullsend fix agent |
|
1 participant
Implement the ingestion health backend (issue 5 of 29, RHIDP-15335 / RHIDP-15337) per the implementation gate scope:
Data model & storage:
(connector_id, timestamp DESC)
cleanupOldAttempts, and batch getLatestAttemptsForAll
(default 100 per connector) with daily scheduled cleanup
Health status API:
connectorType, enabled, status, timestamps, errorSummary, metrics
healthy (all success), degraded (mixed), failing (all failure),
unknown (zero attempts)
with dual enabled flags (startup vs runtime), ready for Connector Config — Zod Schemas and RuntimeConfigResolver Extension (issue 6 of 29) #4044
RuntimeConfigResolver handoff
Error classification:
Explicitly out of scope per gate: RBAC gating (deferred to #4064), admin UI, force-sync, Neo4j panel, connector HealthTracker hooks, full audit event emitters.
Closes #4043
Post-script verification
agent/4043-ingestion-health-backend)22fb4fa4a5d7785581599916da8b2dce43737be3..HEAD)