feat(repo-browser): show artifact cache cached_at + expires_at in details dialog (closes #449)#451
Closed
knowinglyAnonymous wants to merge 2 commits into
Closed
Conversation
…ails dialog (artifact-keeper#449) Adds two relative-time rows to the artifact details dialog ('Cached' and 'Cache expires') for Remote (proxy) repositories, so operators reading the panel can decide between waiting for the natural refresh and clicking the 'Invalidate cache' button (artifact-keeper#446 / artifact-keeper#447) without leaving the UI. Companion backend work: artifact-keeper#1541 / #1542 exposes the cache_cached_at and cache_expires_at fields on GET /:key/artifacts/:path. This change is forward-compatible -- until the backend PR lands the rows simply don't appear, no errors. Behaviour: - Both rows are gated on repository.repo_type === 'remote' AND the value being present. Local / Virtual / Staging never show them; Remote repos without a cache-metadata blob (e.g. an artifact direct-uploaded but never proxy-fetched) also hide the rows. - Visible value is the relative-time string ('in 4 hours' / '12 minutes ago' / 'expired 3 days ago, will re-fetch on next download') for quick scanning; the hover-tooltip carries the full ISO-8601 timestamp. New helpers in src/lib/cache-time.ts (pure, testable): - formatRelativeTimestamp(iso, now?) -> 'in 4 hours' style. Uses Intl.RelativeTimeFormat for locale-aware output without a date-fns dep. Picks the largest unit that gives a magnitude >= 1 so output stays compact (we surface 'in 2 days' not 'in 48 hours'). - formatCacheExpiry(iso, now?) -> biased for the expires framing; past timestamps wrap as 'expired ..., will re-fetch on next download'. DetailRow gains an optional title prop so the visible value (relative time) can differ from the hover-tooltip (absolute timestamp). Artifact type gains optional cache_cached_at / cache_expires_at. The SDK doesn't carry these fields yet (regenerates from OpenAPI after the backend PR lands); until then adaptArtifact plumbs them via a narrowed runtime cast. Once the SDK regenerates, the cast can collapse. Tests: - src/lib/__tests__/cache-time.test.ts: 9 cases covering future / past direction, largest-unit selection, unparseable-timestamp fallback, and the 'expired ..., will re-fetch' framing including the boundary t == now case. - src/lib/api/__tests__/artifacts.test.ts: 3 new cases pinning adaptArtifact plumbing -- fields populated when present, undefined when omitted, undefined when explicitly null (defensive). Closes artifact-keeper#449
brandonrc
reviewed
Jun 2, 2026
brandonrc
left a comment
brandonrc
left a comment
Contributor
There was a problem hiding this comment.
Accessibility review (WCAG 2.2 AA) — repo-detail-content.tsx Cached / Cache expires DetailRows.
This is display-only and broadly fine: the relative-time strings ("in 4 hours", "expired 12 minutes ago, will re-fetch on next download") are real visible text inside labelled DetailRows, so screen readers read both the label and value. No icon-only controls, no new interactive elements. Two notes:
-
The full ISO timestamp lives only in the
titleattribute (1.1.1 / 1.3.1).titleis not reliably exposed to screen-reader users and is invisible to keyboard-only users (it only shows on mouse hover). The relative text is the primary, accessible value, so this is acceptable, but if the exact timestamp is considered important info, expose it as visible text or viaaria-label/visually-hidden text rather than relying ontitlealone. Recommend keeping the relative string as the visible value (good) and treatingtitleas purely supplementary. -
Minor — relative timestamps are computed once at render and go stale (a row reading "in 1 minute" stays that way). Not a WCAG issue, but for the "Cache expires" row consider that a stale "in 1 minute" can mislead; out of scope for a11y sign-off.
No blocking accessibility issues. Approve from an a11y standpoint (leaving formal approval to code owners).
Closed
9 tasks
brandonrc
reviewed
Jun 2, 2026
brandonrc
left a comment
brandonrc
left a comment
Contributor
There was a problem hiding this comment.
Frontend correctness review. Clean and well-tested. The adaptArtifact runtime-cast approach for the not-yet-in-SDK fields is consistent with the documented apiFetch/SDK-lag pattern, the ?? undefined normalization (covering both omitted and explicit-null) is correct, and cache-time.ts is pure, dependency-free, and deterministically testable via the injected now. The repo_type === "remote" + truthy-field gates on the detail rows are right. Good unit coverage on the relative-time edge cases.
No blockers. Minor items:
💭 Missing trailing newline (repo-detail-content.tsx)
The diff ends with \ No newline at end of file on the closing brace of DetailRow. If the repo's eslint/prettier enforces eol-last this will fail lint. Add the newline.
💭 formatRelativeTimestamp unit-selection loop is correct but the final return iso is dead
The loop always returns on the unit === "second" iteration, so the trailing return iso; after the loop can never execute. Harmless, but a reader may wonder when it fires. Consider a comment or removing it.
💭 formatCacheExpiry boundary semantics
Treating t <= now as "expired" (including exact equality) is the right product call and you pinned it in a test. No action needed; noting it because it's the kind of thing a future refactor could flip.
Note (not a defect): touches repo-detail-content.tsx (shared with #447 and #458/#460/#462/#464) and src/lib/api/artifacts.ts + artifacts.test.ts (shared with #447 and #454). #447 adds a test block to artifacts.test.ts at a nearby location and #454 edits the same file; expect mechanical conflicts in the test file. Flag merge order. The adaptArtifact change here and the createDownloadTicket change in #454 are in the same module but different functions, so source-level conflicts are unlikely.
5 tasks
brandonrc
reviewed
Jun 2, 2026
brandonrc
left a comment
brandonrc
left a comment
Contributor
There was a problem hiding this comment.
Security/API review (comment-only).
Reviewed for sensitive-data exposure in the artifact details dialog. The two new rows render cache_cached_at and cache_expires_at only, formatted through formatRelativeTimestamp / formatCacheExpiry (timestamps, gracefully falling back to the raw input string for unparseable values). No internal storage keys, filesystem/S3 paths, upstream credentials, or backend object identifiers are surfaced. The rows are gated on repo_type === "remote" and on the field being present.
The adaptArtifact plumbing surfaces only these two timestamp fields and normalises both missing and explicit-null to undefined (tests pin all three cases), so non-remote repos and uncached entries hide the rows rather than rendering empty/"null" text. No new fields beyond the two timestamps ride through the cast.
Rendering is via React JSX (auto-escaped); no dangerouslySetInnerHTML. The new title prop on DetailRow is a string attribute and is also escaped. No XSS surface.
Note: missing trailing newline at EOF on repo-detail-content.tsx (cosmetic, lint may flag).
No blocking issues.
9 tasks
brandonrc
added a commit
that referenced
this pull request
Jun 2, 2026
…firm, dead code Addresses code-review findings on the cache-UI PRs that landed on release/web-1.2.0 after the integration branch was built. repo-settings-tab.tsx (#450 proxy cache TTL): - Associate the TTL validation error with the input via aria-describedby pointing at a persistent role="alert" element (was a conditional, unassociated paragraph), mirroring the age-policy field pattern. - Disable the Discard button while a Save is in flight so it cannot race an in-flight mutation. repo-detail-content.tsx (#447 invalidate cache): - Wrap the destructive "Invalidate cache" action in an AlertDialog confirmation instead of firing immediately on click. - After a successful invalidate, close the details dialog so it does not show stale cache_cached_at / cache_expires_at fields. - Announce delete and invalidate outcomes through a dedicated polite live region in addition to the toast. - Add the missing trailing newline at EOF (eol-last). cache-time.ts (#451 cache rows): - Remove the unreachable "return iso" after the exhaustive unit loop by dropping the always-matching "second" ladder entry and making the sub-minute case an explicit fallback. Validation: eslint clean on changed files, tsc reports only the two accepted pre-existing test errors, affected vitest suites pass (55 tests), playwright --list parses all 528 specs.
Addresses review feedback on artifact-keeper#449: - Add the missing trailing newline at EOF on repo-detail-content.tsx so the repo's eol-last lint rule is satisfied. - Remove the unreachable post-loop `return iso` in formatRelativeTimestamp. The unit table dropped the explicit ["second", 1] row and the loop now iterates minute-and-up; the sub-minute / exact-now case is the terminal `return fmt.format(deltaSec, "second")` after the loop, so the function is total without a dead branch. Behavior is unchanged (Math.round(deltaSec/1) === deltaSec). The adaptArtifact runtime cast is intentionally retained: the installed @artifact-keeper/sdk has not yet regenerated the ArtifactResponse type with cache_cached_at / cache_expires_at, so collapsing it now would not typecheck. Co-authored-by: Cursor <cursoragent@cursor.com>
brandonrc
added a commit
that referenced
this pull request
Jun 3, 2026
* feat(repo-browser): add 'Invalidate cache' action to artifact details dialog (#446) Adds an Invalidate cache button in the artifact-details dialog that calls the new backend endpoint POST /api/v1/repositories/{key}/cache/invalidate ?path=... (companion artifact-keeper#1540 / artifact-keeper#1539). Behaviour: - Button is gated on repository.repo_type === 'remote', so the action is hidden for Local / Virtual / Staging repos that have no proxy cache. The backend also rejects with 400 there, defence in depth. - React-Query mutation invalidates ['artifacts', repoKey] and ['repository', repoKey] on success so subsequent fetches go back to upstream and the cached metadata refreshes. - Toast on success says next download will re-fetch from upstream so operators understand what happened. Errors surface via the existing mutationErrorToast helper, including the backend's user-message. - Pending state disables the button and shows 'Invalidating...' text to prevent double-fires. The API wrapper goes through apiFetch (src/lib/api/fetch.ts) because the generated SDK has not been regenerated against the new endpoint yet; once it has, the wrapper can collapse to the typed SDK call in a follow-up. Tests: - artifactsApi.invalidateCache: pins the URL shape (POST, /cache/invalidate, ?path=, credentials: include). - URL-encoding of both repo key and path so '+', '/', and spaces survive round-trip back to the backend. - Surfaces non-ok responses (e.g. 400 for non-remote repo, 503 for missing storage) as thrown errors. Closes #446 * feat(settings): show + edit proxy cache TTL on Remote repo Settings tab (#448) Adds a Proxy Cache section to the repo Settings tab that displays and edits the proxy cache TTL on Remote (proxy) repositories. Hidden for Local / Virtual / Staging repos -- those have no proxy cache and the backend rejects PUT /:key/cache-ttl on them anyway. The TTL endpoints (GET / PUT /api/v1/repositories/{key}/cache-ttl) already exist and the SDK already exports getCacheTtl / setCacheTtl, so this is a pure UI change. Behaviour: - Section is gated on repository.repo_type === 'remote'. - Initial value comes from GET /:key/cache-ttl, which falls back to DEFAULT_CACHE_TTL_SECS (86400 = 24h) when nothing is stored -- matching the contract documented by the docs PR #71. - Inline validation mirrors the backend's validate_cache_ttl range (1..=2_592_000). Out-of-range values get an aria-invalid input, an inline error, and disable the Save button. - Helper line shows the human-readable equivalent ('approximately 24 hours', '1 day 6 hours') so operators don't have to mentally convert seconds. - Plugs into the existing hasChanges / Save Changes / Discard workflow. Save dispatches the general-fields update and the new setCacheTtl in parallel via Promise.allSettled; failures on one side don't roll back the other, and each mutation has its own onError toast so the operator can tell which side failed. Adds repositoriesApi.getCacheTtl / setCacheTtl wrappers using the typed SDK calls. No hand-written apiFetch needed. Tests: - getCacheTtl happy path + error surfacing. - setCacheTtl happy path with body-shape pin (cache_ttl_seconds, NOT the legacy 'value' form that the docs PR #71 corrected). - setCacheTtl error surfacing including the backend's 'remote (proxy) repositories' rejection. Closes #448 * feat(repo-browser): show artifact cache cached_at + expires_at in details dialog (#449) Adds two relative-time rows to the artifact details dialog ('Cached' and 'Cache expires') for Remote (proxy) repositories, so operators reading the panel can decide between waiting for the natural refresh and clicking the 'Invalidate cache' button (#446 / #447) without leaving the UI. Companion backend work: artifact-keeper#1541 / #1542 exposes the cache_cached_at and cache_expires_at fields on GET /:key/artifacts/:path. This change is forward-compatible -- until the backend PR lands the rows simply don't appear, no errors. Behaviour: - Both rows are gated on repository.repo_type === 'remote' AND the value being present. Local / Virtual / Staging never show them; Remote repos without a cache-metadata blob (e.g. an artifact direct-uploaded but never proxy-fetched) also hide the rows. - Visible value is the relative-time string ('in 4 hours' / '12 minutes ago' / 'expired 3 days ago, will re-fetch on next download') for quick scanning; the hover-tooltip carries the full ISO-8601 timestamp. New helpers in src/lib/cache-time.ts (pure, testable): - formatRelativeTimestamp(iso, now?) -> 'in 4 hours' style. Uses Intl.RelativeTimeFormat for locale-aware output without a date-fns dep. Picks the largest unit that gives a magnitude >= 1 so output stays compact (we surface 'in 2 days' not 'in 48 hours'). - formatCacheExpiry(iso, now?) -> biased for the expires framing; past timestamps wrap as 'expired ..., will re-fetch on next download'. DetailRow gains an optional title prop so the visible value (relative time) can differ from the hover-tooltip (absolute timestamp). Artifact type gains optional cache_cached_at / cache_expires_at. The SDK doesn't carry these fields yet (regenerates from OpenAPI after the backend PR lands); until then adaptArtifact plumbs them via a narrowed runtime cast. Once the SDK regenerates, the cast can collapse. Tests: - src/lib/__tests__/cache-time.test.ts: 9 cases covering future / past direction, largest-unit selection, unparseable-timestamp fallback, and the 'expired ..., will re-fetch' framing including the boundary t == now case. - src/lib/api/__tests__/artifacts.test.ts: 3 new cases pinning adaptArtifact plumbing -- fields populated when present, undefined when omitted, undefined when explicitly null (defensive). Closes #449 * test(settings): add component tests for new Proxy Cache section Closes the new-code coverage gap that the CI gate flagged (56% on the initial commit; threshold is 80%). The previous commit added the UI section + API wrappers, but only the wrappers had unit-level coverage. This commit adds component-level tests against the React tree, pinning the contract the issue body promises: - Section is hidden for Local / Virtual / Staging repos; the GET is not even issued (gated on 'enabled: isRemote' in the useQuery). - Section is visible for Remote repos and renders the TTL fetched from getCacheTtl. - Editing the input plugs into the existing unsaved-changes bar. - Save invokes setCacheTtl with the parsed integer and shows the success toast; the general-fields update mutation does NOT fire when only the TTL changed (so we don't generate empty audit-log PATCH entries). - Out-of-range values (>2,592,000 and 0) show the inline error, flip aria-invalid on the input, and disable the Save button. - Discard reverts the TTL override to the fetched value. - setCacheTtl failures (e.g. 503 from a misconfigured proxy) surface the existing 'Failed to save cache TTL' error toast. Mock surface for repositoriesApi extended with getCacheTtl / setCacheTtl. No production code changes. * fix(tickets): bind download/stream ticket resource_path to the request path The backend ticket middleware compares the bound resource_path against request.uri().path() by byte equality at consume time, so the minter must send the exact absolute path the later request carries. createDownloadTicket sent `${repoKey}/${artifactPath}` and createStreamTicket sent `migration/${jobId}`. Both were non-absolute (rejected with "resource_path must start with '/'") and would not have byte-matched the real request path even if made absolute, so UI artifact downloads and migration progress streaming failed. Bind to the actual request paths: - /api/v1/repositories/{repoKey}/download/{artifactPath} - /api/v1/migrations/{jobId}/stream Add regression tests asserting the resource_path argument for both call sites. Fixes #453 * fix: absolute download URL and resilient server version display Resolves two v1.2.0 web bugs. #455: The artifact detail "Download URL" field showed a host-less path, so copying it produced a broken URL. Add artifactsApi.getAbsoluteDownloadUrl, which resolves the path against NEXT_PUBLIC_API_URL or the current window origin, and use it for the copyable detail field. #456: The sidebar hid the backend version whenever /health returned a non-2xx status, even though the version is present in the response body. adminApi.getHealth now detects the HealthResponse shape on the SDK error path (the backend returns 503 with a full body when degraded) and adapts it, so the version stays visible. Adds unit tests for both helpers and a Playwright spec under e2e/suites/interactions/repositories verifying the copied Download URL is absolute and well-formed and that the sidebar shows the backend version. * feat(maven): search by GAV and surface POM in the UI Adds Maven GAV-aware search and POM access for Maven repositories. Search (#441): the GAVC tab previously sent groupId/artifactId/version as the backend's path/name/version params, which advanced-search ignores (it matches a single full-text query over name + path + version). The tab now folds the GAV, classifier, and a new Extension field into one query string scoped to the maven format, so coordinate searches return results. A shared buildMavenSearchQuery helper centralizes the term assembly. POM and GAV (#442): the grouped Maven browser now renders each component file as a download link (path derived from the GAV layout) and tags the .pom file with a POM badge. The artifact detail view gains a Maven section showing parsed groupId/artifactId/version and a copy/paste pom.xml dependency snippet. New src/lib/maven.ts holds the pure coordinate helpers (query building, path derivation, POM detection, GAV parsing, snippet rendering) with unit tests, plus a Playwright spec covering GAV search and POM reachability. * fix(a11y): repo dialog and SSO form WCAG 2.2 AA gaps Address four accessibility gaps in the repository create/edit dialogs and SSO settings forms: - #410: the repo edit dialog now mirrors the upstream-auth save outcome into an in-dialog aria-live region (role="status" on success, role="alert" with aria-live="assertive" on error) so screen-reader users hear the result. The parent mutation previously surfaced only a visual toast. - #411: the create-dialog duplicate-key error now has role="alert", and the key input sets aria-invalid and aria-describedby pointing at the message. - #412: toggling the upstream-auth view to edit (and back) now moves focus to the first control of the newly-revealed view instead of dropping focus on the body. - #413: required inputs across the repo dialogs and SSO OIDC/LDAP/SAML forms expose aria-required; the edit-dialog auth-type select gains a programmatic label; the repo row actions menu trigger gains an accessible name; the edit-key warning is associated via aria-describedby. Adds a Playwright spec under e2e/suites/interactions/repositories asserting the error/alert association, focus movement on toggle, and live-region save announcement. * fix(repositories): pagination, file click-through, and full file listing in Maven grouped view Grouped (Maven/Gradle) mode in the repository browser had three defects: - #443: the grouped view rendered no pagination, so only the first 20 GAV components were ever shown. Extracted the DataTable pagination markup into a shared DataTablePagination component and wired it into MavenComponentList, driven by the same page/pageSize state the flat list uses. - #444: file rows inside an expanded GAV group were plain text and did nothing on click. They are now buttons that reconstruct the Maven path (groupId/artifactId/version/filename) and open the artifact detail dialog, matching flat-mode behaviour. - #445: non-jar files such as .zip and checksum sidecars were not openable in grouped mode. The grouped response already carries every filename per GAV; surfacing each as a clickable row now exposes them with full metadata via the detail dialog. Also fixed artifactsApi.get to encode path segments individually so the backend wildcard route (/:key/artifacts/*path) matches reconstructed paths instead of 404ing on percent-encoded slashes. Closes #443 Closes #444 Closes #445 * test(e2e): verify remote-repo cached artifacts show in the browser (#424) Adds a Playwright spec covering the v1.2.0 regression where packages pulled through a remote (proxy) repository filled up storage but never appeared in the repo browser, so they couldn't be browsed or scanned. The spec pulls a small package through the seeded `e2e-npm-remote` proxy, then asserts the cached entry appears both in the listing API (GET /api/v1/repositories/{key}/artifacts) and in the Artifacts tab of the repo detail page. It skips gracefully if the upstream registry is unavailable, since upstream availability is not what's under test. The fix is backend-only (artifact-keeper#1567 / #1548): the listing for remote repos is now reconstructed from the proxy cache. The web client already consumed the corrected data through the same endpoint, so no web code change is required. Closes #424 * feat(search): surface OpenSearch capabilities in the search UI The backend migrated search indexing from Meilisearch to OpenSearch in 1.2.0. This wires the search UI to the capabilities that migration exposes and fixes a health-card regression caused by the renamed health check. Search results: - Add relevance as the default sort (sends no sort_by so the backend applies its own ranking) plus a Downloads sort option. - Add a sort-direction toggle that sends sort_order=asc|desc; it is disabled while sorting by relevance, which has no direction. - Drop the client-side re-sort that only reordered the current page and fought the server's ordering across paginated results. - Render the highlight snippets the backend returns, parsing the <em> markers into React nodes rather than injecting raw HTML. - Surface the formats and repositories facets as clickable refine chips that re-issue the search with the selected facet applied. System health: - Read the search-engine health check from checks.opensearch, falling back to the legacy checks.meilisearch field so the dashboard "Search Engine" card renders against both backend versions. Adds a Playwright spec covering the sort options, direction toggle, sort_by/sort_order request params, relevance default, and facet refine behavior, plus unit tests for the health adapter mapping. * feat(repositories): add release target and routing rules settings Add two repository Settings tab sections: Release target (#260): staging repositories can be linked to a local release repository of the same format. The picker lists eligible local repos and saves through PATCH /repositories/{key} with release_repository_key. An empty selection unlinks. Non-staging repos show a notice instead of the control. Routing rules (#263): view, add, edit, and remove path-rewriting rules for remote, virtual, and staging repositories. Each rule is a regex path_pattern and a rewrite_to template referencing capture groups. Rules are stored as a single ordered list via the routing-rules endpoints; removing the last rule clears the config. Both sections reuse the repositoriesApi wrapper, with new getRoutingRules, setRoutingRules, deleteRoutingRules, and setReleaseTarget methods backed by the shared apiFetch helper since the generated SDK does not expose these endpoints yet. Adds Playwright e2e specs covering the API contracts and the UI flows for both features. * Add package age policy and upload size limit configuration UIs Adds two repository configuration surfaces for v1.2.0: - Package age policy (#265): a Package Age Policy section on the repo Settings tab that holds freshly published packages in quarantine for a configurable cooldown window. Sends quarantine_enabled and quarantine_duration_minutes to PATCH /api/v1/repositories/{key}. - Upload size limit (#189): the admin Settings -> Storage tab now lets admins edit the max upload size (value + MB/GB unit), persisted via POST /api/v1/admin/settings. The repo Settings tab surfaces the effective limit read-only so repo owners see the ceiling that applies. Includes unit tests for the new helpers and components, plus Playwright e2e specs covering both configuration flows. * feat: system config feature flags and rate-limit exemption admin UI Add a SystemConfigProvider that fetches GET /api/v1/system/config and exposes derived feature flags through useSystemConfig/useFeatureFlags. Gate the scanner-dependent sidebar entries (Scan Results, DT Projects) on the reported scanner flags, and surface the configured max upload size in the artifact upload dropzone with a client-side oversize guard. Add a Rate Limits admin page that shows the effective per-window limits and lets admins view, add, and remove rate-limit exemptions for usernames, service accounts, and CIDR ranges. The page degrades gracefully when the backend has not shipped the exemption-management endpoints. Includes unit tests for the new API clients and provider, updates to the sidebar tests for flag-driven gating, and Playwright e2e specs covering feature-flag gating and the exemption admin flow. Closes #271 Closes #270 * fix(settings): reflect persisted max upload size in editor (review #464) The UploadSizeSetting useState initializer seeded from an undefined currentBytes while admin-settings was still loading, so the editor showed an empty 'No limit' even when a limit was configured and never refreshed once the query resolved. Sync local state during render when the persisted value changes, gated on !dirty so unsaved edits are never clobbered. * fix(repositories): a11y error association + write-through guards (review #462/#464) - Routing rules: associate the add-rule validation error with the pattern input via aria-invalid + aria-describedby on a persistent role=alert element, validate the regex inline, and gate the resync on !dirty so a window-focus refetch no longer clobbers unsaved edits. - Age policy: associate the cooldown error with the input via a persistent role=alert region, and disable Save until an explicit change so a pristine form cannot overwrite an existing policy. - Release target: disable Save until the selection changes, so a pristine picker cannot unlink an existing target. * fix(search,admin): aria-live result announcements + exemption a11y, harden e2e (review #463/#465/#464) - Search: add a visually-hidden aria-live status region that announces the result count, active sort, and active facets so sort/filter changes are not silent to screen readers. - Rate-limit exemption dialog: associate the validation error with the value input via aria-invalid + aria-describedby on a persistent role=alert element instead of a toast. - E2E hardening: the upload-size and age-policy save tests now assert the POST/PATCH actually fires and succeeds rather than passing vacuously; the rate-limit exemption add/remove round-trip now asserts instead of skipping. * chore: bump web version to 1.2.0 * test(search): expect facets + quarantine fields from advancedSearch (#463) PR #463 extended the advancedSearch adapter to surface OpenSearch facets and per-item quarantine fields but left the unit test's exhaustive toEqual stale, so it failed once merged. Update the expectation to match the adapter contract. * fix(repositories): cache-UI review fixes for TTL a11y, invalidate confirm, dead code Addresses code-review findings on the cache-UI PRs that landed on release/web-1.2.0 after the integration branch was built. repo-settings-tab.tsx (#450 proxy cache TTL): - Associate the TTL validation error with the input via aria-describedby pointing at a persistent role="alert" element (was a conditional, unassociated paragraph), mirroring the age-policy field pattern. - Disable the Discard button while a Save is in flight so it cannot race an in-flight mutation. repo-detail-content.tsx (#447 invalidate cache): - Wrap the destructive "Invalidate cache" action in an AlertDialog confirmation instead of firing immediately on click. - After a successful invalidate, close the details dialog so it does not show stale cache_cached_at / cache_expires_at fields. - Announce delete and invalidate outcomes through a dedicated polite live region in addition to the toast. - Add the missing trailing newline at EOF (eol-last). cache-time.ts (#451 cache rows): - Remove the unreachable "return iso" after the exhaustive unit loop by dropping the always-matching "second" ladder entry and making the sub-minute case an explicit fallback. Validation: eslint clean on changed files, tsc reports only the two accepted pre-existing test errors, affected vitest suites pass (55 tests), playwright --list parses all 528 specs. * test(e2e): fix repo-create field + force flat view for maven artifact-table tests Two stale-assumption failures that were red across all PRs and on main: - api-comprehensive 'creates test repo' sent `type: local` but the backend requires `repo_type` -> 400 VALIDATION_ERROR (missing field repo_type). - artifact-download and download-url-and-version navigate to a Maven repo and wait for getByRole('table'). Maven repos default to the grouped component view (MavenComponentList), which is not a <table>, so the locator timed out. Navigate with ?view=flat so the flat DataTable renders, matching the tests' row/detail-panel assumptions. Pure test fixes; no app behavior change. * test: raise new-code coverage to >=80% for the v1.2.0 release batch * test(e2e): make v1.2.0 feature specs robust against the e2e environment * test(e2e): fix remaining v1.2.0 interactions spec failures - analytics: combined stat-card locator matched 2 elements, assert .first() - system-config: sidebar is a shadcn data-sidebar container, not a <nav> landmark; key off a guaranteed link and scope scanner-link queries to it - maven-grouped: #444 and #445 seed the same with-zip GAV, so the second seed/retry got 409; tolerate already-deployed (409) in the PUT helper - remote-cached: server-side q filter on a proxy repo can hide the reconstructed cache row; assert the row directly, skip as a backstop since the API-level test is the authoritative #424 guard - repo-dialog-a11y: target the remote repo's actions button by name and retry the menu open (a background list refetch dismissed the dropdown) * test(e2e): bind repo-dialog-a11y to the actual actions trigger The repo row is itself a button whose accessible name concatenates the nested actions-button label, so a substring/.first() match selected the row card (which just selects the repo) instead of the DropdownMenu trigger, so the Edit menuitem never appeared. Match the trigger by exact accessible name. Root-caused from the shard-3 Playwright trace snapshot. --------- Co-authored-by: knowinglyAnonymous <stupidsimpleman8@gmail.com>
Contributor
|
Thank you for this contribution. Your work shipped in v1.2.0: the cached_at / expires_at display in the artifact details dialog feature was incorporated into the release integration branch #466, which was squash-merged to main and tagged as v1.2.0, resolving #449. The feature is live on main now. Closing this PR as shipped-via-#466 (not as a rejection). Apologies that the integration-branch workflow left your original PR open after the release rather than closing it with attribution at merge time. Really appreciate the contribution. |
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
2 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
Surfaces the proxy-cache
cached_atandexpires_attimestamps as relative-time rows on the artifact details dialog, so operators reading the panel can decide between waiting for the natural refresh and clicking "Invalidate cache" without leaving the UI.Closes #449. Companion backend work tracked in artifact-keeper#1541 (PR artifact-keeper#1542).
What changes
src/lib/cache-time.ts(new) — two pure helpers, both(iso, now?) => string:formatRelativeTimestamp(iso)→ "in 4 hours" / "12 minutes ago" / "expired 3 days ago". Picks the largest unit that gives a magnitude>= 1so the output stays compact (no "120 minutes ago"). UsesIntl.RelativeTimeFormatso the output is locale-aware without pulling indate-fns.formatCacheExpiry(iso)→ biased for the "expires" framing; past timestamps render as "expired N units ago, will re-fetch on next download" so operators reading the row know what'll happen without checking docs.nowparameter so the tests can pin output deterministically; production callers omit it.src/types/index.ts—Artifactgainscache_cached_at?: string | nullandcache_expires_at?: string | null. Optional + nullable so the type tolerates the backend'sskip_serializing_if = "Option::is_none"shape (key omitted entirely) and the defensive case of an explicitnull.src/lib/api/artifacts.ts—adaptArtifactplumbs the new fields through. The generated SDK type doesn't carry them yet (the SDK regenerates from the OpenAPI spec only after the backend PR lands); until then they ride through via a narrowed runtime cast. Once the SDK regenerates, the cast can collapse to direct property access — comment in the file points this out.src/app/(app)/repositories/_components/repo-detail-content.tsx— adds two newDetailRows in the dialog "Details" tab (between "Created" and "SHA-256"):formatRelativeTimestamp(cache_cached_at)as the visible value, full ISO-8601 timestamp as the hover-tooltip.formatCacheExpiry(cache_expires_at), same tooltip pattern.repository.repo_type === "remote"AND the value being present. Local / Virtual / Staging never show them; Remote repos without a cache-metadata blob (e.g. an artifact direct-uploaded but never proxy-fetched) also hide the rows rather than rendering empty cells.DetailRowgains an optionaltitleprop so the visible value (e.g. "in 4 hours") can differ from the hover-tooltip (e.g. "6/2/2026, 11:00:00 AM"). Defaults to the value, so existing callers are unaffected.What does not change
Test Checklist
src/lib/__tests__/cache-time.test.ts(new) — 9 cases covering: future direction, past direction, largest-unit selection (days, not 36 hours), unparseable-timestamp fallback, and the "expired ..., will re-fetch" framing including the boundaryt == nowcase.src/lib/api/__tests__/artifacts.test.ts— 3 new cases pinningadaptArtifactplumbing: fields populated when present, fieldsundefinedwhen omitted, fieldsundefinedwhen the backend explicitly sendsnull(defensive against future serialization changes). Total: 28/28 pass (was 25 before).repo_type === "remote"gate cover the visibility contract.npm run lint(0 errors, 36 pre-existing warnings unchanged),npm run build(clean),npx tsc --noEmit(no new errors; same 2 pre-existing onmainunchanged),npx vitest runfor both new test files (37/37 pass).UI Changes
DetailRowgrid (grid-cols-[100px_1fr]) used by Created / SHA-256 / Download URL.text-muted-foreground); no custom colors.Companion Work
cache_cached_atandcache_expires_atonGET /:key/artifacts/:path. Hard dependency: the rows render only when the response actually carries the fields. The web change is forward-compatible — until the backend PR lands the rows simply don't appear, no errors.