feat(console): add the Ariada accessibility audit console (OSS) - #257
feat(console): add the Ariada accessibility audit console (OSS)#257alexander-brichkin wants to merge 1 commit into
Conversation
A SvelteKit (adapter-static) operator console for managing accessibility audits across projects — the whole GNU Taler ecosystem is the first project. Self-contained: depends only on its own vendored kit — @ariada-org/admin-surface (a framework-neutral surface/grid/profile contract) and @ariada-org/admin-svelte (a Svelte 5 renderer over ag-grid, zero runtime deps) — plus ag-grid-community. No external UI framework, no shared/private dependency. Live demo: app.ariada.org. Signed-off-by: Alexander Brichkin (Agonist Development AB) <git@ariada.org>
📝 WalkthroughWalkthroughChangesAdmin surface contracts
Svelte renderer
Console application
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant Console
participant AdminGrid
participant AuditAPI
Browser->>Console: authenticate and open dashboard
Console->>AdminGrid: pass board, profile, and audit rows
AdminGrid->>AuditAPI: submit selected row action
AuditAPI-->>AdminGrid: return action response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| it('renders nothing when no row is open', () => { | ||
| const { body } = render(RowDetailDrawer, { props: { surface: SURFACE, row: null, onClose: () => {} } }); | ||
| expect(body.replace(/<!--[\s\S]*?-->/g, '').trim()).toBe(''); |
| /** a 24x24 stroke-icon SVG string for an action key (falls back to a dot). */ | ||
| export function actionIconSvg(key: string, size = 14): string { | ||
| const paths = ACTION_ICON_PATHS[key] ?? FALLBACK_ICON; | ||
| return `<svg width="${size}" height="${size}" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.8" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true">${paths}</svg>`; |
| /** a 24x24 stroke-icon SVG string for an action key (falls back to a dot). */ | ||
| export function actionIconSvg(key: string, size = 14): string { | ||
| const paths = ACTION_ICON_PATHS[key] ?? FALLBACK_ICON; | ||
| return `<svg width="${size}" height="${size}" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.8" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true">${paths}</svg>`; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (3)
packages/admin-surface/.gitignore (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlso ignore the build output.
tsconfig.build.jsonemits todist/, which isn't ignored here.♻️ Proposed change
node_modules/ +dist/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/admin-surface/.gitignore` at line 1, Update the ignore rules in the project’s .gitignore to include the dist/ directory emitted by tsconfig.build.json, while preserving the existing node_modules/ rule.packages/admin-svelte/package.json (1)
58-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
workspace:*instead offile:for the sibling package.The README's own install example uses
"@ariada-org/admin-surface": "workspace:*", but this devDependency pinsfile:../admin-surface. Mixing link protocols for the same in-repo package across the monorepo increases the odds of exactly the kind of lockfile-specifier drift currently failing CI.♻️ Proposed fix
"devDependencies": { - "`@ariada-org/admin-surface`": "file:../admin-surface", + "`@ariada-org/admin-surface`": "workspace:*",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/admin-svelte/package.json` at line 58, Update the `@ariada-org/admin-surface` devDependency in packages/admin-svelte/package.json from the file:../admin-surface protocol to workspace:*, matching the monorepo’s established workspace dependency specifier and README example.packages/admin-svelte/src/format.ts (1)
101-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLeftover domain-specific vocabulary in a supposedly generic render layer.
TAG_COLORbakes in ad-tech/traffic-specific values (feeder,barter,ftp,block,widget,in-article,header,footer) right next to aSEVERITYmap explicitly commented as "Generic operational vocabulary only; no product-specific statuses" — andDEFAULT_WIKIhardcodeswiki.klarads.com, an unrelated product's domain. This package is now vendored for Ariada/accessibility-audit use, and the README emphasizes "no product name appears in the render layer." Unused, unrelated fallback vocabulary and a foreign default domain undercut that guarantee and can mislead the next integrator about what's actually generic.Also applies to: 169-177
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/admin-svelte/src/format.ts` around lines 101 - 131, Remove the ad-tech/traffic-specific entries from TAG_COLOR, retaining only vocabulary that is generic to the render layer and preserving the existing fallback behavior in tagColor. Also update DEFAULT_WIKI to eliminate the hardcoded Klarads domain, using the package’s generic/configurable default instead. Ensure no product-specific names or fallback vocabulary remain in the render-layer formatting definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ariada-console/package.json`:
- Around line 14-16: Regenerate the workspace lockfile after adding the linked
dependencies in the apps/ariada-console manifest, ensuring the lockfile reflects
the packages/admin-surface specifiers and allows pnpm install --frozen-lockfile
to succeed. Commit the updated pnpm-lock.yaml without changing unrelated
dependency definitions.
In `@apps/ariada-console/src/lib/projects/taler.ts`:
- Line 7: Remove the internal “grants/” repository-path reference from the
comment text in taler.ts, replacing it with public-facing wording or removing
that reference while preserving the surrounding documentation meaning.
- Around line 48-49: Update the project resource definitions for taler-wallet
and taler-popup so scans target distinct reachable surfaces instead of sharing
the same URL and results. Prefer adding an explicit scope or selector supported
by AuditRow; otherwise use distinct valid targets or consolidate them into one
resource, ensuring findings are not duplicated or misattributed.
In `@apps/ariada-console/src/routes/`+page.svelte:
- Around line 11-23: Wire apps/ariada-console/src/routes/+page.svelte lines
11-23 through the action service so scan/remediation execute, report actions use
the returned report token, and unavailable actions are disabled or omitted
instead of only logged. In apps/ariada-console/src/routes/r/[token]/+page.svelte
lines 20-27, resolve the token via the report service and enforce TTL/expiry and
revocation checks before rendering the report.
In `@apps/ariada-console/src/routes/subject/`+page.svelte:
- Around line 6-7: Validate the query-derived url before it reaches the DOM:
update the url derivation in the subject page to parse the value and retain it
only when its protocol is http: or https:, otherwise use an empty value. Ensure
both the subject link href and iframe src consume this validated value,
preserving the existing heal behavior.
In `@packages/admin-surface/package.json`:
- Around line 26-30: Update the root pnpm-lock.yaml by running pnpm install from
the repository root so the importers for packages/admin-surface,
packages/admin-svelte, and apps/ariada-console include their declared
devDependencies, including `@types/node`, typescript, and vitest. Commit the
regenerated lockfile without changing the package manifests.
In `@packages/admin-surface/README.md`:
- Around line 3-6: Retarget the README’s product and package references from
KlarAds, Audiofirst, Agonist, and non-vendored consumers to the Ariada console
and the vendored `@ariada-org/admin-surface` and `@ariada-org/admin-svelte`
packages. Update the renderer table accordingly, removing the
`@ariada-org/admin-ui/React` row or clearly marking it as not yet vendored.
In `@packages/admin-surface/src/index.ts`:
- Around line 372-373: Validate the profile `accent` property with the existing
literal `CSS_HEX` rule before it reaches styling consumers, rejecting arbitrary
CSS values such as URLs or declarations. Update the related profile validation
around `accent` and ensure `CSS_HEX` is declared before that validator, or
otherwise arrange the check without relying on a later declaration; preserve
valid hex accent values.
- Around line 366-368: Require actions in validateOperatorDashboardProfile so
every accepted OperatorDashboardProfile includes the non-optional readonly
actions array. Update the validator’s optional-field handling to reject profiles
that omit actions while preserving its existing validation for provided action
keys.
In `@packages/admin-svelte/package.json`:
- Around line 1-67: Regenerate the workspace pnpm-lock.yaml so its specifiers
match the current packages/admin-surface/package.json dependency declarations,
then include the updated lockfile with the change. Use pnpm install
--no-frozen-lockfile before committing so frozen-lockfile CI can install and run
the admin-svelte build, test, and typecheck scripts.
In `@packages/admin-svelte/src/renderers.ts`:
- Around line 243-253: Update the column-help affordance in the renderer around
info and its mouse listeners so it is keyboard- and touch-operable: make the
element focusable, open the popover on focus and an appropriate click/activation
event, and schedule closing on blur while preserving the existing mouse behavior
and propagation handling.
In `@packages/admin-svelte/src/RowDetailDrawer.svelte`:
- Around line 53-63: Update setField and save in RowDetailDrawer so numeric-kind
inputs are validated before persistence: track whether any numeric field
contains a non-finite or otherwise invalid value, show the existing validation
feedback for that field, and prevent save() from invoking onSave while invalid
numeric input remains. Preserve current coercion for valid numeric values and
normal string-field saving.
In `@packages/admin-svelte/src/tokens.css`:
- Around line 79-83: Insert a blank line before the color-scheme declaration in
the token block containing --adm-shadow-sm, --adm-shadow-md, and
--adm-shadow-lg, satisfying the declaration-empty-line-before Stylelint rule
without changing the declaration values.
- Around line 58-59: Update the font-family declarations `--adm-font` and
`--adm-font-mono` in `tokens.css` with a narrowly scoped Stylelint disable for
`value-keyword-case`, preserving the canonical casing of all font names and
keeping the suppression limited to these declarations.
---
Nitpick comments:
In `@packages/admin-surface/.gitignore`:
- Line 1: Update the ignore rules in the project’s .gitignore to include the
dist/ directory emitted by tsconfig.build.json, while preserving the existing
node_modules/ rule.
In `@packages/admin-svelte/package.json`:
- Line 58: Update the `@ariada-org/admin-surface` devDependency in
packages/admin-svelte/package.json from the file:../admin-surface protocol to
workspace:*, matching the monorepo’s established workspace dependency specifier
and README example.
In `@packages/admin-svelte/src/format.ts`:
- Around line 101-131: Remove the ad-tech/traffic-specific entries from
TAG_COLOR, retaining only vocabulary that is generic to the render layer and
preserving the existing fallback behavior in tagColor. Also update DEFAULT_WIKI
to eliminate the hardcoded Klarads domain, using the package’s
generic/configurable default instead. Ensure no product-specific names or
fallback vocabulary remain in the render-layer formatting definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0595799b-60ae-4fc8-a99b-0c555f68eb3f
⛔ Files ignored due to path filters (1)
apps/ariada-console/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
apps/ariada-console/.gitignoreapps/ariada-console/package.jsonapps/ariada-console/src/app.d.tsapps/ariada-console/src/app.htmlapps/ariada-console/src/lib/boards/audits.tsapps/ariada-console/src/lib/projects/taler.tsapps/ariada-console/src/routes/+layout.svelteapps/ariada-console/src/routes/+layout.tsapps/ariada-console/src/routes/+page.svelteapps/ariada-console/src/routes/app.cssapps/ariada-console/src/routes/r/[token]/+page.svelteapps/ariada-console/src/routes/subject/+page.svelteapps/ariada-console/svelte.config.jsapps/ariada-console/tsconfig.jsonapps/ariada-console/vite.config.tspackages/admin-surface/.gitignorepackages/admin-surface/README.mdpackages/admin-surface/package.jsonpackages/admin-surface/src/chart.test.tspackages/admin-surface/src/grid.test.tspackages/admin-surface/src/index.test.tspackages/admin-surface/src/index.tspackages/admin-surface/templates/admin-surface.ts.templatepackages/admin-surface/tsconfig.build.jsonpackages/admin-surface/tsconfig.jsonpackages/admin-svelte/.gitignorepackages/admin-svelte/README.mdpackages/admin-svelte/package.jsonpackages/admin-svelte/src/AdminGrid.sveltepackages/admin-svelte/src/MetricChart.sveltepackages/admin-svelte/src/RowDetailDrawer.sveltepackages/admin-svelte/src/chart.test.tspackages/admin-svelte/src/chart.tspackages/admin-svelte/src/components.test.tspackages/admin-svelte/src/format.test.tspackages/admin-svelte/src/format.tspackages/admin-svelte/src/i18n.tspackages/admin-svelte/src/icons.tspackages/admin-svelte/src/index.tspackages/admin-svelte/src/renderers.test.tspackages/admin-svelte/src/renderers.tspackages/admin-svelte/src/ssr.test.tspackages/admin-svelte/src/theme.tspackages/admin-svelte/src/tokens.csspackages/admin-svelte/tsconfig.build.jsonpackages/admin-svelte/tsconfig.jsonpackages/admin-svelte/vitest.config.ts
| "@ariada-org/admin-surface": "file:../../packages/admin-surface", | ||
| "@ariada-org/admin-svelte": "file:../../packages/admin-svelte", | ||
| "ag-grid-community": "^36.0.2" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Regenerate and commit the workspace lockfile.
pnpm install --frozen-lockfile currently fails because pnpm-lock.yaml does not match the linked package manifests, so CI cannot install or build this app. Run the workspace install and commit the updated lockfile, including the packages/admin-surface specifiers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ariada-console/package.json` around lines 14 - 16, Regenerate the
workspace lockfile after adding the linked dependencies in the
apps/ariada-console manifest, ensuring the lockfile reflects the
packages/admin-surface specifiers and allows pnpm install --frozen-lockfile to
succeed. Commit the updated pnpm-lock.yaml without changing unrelated dependency
definitions.
Source: Pipeline failures
| // The first dogfood PROJECT: the GNU Taler / GNUnet ("никс") ecosystem. One | ||
| // selectable entity grouping every resource — core surfaces, the two pilot | ||
| // banks, partners, and the grant-funded module landings (see | ||
| // grants/TALER_ECOSYSTEM_CATALOG_2026-07-30.md). Rows feed AUDITS_BOARD. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Remove the internal repository-path reference.
The content gate fails on grants/, blocking this PR. Replace it with public-facing text or remove the reference.
🧰 Tools
🪛 GitHub Actions: content-gate / 0_content-gate.txt
[error] 7-7: Content policy scan failed in node packages/ariada-content-policy/dist/cli.js $FILES. Reported internal-path violation: "grants/"
🪛 GitHub Actions: content-gate / content-gate
[error] 7-7: Content policy scan failed: internal path violation flagged for "grants/".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ariada-console/src/lib/projects/taler.ts` at line 7, Remove the internal
“grants/” repository-path reference from the comment text in taler.ts, replacing
it with public-facing wording or removing that reference while preserving the
surrounding documentation meaning.
Source: Pipeline failures
| { id: 'taler-wallet', label: 'Taler wallet — full UI', url: 'https://addons.mozilla.org/firefox/addon/taler-wallet/', group: 'core', owner: 'GNU Taler', scanStatus: 'scanned', score: 9, critical: 0, serious: 4, findings: 8, blastRadius: null, gate: 'blocked', reverter: 'recommended' }, | ||
| { id: 'taler-popup', label: 'Taler wallet — confirm popup', url: 'https://addons.mozilla.org/firefox/addon/taler-wallet/', group: 'core', owner: 'GNU Taler', scanStatus: 'scanned', score: 9, critical: 0, serious: 4, findings: 8, blastRadius: null, gate: 'blocked', reverter: 'recommended' }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Give these distinct surfaces distinct scan targets.
Both rows use the same Mozilla Add-ons URL and identical results, while AuditRow has no scope/selector field. Scans cannot distinguish the wallet UI from its confirmation popup, so counts can be duplicated or attributed to the wrong surface. Use distinct reachable targets, add an explicit scan scope, or represent this as one resource.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ariada-console/src/lib/projects/taler.ts` around lines 48 - 49, Update
the project resource definitions for taler-wallet and taler-popup so scans
target distinct reachable surfaces instead of sharing the same URL and results.
Prefer adding an explicit scope or selector supported by AuditRow; otherwise use
distinct valid targets or consolidate them into one resource, ensuring findings
are not duplicated or misattributed.
| function onAction(row: any, action: { key: string; endpoint: string }, reason?: string) { | ||
| if (action.key === 'plugin') { | ||
| // Live-plugin loop: open the console's subject-viewer with the URL so the | ||
| // Chrome extension can show the healed before/after. Placeholder route. | ||
| window.open(`/subject?url=${encodeURIComponent(row.url)}&heal=1`, '_blank'); | ||
| return; | ||
| } | ||
| if (action.key === 'report' && row.reportToken) { | ||
| window.open(`/r/${row.reportToken}`, '_blank'); | ||
| return; | ||
| } | ||
| // scan / remediate → guarded API (not wired in the scaffold) | ||
| console.info('action', action.key, row.id, reason); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Wire the advertised action and report flows before exposing them.
AuditRow has no reportToken, so “Open report” always falls through to logging; scan and remediation also only log. The public token route then renders no report and performs no TTL/revocation validation.
apps/ariada-console/src/routes/+page.svelte#L11-L23: invoke the action service, use its returned report token, and disable or omit unavailable actions.apps/ariada-console/src/routes/r/[token]/+page.svelte#L20-L27: resolve the token through the report service and enforce its expiry/revocation policy before rendering.
📍 Affects 2 files
apps/ariada-console/src/routes/+page.svelte#L11-L23(this comment)apps/ariada-console/src/routes/r/[token]/+page.svelte#L20-L27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ariada-console/src/routes/`+page.svelte around lines 11 - 23, Wire
apps/ariada-console/src/routes/+page.svelte lines 11-23 through the action
service so scan/remediation execute, report actions use the returned report
token, and unavailable actions are disabled or omitted instead of only logged.
In apps/ariada-console/src/routes/r/[token]/+page.svelte lines 20-27, resolve
the token via the report service and enforce TTL/expiry and revocation checks
before rendering the report.
| const url = $derived(page.url.searchParams.get('url') ?? ''); | ||
| const heal = $derived(page.url.searchParams.get('heal') === '1'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate the subject URL before binding it to the DOM.
url is attacker-controlled query-string data and is used directly in href and iframe src. Svelte escapes the attribute value but does not reject executable schemes such as javascript:. Parse the value and allow only the intended http:/https: protocols before rendering either element.
Proposed fix
- const url = $derived(page.url.searchParams.get('url') ?? '');
+ const url = $derived.by(() => {
+ const raw = page.url.searchParams.get('url') ?? '';
+ if (!raw) return '';
+ try {
+ const parsed = new URL(raw, page.url.origin);
+ return parsed.protocol === 'http:' || parsed.protocol === 'https:'
+ ? parsed.href
+ : '';
+ } catch {
+ return '';
+ }
+ });Also applies to: 16-16, 23-24
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ariada-console/src/routes/subject/`+page.svelte around lines 6 - 7,
Validate the query-derived url before it reaches the DOM: update the url
derivation in the subject page to parse the value and retain it only when its
protocol is http: or https:, otherwise use an empty value. Ensure both the
subject link href and iframe src consume this validated value, preserving the
existing heal behavior.
| { | ||
| "name": "@ariada-org/admin-svelte", | ||
| "version": "0.1.0", | ||
| "description": "Shared Svelte 5 render layer for Agonist admin surfaces: contract-driven AG Grid, declarative charts, design tokens. The Svelte twin of @ariada-org/admin-ui.", | ||
| "license": "EUPL-1.2", | ||
| "author": "Agonist Development AB", | ||
| "keywords": [ | ||
| "design-system", | ||
| "svelte", | ||
| "ag-grid", | ||
| "admin-ui", | ||
| "agonist" | ||
| ], | ||
| "type": "module", | ||
| "sideEffects": [ | ||
| "*.css" | ||
| ], | ||
| "main": "./dist/index.js", | ||
| "types": "./dist/index.d.ts", | ||
| "svelte": "./src/index.ts", | ||
| "exports": { | ||
| ".": { | ||
| "types": "./dist/index.d.ts", | ||
| "import": "./dist/index.js", | ||
| "default": "./dist/index.js" | ||
| }, | ||
| "./AdminGrid.svelte": { | ||
| "svelte": "./src/AdminGrid.svelte", | ||
| "default": "./src/AdminGrid.svelte" | ||
| }, | ||
| "./MetricChart.svelte": { | ||
| "svelte": "./src/MetricChart.svelte", | ||
| "default": "./src/MetricChart.svelte" | ||
| }, | ||
| "./RowDetailDrawer.svelte": { | ||
| "svelte": "./src/RowDetailDrawer.svelte", | ||
| "default": "./src/RowDetailDrawer.svelte" | ||
| }, | ||
| "./tokens.css": "./src/tokens.css", | ||
| "./package.json": "./package.json" | ||
| }, | ||
| "files": [ | ||
| "dist", | ||
| "src", | ||
| "README.md" | ||
| ], | ||
| "peerDependencies": { | ||
| "@ariada-org/admin-surface": ">=0.1.0", | ||
| "ag-grid-community": ">=36", | ||
| "svelte": ">=5" | ||
| }, | ||
| "scripts": { | ||
| "build": "tsc -p tsconfig.build.json", | ||
| "test": "vitest run src", | ||
| "typecheck": "tsc --noEmit -p tsconfig.json && svelte-check --tsconfig ./tsconfig.json --threshold error" | ||
| }, | ||
| "devDependencies": { | ||
| "@ariada-org/admin-surface": "file:../admin-surface", | ||
| "@sveltejs/vite-plugin-svelte": "^4.0.4", | ||
| "ag-grid-community": "^36.0.2", | ||
| "svelte": "^5.1.0", | ||
| "svelte-check": "^4.4.6", | ||
| "typescript": "^5.8.3", | ||
| "vite": "^5.4.21", | ||
| "vitest": "^2.1.9" | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
CI is failing on frozen-lockfile install; this package's install/build/test will not run.
Pipeline logs show pnpm install --frozen-lockfile fails because pnpm-lock.yaml doesn't match packages/admin-surface/package.json specifiers. Since this package depends on admin-surface in the same workspace, the whole install (and therefore this package's build/test/typecheck scripts) is blocked until the lockfile is regenerated.
# regenerate the lockfile after any package.json dependency change
pnpm install --no-frozen-lockfile
git add pnpm-lock.yaml🧰 Tools
🪛 GitHub Actions: Snyk / Snyk SCA + Code
[error] 1-1: pnpm install --frozen-lockfile failed (ERR_PNPM_OUTDATED_LOCKFILE). pnpm-lock.yaml is not up to date with packages/admin-surface/package.json. specifiers in the lockfile ({}) don't match specs in package.json ({"@types/node":"^22.0.0","typescript":"^5.8.3","vitest":"^2.1.9"}).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/admin-svelte/package.json` around lines 1 - 67, Regenerate the
workspace pnpm-lock.yaml so its specifiers match the current
packages/admin-surface/package.json dependency declarations, then include the
updated lockfile with the change. Use pnpm install --no-frozen-lockfile before
committing so frozen-lockfile CI can install and run the admin-svelte build,
test, and typecheck scripts.
Source: Pipeline failures
| const help = params.help; | ||
| if (!help) return; | ||
| const info = document.createElement('span'); | ||
| info.className = 'adm-header-info'; | ||
| info.textContent = 'ⓘ'; | ||
| info.setAttribute('role', 'note'); | ||
| info.setAttribute('aria-label', help.description); | ||
| info.addEventListener('click', (event) => event.stopPropagation()); | ||
| info.addEventListener('mouseenter', () => this.open(info, params, help)); | ||
| info.addEventListener('mouseleave', () => this.scheduleClose()); | ||
| this.gui.append(info); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Column help popover is mouse-only — unreachable by keyboard or touch.
info only listens for mouseenter/mouseleave, has no tabindex, and no focus/blur/click handling. Since role="note" is already set, Svelte's compiler a11y check for click/hover-without-keyboard won't flag this (role suppresses that warning), so components.test.ts's "zero warnings" assertion won't catch it — but keyboard and touch users still have no way to see the column description/formula/wiki link this popover exists to surface.
♿ Suggested fix: make the info affordance focusable and keyboard-operable
const info = document.createElement('span');
info.className = 'adm-header-info';
info.textContent = 'ⓘ';
- info.setAttribute('role', 'note');
+ info.setAttribute('role', 'button');
+ info.setAttribute('tabindex', '0');
info.setAttribute('aria-label', help.description);
info.addEventListener('click', (event) => event.stopPropagation());
info.addEventListener('mouseenter', () => this.open(info, params, help));
info.addEventListener('mouseleave', () => this.scheduleClose());
+ info.addEventListener('focus', () => this.open(info, params, help));
+ info.addEventListener('blur', () => this.scheduleClose());
+ info.addEventListener('keydown', (event) => {
+ if (event.key === 'Enter' || event.key === ' ') { event.preventDefault(); this.open(info, params, help); }
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/admin-svelte/src/renderers.ts` around lines 243 - 253, Update the
column-help affordance in the renderer around info and its mouse listeners so it
is keyboard- and touch-operable: make the element focusable, open the popover on
focus and an appropriate click/activation event, and schedule closing on blur
while preserving the existing mouse behavior and propagation handling.
| const NUMERIC_KINDS = new Set<string>(['count', 'currency', 'percent', 'score', 'duration', 'ratio']); | ||
| const isNumeric = (column: AdminMetricColumn) => NUMERIC_KINDS.has(column.kind); | ||
|
|
||
| function setField(column: AdminMetricColumn, raw: string): void { | ||
| draft = { ...draft, [column.key]: isNumeric(column) ? toNumber(raw) : raw }; | ||
| } | ||
|
|
||
| function save(): void { | ||
| onSave?.({ ...draft }); | ||
| editing = false; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
No validation feedback for numeric-kind edits — invalid input can be saved silently.
setField coerces numeric-kind values via toNumber(raw) into draft, and save() forwards draft to onSave unconditionally. A non-numeric or cleared value in a numeric field produces an invalid (e.g. NaN) value with no visual cue, and nothing blocks save() from persisting it.
🛡️ Proposed guard before allowing save
function save(): void {
+ for (const column of surface.columns) {
+ if (isNumeric(column) && Number.isNaN(draft[column.key])) return;
+ }
onSave?.({ ...draft });
editing = false;
}Also applies to: 117-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/admin-svelte/src/RowDetailDrawer.svelte` around lines 53 - 63,
Update setField and save in RowDetailDrawer so numeric-kind inputs are validated
before persistence: track whether any numeric field contains a non-finite or
otherwise invalid value, show the existing validation feedback for that field,
and prevent save() from invoking onSave while invalid numeric input remains.
Preserve current coercion for valid numeric values and normal string-field
saving.
| --adm-font: Inter, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; | ||
| --adm-font-mono: ui-monospace, SFMono-Regular, Menlo, monospace; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stylelint value-keyword-case failure on font names.
Font-family identifiers are conventionally kept in their canonical casing (Inter, BlinkMacSystemFont, SFMono-Regular, Menlo); a scoped disable is safer than lowercasing them.
🔧 Proposed fix
- --adm-font: Inter, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
- --adm-font-mono: ui-monospace, SFMono-Regular, Menlo, monospace;
+ /* stylelint-disable-next-line value-keyword-case -- font names must keep canonical casing */
+ --adm-font: Inter, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
+ /* stylelint-disable-next-line value-keyword-case -- font names must keep canonical casing */
+ --adm-font-mono: ui-monospace, SFMono-Regular, Menlo, monospace;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --adm-font: Inter, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; | |
| --adm-font-mono: ui-monospace, SFMono-Regular, Menlo, monospace; | |
| /* stylelint-disable-next-line value-keyword-case -- font names must keep canonical casing */ | |
| --adm-font: Inter, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; | |
| /* stylelint-disable-next-line value-keyword-case -- font names must keep canonical casing */ | |
| --adm-font-mono: ui-monospace, SFMono-Regular, Menlo, monospace; |
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 58-58: Expected "Inter" to be "inter" (value-keyword-case)
(value-keyword-case)
[error] 58-58: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)
(value-keyword-case)
[error] 59-59: Expected "SFMono-Regular" to be "sfmono-regular" (value-keyword-case)
(value-keyword-case)
[error] 59-59: Expected "Menlo" to be "menlo" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/admin-svelte/src/tokens.css` around lines 58 - 59, Update the
font-family declarations `--adm-font` and `--adm-font-mono` in `tokens.css` with
a narrowly scoped Stylelint disable for `value-keyword-case`, preserving the
canonical casing of all font names and keeping the suppression limited to these
declarations.
Source: Linters/SAST tools
| --adm-shadow-sm: 0 1px 3px rgba(0, 0, 0, 0.45), 0 1px 2px rgba(0, 0, 0, 0.3); | ||
| --adm-shadow-md: 0 8px 24px rgba(0, 0, 0, 0.5), 0 2px 8px rgba(0, 0, 0, 0.35); | ||
| --adm-shadow-lg: 0 28px 60px rgba(0, 0, 0, 0.62), 0 10px 20px rgba(0, 0, 0, 0.4); | ||
| color-scheme: dark; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stylelint declaration-empty-line-before failure.
Missing blank line before color-scheme: dark;.
🔧 Proposed fix
--adm-shadow-lg: 0 28px 60px rgba(0, 0, 0, 0.62), 0 10px 20px rgba(0, 0, 0, 0.4);
+
color-scheme: dark;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --adm-shadow-sm: 0 1px 3px rgba(0, 0, 0, 0.45), 0 1px 2px rgba(0, 0, 0, 0.3); | |
| --adm-shadow-md: 0 8px 24px rgba(0, 0, 0, 0.5), 0 2px 8px rgba(0, 0, 0, 0.35); | |
| --adm-shadow-lg: 0 28px 60px rgba(0, 0, 0, 0.62), 0 10px 20px rgba(0, 0, 0, 0.4); | |
| color-scheme: dark; | |
| } | |
| --adm-shadow-sm: 0 1px 3px rgba(0, 0, 0, 0.45), 0 1px 2px rgba(0, 0, 0, 0.3); | |
| --adm-shadow-md: 0 8px 24px rgba(0, 0, 0, 0.5), 0 2px 8px rgba(0, 0, 0, 0.35); | |
| --adm-shadow-lg: 0 28px 60px rgba(0, 0, 0, 0.62), 0 10px 20px rgba(0, 0, 0, 0.4); | |
| color-scheme: dark; | |
| } |
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 82-82: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/admin-svelte/src/tokens.css` around lines 79 - 83, Insert a blank
line before the color-scheme declaration in the token block containing
--adm-shadow-sm, --adm-shadow-md, and --adm-shadow-lg, satisfying the
declaration-empty-line-before Stylelint rule without changing the declaration
values.
Source: Linters/SAST tools
2 participants
Adds the OSS accessibility audit console + its self-contained vendored kit (@ariada-org/admin-surface + @ariada-org/admin-svelte). SvelteKit static build; live demo at app.ariada.org. First project = GNU Taler ecosystem. New-package publication.
Summary by CodeRabbit
New Features
Documentation
Tests