feat: add includedURLs support to patchAuditForSite endpoint#2355
Open
noruiz wants to merge 13 commits into
Open
feat: add includedURLs support to patchAuditForSite endpoint#2355noruiz wants to merge 13 commits into
noruiz wants to merge 13 commits into
Conversation
Extend the PATCH /sites/:siteId/:auditType endpoint to accept and persist includedURLs per audit handler, with merge/replace semantics controlled by replaceIncludedURLs and replaceExcludedURLs flags. The Config model (external package) has no updateIncludedURLs setter, so includedURLs is applied by patching the configObj returned by Config.toDynamoItem() before calling site.setConfig(). This correctly handles the edge case where getHandlers() returns undefined. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
noruiz
changed the title
…fix test organisation - Shallow-copy configObj.handlers and configObj.handlers[auditType] before assigning includedURLs to avoid mutating live Config state through the shared reference returned by toDynamoItem - Update unit tests to assert via site.setConfig args instead of the now- isolated handler object (tests were previously passing only due to the shared-reference mutation that the fix removes) - Add integration tests for PATCH /sites/:siteId/:auditType includedURLs merge, replace, clear, persist round-trip, validation, and access control - Move replaceExcludedURLs unit tests into their own describe block - Add replaceIncludedURLs/replaceExcludedURLs to UpdateHandlerTypeConfig OpenAPI example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
context.data can be null/undefined with malformed requests before body
parsing completes; destructuring it directly throws a 500 instead of
returning a 400. Adding ?? {} makes the destructure safe.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…URLs write path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…:auditType 500s Configuration is read from S3 (not PostgREST), so the dummy-config-bucket was missing in MinIO, causing Configuration.findLatest() to throw on every PATCH request and surface as a 500 in IT tests. Seed a minimal config with all known handlers on harness startup. Also adds the context.data null branch test for patchAuditForSite (100% branch coverage on audits.js). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Configuration.findLatest() returns null when no config exists in S3 (NoSuchKey case). Calling .getHandlers() on null threw a TypeError, surfacing as a 500 in environments where the config bucket exists but has no object (IT test VPC environment). Skip the audit-type validation when no configuration is available — in production the config is always present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tes/:id/:auditType 500s" This reverts commit c0d92c3.
…orSite findLatest throws (not returns null) when S3 throws NoSuchBucket or AccessDenied. Wrap in try-catch so unavailable configuration skips the audit-type check instead of surfacing a 500.
noruiz
force-pushed
the
fix/LLMO-4070-audit-included-urls
branch
from
d60096c to
d156595
Compare
Bare catch {} swallowed all errors silently. Log the error at warn level
so S3 unavailability is visible in logs while still allowing the request
to proceed without the audit-type registration check.
baranovskyalexandr
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes LLMO-4070 (backend portion)
What changed
PATCH /sites/:siteId/:auditTypeto acceptincludedURLs(array of URLs) with merge/replace semantics viareplaceIncludedURLsboolean flagreplaceExcludedURLsflag to give callers atomic control overexcludedURLsincludedURLsand both replace flags to theUpdateHandlerTypeConfigandHandlerConfigOpenAPI schemasWhy
The
config.handlers[auditType].includedURLsfield existed in the data model but had no API surface. TheConfigmodel (external package) has noupdateIncludedURLssetter, soincludedURLsis applied by patching the plain object returned byConfig.toDynamoItem()directly beforesite.setConfig().Assumptions made
Config.toDynamoItem()returnshandlersas the same object reference asstate.handlers, making the configObj patch equivalent to a state mutation in the normal casestate.handlersis always initialized to{}by schema default — the undefined-handlers path is an edge caseTesting
Unit tests cover: non-array input, invalid URL validation, empty-array clear, merge with no existing, merge with existing, deduplication, atomic replace,
replaceExcludedURLs: falsemerge regression, combined includedURLs+excludedURLs request, undefined-handlers persistence, response body correctnessCode review
This solution went through 3 automated subagent review iterations.