Conversation
* feat(cli): add session convert command * feat(web-ui): add session convert action * feat(session): persist derived conversions * test(e2e): gate session derived conversion * test(e2e): expand session conversion fixtures * feat(sessions): add claude resume command copy * feat(sessions): support gemini resume command copy * test(e2e): cover claude/gemini resume commands * test(e2e): guard derived content correctness * fix(sessions): place derived codex sessions in codex store --------- Co-authored-by: ymkiux <ymkiux@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR introduces an SVN log browser plugin with WebDAV support, renames the prompt-templates metadata export for naming consistency, adds dynamic plugin navigation, and updates translations. The SVN feature includes backend server endpoints, frontend UI panels, library utilities for WebDAV operations, and comprehensive E2E tests. ChangesPrompt-Templates Metadata Renaming
SVN Log Browser Plugin & Web UI Integration
Minor Updates
Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI as Web UI
participant CLI as CLI API
participant SVNLib as SVN Library
participant WebDAV as SVN WebDAV Server
participant AppServer as Plugin Server
User->>WebUI: Enter SVN URL & load
WebUI->>WebUI: Validate & normalize URL
WebUI->>CLI: POST /api svn-info
CLI->>SVNLib: getSvnInfo(url, creds)
SVNLib->>WebDAV: PROPFIND (discover repo)
WebDAV-->>SVNLib: multistatus XML
SVNLib->>SVNLib: Parse & cache metadata
SVNLib-->>CLI: {repositoryRoot, relativePath, cached}
CLI-->>WebUI: info payload
WebUI->>WebUI: Display repo metadata & cache status
User->>WebUI: Fetch logs (page 1)
WebUI->>CLI: POST /api svn-logs (page, pageSize)
CLI->>SVNLib: getSvnLogs(url, creds, page, pageSize)
SVNLib->>SVNLib: Check cache (hit or miss)
alt Cache miss or insufficient
SVNLib->>WebDAV: REPORT (log-report XML)
WebDAV-->>SVNLib: log-items XML
SVNLib->>SVNLib: Parse & normalize items
SVNLib->>SVNLib: Filter by path & cache
end
SVNLib->>SVNLib: Slice paged results
SVNLib-->>CLI: {items, hasMore, cached}
CLI-->>WebUI: logs payload
WebUI->>WebUI: Apply keyword filter & render
WebUI->>User: Display paginated log list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.9.0)web-ui/styles/sessions-usage.cssJSONError: JSON Error in /package.json: web-ui/styles/plugins-panel.cssJSONError: JSON Error in /package.json: Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 7
🧹 Nitpick comments (2)
tools/dev/generate-plugins-registry.js (1)
52-55: ⚡ Quick winValidate the manifest export name at generation time to catch convention violations early.
The generator blindly assumes every
manifest.mjsexports a symbol named${camel}PluginMeta. When that assumption is violated, the generatedregistry.mjswill contain a broken static import, which Node.js will surface as aSyntaxErroronly at module load time — not during generation. A simple content check at generation time makes the failure both earlier and more actionable.♻️ Proposed addition to
isPluginFolderor inline in the loopfunction isPluginFolder(name) { const manifestPath = path.join(pluginsDir, name, 'manifest.mjs'); const overviewPath = path.join(pluginsDir, name, 'overview.mjs'); return fs.existsSync(manifestPath) && fs.existsSync(overviewPath); } +function validateManifestExport(folder, expectedExport) { + const manifestPath = path.join(pluginsDir, folder, 'manifest.mjs'); + const src = fs.readFileSync(manifestPath, 'utf8'); + if (!src.includes(`export const ${expectedExport}`)) { + throw new Error( + `Plugin "${folder}": manifest.mjs must export \`${expectedExport}\` ` + + `(convention: <camelCase(folder)>PluginMeta). Got:\n${src.slice(0, 300)}` + ); + } +} function generatePluginsRegistrySource() { const folders = listPluginFolders().filter((name) => isPluginFolder(name)); const imports = []; const entries = []; for (const folder of folders) { const camel = toCamelCase(folder); const pascal = toPascalCase(folder); const manifestExport = `${camel}PluginMeta`; + validateManifestExport(folder, manifestExport); const metaName = `${camel}Meta`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/dev/generate-plugins-registry.js` around lines 52 - 55, The generator currently assumes each manifest.mjs exports `${manifestExport}` which leads to broken static imports in registry.mjs; update isPluginFolder (or the plugin loop in generate-plugins-registry.js) to read the manifest.mjs file content for each plugin and validate that the expected export symbol (the value of manifestExport, e.g., `${camel}PluginMeta`) is actually exported before adding the static import to the imports array; if the export is missing, throw a clear generation-time error mentioning the plugin folder and expected symbol so registry.mjs is never written with an invalid import.plugins/svn-log-browser/test/e2e.mjs (1)
155-183: ⚡ Quick winAdd one regression case for HTML escaping.
This test suite already covers the happy path well, but it doesn't protect the UI against malicious
repositoryRoot/author/datevalues from the mock SVN server. A single case with markup-like payloads would catch the DOM XSS path inplugins/svn-log-browser/public/index.htmlbefore it ships again.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/svn-log-browser/test/e2e.mjs` around lines 155 - 183, Add a regression test that submits mock SVN payloads containing markup-like strings to ensure UI escaping: call the existing endpoints used in this file (postJson to '/api/svn/info' and '/api/svn/logs') with a repo whose repositoryRoot/author/date fields contain HTML/JS-like payloads, then assert the returned page text and API data (via getText and the logs responses like firstInfo, logsPage1, brokenLogs) do not contain unescaped markup (e.g., assert the rendered page or the JSON fields show escaped characters or lack raw "<" ">" that would lead to DOM XSS). Use the same test helpers (getText, postJson) and the existing variables (svnBase/svnBroken) to add one focused case after the existing happy-path checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/svn-log-browser/config.js`:
- Around line 1-5: Remove the committed credential-bearing config.js and switch
to environment variables: delete the file that exports
maxxApiUrl/maxxApiKey/maxxModel and update any places that do
require('./config.js') in server.js to read process.env.MAXX_API_URL,
process.env.MAXX_API_KEY, and process.env.MAXX_MODEL (provide sensible fallbacks
or throw on missing secrets). Also add a .env.example or README entries showing
the MAXX_* env var names for developers to follow, and ensure no secret values
are checked into git.
In `@plugins/svn-log-browser/public/index.html`:
- Around line 563-568: The code sets el('info').innerHTML using
server-controlled fields (repositoryRoot, relativePath, baselineCollection,
author, date) which are not escaped; fix by never injecting raw HTML for these
values—either escape them properly before concatenation or (preferably) build
the DOM using text nodes or set textContent for each field instead of innerHTML;
update the places assigning el('info').innerHTML (and the similar block around
lines 583-588) to use safe rendering of
repositoryRoot/relativePath/baselineCollection/author/date and keep message and
path behavior consistent with their existing escaping.
In `@plugins/svn-log-browser/server.js`:
- Around line 351-352: The code computes requestedLimit as page * pageSize
(using pickPaging) which can produce extremely large REPORT limits; introduce a
MAX_TOTAL_FETCH (e.g., MAX_TOTAL_LIMIT) constant and replace requestedLimit
calculation with requestedLimit = Math.min(page * pageSize, MAX_TOTAL_FETCH) so
total items fetched across pages never exceeds the global cap; apply the same
change where requestedLimit is computed in the other block (lines 376-383) so
both code paths use the global cap and avoid huge XML REPORT responses and
memory spikes.
- Around line 207-209: The REPORT XML builder is inserting relativePath raw into
lines via lines.push(` <S:path>/${relativePath.replace(/^\/+/, '')}</S:path>`),
which breaks XML when the path contains &, <, >, etc.; fix by XML-escaping the
sanitized path before embedding it (e.g., call the existing HTML/xml escape
utility used by the UI or add an xmlEscape that replaces &, <, >, " and '), then
use that escaped value in the lines.push call so the <S:path> element is always
valid XML.
- Around line 422-425: The MAXX proxy endpoints (e.g., app.post('/api/ai/chat')
and the other AI endpoints around the same file) currently forward requests to
the upstream MAXX API whenever MAXX_API_* is set, which lets any remote caller
consume the configured API key; add an authentication or access restriction
before proxying: locate the handlers for '/api/ai/chat' and the other AI
endpoints mentioned and validate an auth token/session or enforce request origin
(e.g., only allow from localhost or require a validated API key/header) and
return 401/403 if unauthorized; ensure this check runs before calling
loadMaxxConfig() or before sending messages upstream so the configured MAXX API
key cannot be used by unauthenticated callers.
- Around line 305-306: The global call to app.use(cors()) exposes the SVN proxy
to SSRF; remove or disable global CORS and instead enable CORS only for the UI
origins or specific routes that need it (replace app.use(cors()) with
origin-restricted middleware or route-level cors(...) calls). Additionally,
harden handlers that accept arbitrary url parameters by validating/whitelisting
target hosts and rejecting private/loopback IPs and hostnames (the proxy route
handlers that read the incoming url param or body should perform this check
before fetching). Ensure bodyParser.json(...) stays but do not rely on it for
security; centralize URL validation logic used by the proxy routes so only
allowed external hosts are reachable.
- Around line 32-34: The cache currently embeds plaintext passwords in
buildCacheKey and never removes expired entries (only checks expiresAt), causing
long-term retention of credentials; change buildCacheKey to exclude the raw
password (use url + username only or a non-reversible hash of the password),
move the password into the cached value (or store a transient credential token)
and ensure you delete entries when they expire by adding a cleanup/purge routine
or actively removing the map entry when expiresAt is reached; update any logic
that reads the cache (the code using expiresAt) to drop and delete expired items
instead of keeping them, and if you keep any password-derived data store only a
one-way hash or ephemeral token and zero-out/overwrite plaintext on eviction
(refer to buildCacheKey and the expiresAt-handling code paths).
---
Nitpick comments:
In `@plugins/svn-log-browser/test/e2e.mjs`:
- Around line 155-183: Add a regression test that submits mock SVN payloads
containing markup-like strings to ensure UI escaping: call the existing
endpoints used in this file (postJson to '/api/svn/info' and '/api/svn/logs')
with a repo whose repositoryRoot/author/date fields contain HTML/JS-like
payloads, then assert the returned page text and API data (via getText and the
logs responses like firstInfo, logsPage1, brokenLogs) do not contain unescaped
markup (e.g., assert the rendered page or the JSON fields show escaped
characters or lack raw "<" ">" that would lead to DOM XSS). Use the same test
helpers (getText, postJson) and the existing variables (svnBase/svnBroken) to
add one focused case after the existing happy-path checks.
In `@tools/dev/generate-plugins-registry.js`:
- Around line 52-55: The generator currently assumes each manifest.mjs exports
`${manifestExport}` which leads to broken static imports in registry.mjs; update
isPluginFolder (or the plugin loop in generate-plugins-registry.js) to read the
manifest.mjs file content for each plugin and validate that the expected export
symbol (the value of manifestExport, e.g., `${camel}PluginMeta`) is actually
exported before adding the static import to the imports array; if the export is
missing, throw a clear generation-time error mentioning the plugin folder and
expected symbol so registry.mjs is never written with an invalid import.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be275e57-debe-4447-8dc1-8b08609b4ac3
📒 Files selected for processing (13)
plugins/prompt-templates/index.mjsplugins/prompt-templates/manifest.mjsplugins/registry.mjsplugins/svn-log-browser/config.jsplugins/svn-log-browser/index.mjsplugins/svn-log-browser/manifest.mjsplugins/svn-log-browser/overview.mjsplugins/svn-log-browser/package.jsonplugins/svn-log-browser/public/index.htmlplugins/svn-log-browser/server.jsplugins/svn-log-browser/test/e2e.mjstools/dev/generate-plugins-registry.jsweb-ui/styles/sessions-usage.css
📜 Review details
🔇 Additional comments (7)
web-ui/styles/sessions-usage.css (1)
490-491: No action needed — CSS change is internally consistent with no JS impact.The CSS cascade is correct: all consumers of
--usage-heat-celland--usage-heat-gapusevar(), and--usage-heat-weekauto-derives to12px. A search of the JavaScript heatmap files (web-ui/logic.sessions.mjs,web-ui/modules/app.computed.session.mjs) found no hardcoded pixel constants (11, 3, 14) related to heatmap cell, gap, or week dimensions. The change is safe.plugins/svn-log-browser/overview.mjs (1)
1-3: LGTM — intentional stub for the prototype.
loadSvnLogBrowserOverviewreturningtrueis consistent with thestatusLabel: 'prototype'in the manifest. No concerns.plugins/svn-log-browser/manifest.mjs (1)
1-13: LGTM.Naming convention (
svnLogBrowserPluginMeta) aligns with what the code generator expects. The emptycreatedBy/maintainersis reasonable for a prototype.plugins/svn-log-browser/index.mjs (1)
1-2: LGTM — clean entry-point re-exports matching the established pattern.plugins/prompt-templates/manifest.mjs (1)
3-15: LGTM — rename is clean and consistent with the new convention.plugins/prompt-templates/index.mjs (1)
3-3: LGTM — export rename correctly tracks the manifest change.plugins/registry.mjs (1)
1-9: LGTM — imports and registry entries are consistent with manifest exports and generator output.
| module.exports = { | ||
| maxxApiUrl: '', | ||
| maxxApiKey: '', | ||
| maxxModel: '' | ||
| }; |
There was a problem hiding this comment.
Committed config file with an API-key field is a credential-leak risk.
maxxApiKey living in a committed config.js as an editable field tempts developers to put a real secret here and commit it. Even though the field is currently empty, the pattern bypasses env-var hygiene and there is no warning comment or .gitignore guard in place.
Preferred fix: remove this file and read the env vars directly in server.js:
🛡️ Preferred: eliminate config.js, use env vars directly in server.js
In server.js, replace require('./config.js') references with direct process.env reads:
-const config = require('./config.js');
-const apiUrl = process.env.MAXX_API_URL || config.maxxApiUrl;
-const apiKey = process.env.MAXX_API_KEY || config.maxxApiKey;
-const model = process.env.MAXX_MODEL || config.maxxModel;
+const apiUrl = process.env.MAXX_API_URL || '';
+const apiKey = process.env.MAXX_API_KEY || '';
+const model = process.env.MAXX_MODEL || '';🛡️ Minimal: add a warning comment to config.js
+// DO NOT set credentials here. Configure via environment variables:
+// MAXX_API_URL, MAXX_API_KEY, MAXX_MODEL
+// This file provides shape/defaults only and is committed to version control.
module.exports = {
maxxApiUrl: '',
maxxApiKey: '',
maxxModel: ''
};📝 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.
| module.exports = { | |
| maxxApiUrl: '', | |
| maxxApiKey: '', | |
| maxxModel: '' | |
| }; | |
| // DO NOT set credentials here. Configure via environment variables: | |
| // MAXX_API_URL, MAXX_API_KEY, MAXX_MODEL | |
| // This file provides shape/defaults only and is committed to version control. | |
| module.exports = { | |
| maxxApiUrl: '', | |
| maxxApiKey: '', | |
| maxxModel: '' | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/svn-log-browser/config.js` around lines 1 - 5, Remove the committed
credential-bearing config.js and switch to environment variables: delete the
file that exports maxxApiUrl/maxxApiKey/maxxModel and update any places that do
require('./config.js') in server.js to read process.env.MAXX_API_URL,
process.env.MAXX_API_KEY, and process.env.MAXX_MODEL (provide sensible fallbacks
or throw on missing secrets). Also add a .env.example or README entries showing
the MAXX_* env var names for developers to follow, and ensure no secret values
are checked into git.
| el('info').innerHTML = [ | ||
| `<div><strong>repositoryRoot</strong>：${info.repositoryRoot || '-'}</div>`, | ||
| `<div><strong>relativePath</strong>：${info.relativePath || '-'}</div>`, | ||
| `<div><strong>revision</strong>：${info.revision || '-'}</div>`, | ||
| `<div><strong>baselineCollection</strong>：${info.baselineCollection || '-'}</div>` | ||
| ].join(''); |
There was a problem hiding this comment.
Escape all server-controlled fields before writing innerHTML.
message and path are escaped, but repositoryRoot, relativePath, baselineCollection, author, and date are not. Those values come from the backend's SVN responses, so a malicious repository can inject markup/script into this page.
Suggested fix
function renderInfo(info, cached) {
el('cache-pill').textContent = cached ? '缓存：命中' : '缓存：未命中';
if (!info) {
el('info').innerHTML = '<div><strong>repositoryRoot</strong>：-</div><div><strong>relativePath</strong>：-</div><div><strong>revision</strong>：-</div><div><strong>baselineCollection</strong>：-</div>';
return;
}
el('info').innerHTML = [
- `<div><strong>repositoryRoot</strong>：${info.repositoryRoot || '-'}</div>`,
- `<div><strong>relativePath</strong>：${info.relativePath || '-'}</div>`,
- `<div><strong>revision</strong>：${info.revision || '-'}</div>`,
- `<div><strong>baselineCollection</strong>：${info.baselineCollection || '-'}</div>`
+ `<div><strong>repositoryRoot</strong>：${escapeHtml(info.repositoryRoot || '-')}</div>`,
+ `<div><strong>relativePath</strong>：${escapeHtml(info.relativePath || '-')}</div>`,
+ `<div><strong>revision</strong>：${escapeHtml(info.revision || '-')}</div>`,
+ `<div><strong>baselineCollection</strong>：${escapeHtml(info.baselineCollection || '-')}</div>`
].join('');
}
...
- ` <span class="author" title="${author}"><i class="ri-user-3-line"></i>${author}</span>`,
- ` <span title="${date}"><i class="ri-time-line"></i>${date}</span>`,
+ ` <span class="author" title="${escapeHtml(author)}"><i class="ri-user-3-line"></i>${escapeHtml(author)}</span>`,
+ ` <span title="${escapeHtml(date)}"><i class="ri-time-line"></i>${escapeHtml(date)}</span>`,Also applies to: 583-588
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/svn-log-browser/public/index.html` around lines 563 - 568, The code
sets el('info').innerHTML using server-controlled fields (repositoryRoot,
relativePath, baselineCollection, author, date) which are not escaped; fix by
never injecting raw HTML for these values—either escape them properly before
concatenation or (preferably) build the DOM using text nodes or set textContent
for each field instead of innerHTML; update the places assigning
el('info').innerHTML (and the similar block around lines 583-588) to use safe
rendering of repositoryRoot/relativePath/baselineCollection/author/date and keep
message and path behavior consistent with their existing escaping.
| function buildCacheKey(url, username, password) { | ||
| return `${url}\n${username || ''}\n${password || ''}`; | ||
| } |
There was a problem hiding this comment.
The cache keeps raw credentials longer than the advertised TTL.
expiresAt only stops reuse; expired entries are never removed, and the cache key itself includes the plaintext password. In a long-lived process this will accumulate old credentials and repo metadata indefinitely instead of for ~60 seconds.
Also applies to: 302-303, 330-335, 397-402
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/svn-log-browser/server.js` around lines 32 - 34, The cache currently
embeds plaintext passwords in buildCacheKey and never removes expired entries
(only checks expiresAt), causing long-term retention of credentials; change
buildCacheKey to exclude the raw password (use url + username only or a
non-reversible hash of the password), move the password into the cached value
(or store a transient credential token) and ensure you delete entries when they
expire by adding a cleanup/purge routine or actively removing the map entry when
expiresAt is reached; update any logic that reads the cache (the code using
expiresAt) to drop and delete expired items instead of keeping them, and if you
keep any password-derived data store only a one-way hash or ephemeral token and
zero-out/overwrite plaintext on eviction (refer to buildCacheKey and the
expiresAt-handling code paths).
| if (relativePath) { | ||
| lines.push(` <S:path>/${relativePath.replace(/^\/+/, '')}</S:path>`); | ||
| } |
There was a problem hiding this comment.
Escape relativePath before embedding it into the REPORT XML.
If the SVN path contains &, <, or >, the generated XML becomes invalid and log loading fails for that branch. This should be XML-escaped the same way the UI already escapes HTML output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/svn-log-browser/server.js` around lines 207 - 209, The REPORT XML
builder is inserting relativePath raw into lines via lines.push(`
<S:path>/${relativePath.replace(/^\/+/, '')}</S:path>`), which breaks XML when
the path contains &, <, >, etc.; fix by XML-escaping the sanitized path before
embedding it (e.g., call the existing HTML/xml escape utility used by the UI or
add an xmlEscape that replaces &, <, >, " and '), then use that escaped value in
the lines.push call so the <S:path> element is always valid XML.
| app.use(cors()); | ||
| app.use(bodyParser.json({ limit: '2mb' })); |
There was a problem hiding this comment.
Lock down cross-origin access to the SVN proxy.
These routes fetch arbitrary url values, and with global cors() plus a 0.0.0.0 bind they can be driven by other sites as an SSRF primitive against internal hosts. The UI is already served from the same app, so CORS should be off by default here, and private/loopback targets should be explicitly gated if you need them.
Also applies to: 309-310, 345-346, 464-467
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/svn-log-browser/server.js` around lines 305 - 306, The global call to
app.use(cors()) exposes the SVN proxy to SSRF; remove or disable global CORS and
instead enable CORS only for the UI origins or specific routes that need it
(replace app.use(cors()) with origin-restricted middleware or route-level
cors(...) calls). Additionally, harden handlers that accept arbitrary url
parameters by validating/whitelisting target hosts and rejecting
private/loopback IPs and hostnames (the proxy route handlers that read the
incoming url param or body should perform this check before fetching). Ensure
bodyParser.json(...) stays but do not rely on it for security; centralize URL
validation logic used by the proxy routes so only allowed external hosts are
reachable.
| const { page, pageSize } = pickPaging(req.body || {}); | ||
| const requestedLimit = page * pageSize; |
There was a problem hiding this comment.
Cap the total log fetch size independently of the page size.
A request like page=10000&pageSize=200 turns into S:limit=2000000, which can force a huge REPORT response and a large in-memory XML parse in a single call. The per-page clamp helps, but the product still needs its own upper bound.
Also applies to: 376-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/svn-log-browser/server.js` around lines 351 - 352, The code computes
requestedLimit as page * pageSize (using pickPaging) which can produce extremely
large REPORT limits; introduce a MAX_TOTAL_FETCH (e.g., MAX_TOTAL_LIMIT)
constant and replace requestedLimit calculation with requestedLimit =
Math.min(page * pageSize, MAX_TOTAL_FETCH) so total items fetched across pages
never exceeds the global cap; apply the same change where requestedLimit is
computed in the other block (lines 376-383) so both code paths use the global
cap and avoid huge XML REPORT responses and memory spikes.
| app.post('/api/ai/chat', async (req, res) => { | ||
| const { maxxApiUrl, maxxApiKey, maxxModel } = loadMaxxConfig(); | ||
| if (!maxxApiUrl || !maxxApiKey) { | ||
| return res.status(400).json({ error: 'missing maxx config' }); |
There was a problem hiding this comment.
Protect the MAXX proxy before enabling it.
Once MAXX_API_* is configured, any caller that can reach this server can spend that API key by posting arbitrary messages. This needs an auth check or localhost-only exposure; otherwise it becomes an open relay for the upstream model.
Also applies to: 432-442, 464-467
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/svn-log-browser/server.js` around lines 422 - 425, The MAXX proxy
endpoints (e.g., app.post('/api/ai/chat') and the other AI endpoints around the
same file) currently forward requests to the upstream MAXX API whenever
MAXX_API_* is set, which lets any remote caller consume the configured API key;
add an authentication or access restriction before proxying: locate the handlers
for '/api/ai/chat' and the other AI endpoints mentioned and validate an auth
token/session or enforce request origin (e.g., only allow from localhost or
require a validated API key/header) and return 401/403 if unauthorized; ensure
this check runs before calling loadMaxxConfig() or before sending messages
upstream so the configured MAXX API key cannot be used by unauthenticated
callers.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/prompt-templates/methods.mjs`:
- Around line 93-104: The onPluginsToolClick flow can leave pluginsRequestedId
set when a pointer-based navigation already consumed and cleared it, so after
calling the click handler you must clear any stale value; specifically, inside
onPluginsToolClick when typeof this.onMainTabClick === 'function' call
this.onMainTabClick('plugins') as before and then immediately set
this.pluginsRequestedId = '' (or otherwise clear it) so a no-op/consumed
pointer-commit does not leave a stale pluginsRequestedId; keep the same behavior
for the switchMainTab branch (do not clear before calling switchMainTab).
In `@tests/e2e/test-plugin-svn-log-browser.js`:
- Around line 52-63: Replace the hardcoded port assignment and avoid racey
readiness checks: acquire an ephemeral free port before spawning the server
(e.g., create a temporary net.Server listening on port 0 to get
server.address().port, or use a utility like get-port) and assign that value to
the variable used in the spawn env (the current port const and env PORT), then
spawn the child (spawn(..., env: { ..., PORT: String(port) })) and call
waitForHttpServer(port) as before; this ensures waitForHttpServer checks the
actual ephemeral port owned by our spawned process rather than a possibly
preoccupied fixed port.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04fa2a81-3148-4001-a24b-a28b23a4fe00
⛔ Files ignored due to path filters (1)
plugins/svn-log-browser/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
plugins/prompt-templates/methods.mjsplugins/svn-log-browser/overview.mjstests/e2e/run.jstests/e2e/test-plugin-svn-log-browser.jstests/e2e/test-web-ui-assets.jstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-plugins.htmlweb-ui/session-helpers.mjsweb-ui/styles/plugins-panel.css
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.42.1)
web-ui/partials/index/panel-plugins.html
[warning] 251-251: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context: {{ t('plugins.svnLogBrowser.launch.open') }}
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html
(plaintext-http-link-html)
🔇 Additional comments (3)
tests/e2e/test-web-ui-assets.js (1)
43-43: LGTM – new assertions are well-formed and consistent with the existing test style.All three new assertions mirror the pattern used throughout the file and their string literals match the DOM/bundle content added by this PR.
Also applies to: 60-64, 94-94
plugins/svn-log-browser/overview.mjs (1)
1-11: LGTM – logic is correct and the$nextTickguard makes browser context implicit.The defensive normalization of
ctxandoptions, the compound guard onmainTab/pluginsActiveId/$nextTick, and the matching DOM selector (#panel-plugins .svn-log-browser-launch .btn-mini) all align with the surrounding template structure. The unconditionalreturn trueis consistent with how other overview loaders are consumed byloadPluginsOverview.web-ui/session-helpers.mjs (1)
191-199: LGTM – plugin resolution logic is correct.Capturing
requestedbefore clearingpluginsRequestedIdavoids TOCTOU, thegetPluginEntryguard correctly falls back to'prompt-templates'for unknown IDs, andpromptTemplatesMode = 'compose'is now conditionally set only for the prompt-templates target — preserving prior behavior for the default case while leaving other plugins unaffected.
| onPluginsToolClick(pluginId) { | ||
| const id = typeof pluginId === 'string' ? pluginId.trim() : ''; | ||
| if (!id) return; | ||
| this.pluginsRequestedId = id; | ||
| if (typeof this.onMainTabClick === 'function') { | ||
| this.onMainTabClick('plugins'); | ||
| return; | ||
| } | ||
| if (typeof this.switchMainTab === 'function') { | ||
| this.switchMainTab('plugins'); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Stale pluginsRequestedId after a pointer-commit-consumed click can override plugin selection on the next tab entry.
The sequence for a normal mouse click is:
pointerdown→onPluginsToolPointerDownsetspluginsRequestedId = id, callsonMainTabPointerDown→switchMainTabruns →pluginsRequestedIdis cleared insession-helpers.mjs.click→onPluginsToolClickunconditionally re-setspluginsRequestedId = id, then callsonMainTabClick→consumePointerNavCommitreturnstrue→switchMainTabis skipped →pluginsRequestedIdis now stale.
Any subsequent entry into the plugins tab (e.g., the user manually navigates away, changes plugins within the panel, then returns via the main-tab button) will consume this stale value and force the panel back to the shortcut-clicked plugin, ignoring the user's deliberate in-panel selection.
The fix is to clear pluginsRequestedId after onMainTabClick returns, when navigation was no-oped (detectable because switchMainTab would have already cleared the value):
🛡️ Proposed fix
onPluginsToolClick(pluginId) {
const id = typeof pluginId === 'string' ? pluginId.trim() : '';
if (!id) return;
this.pluginsRequestedId = id;
if (typeof this.onMainTabClick === 'function') {
this.onMainTabClick('plugins');
+ // If onMainTabClick no-oped (pointer nav commit consumed), the value set
+ // above is stale. switchMainTab would have already cleared it on a real
+ // navigation, so if it is still set we know navigation did not occur.
+ if (this.pluginsRequestedId === id) {
+ this.pluginsRequestedId = '';
+ }
return;
}
if (typeof this.switchMainTab === 'function') {
this.switchMainTab('plugins');
}
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/prompt-templates/methods.mjs` around lines 93 - 104, The
onPluginsToolClick flow can leave pluginsRequestedId set when a pointer-based
navigation already consumed and cleared it, so after calling the click handler
you must clear any stale value; specifically, inside onPluginsToolClick when
typeof this.onMainTabClick === 'function' call this.onMainTabClick('plugins') as
before and then immediately set this.pluginsRequestedId = '' (or otherwise clear
it) so a no-op/consumed pointer-commit does not leave a stale
pluginsRequestedId; keep the same behavior for the switchMainTab branch (do not
clear before calling switchMainTab).
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
plugins/prompt-templates/methods.mjs (1)
96-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear stale
pluginsRequestedIdafter click-path no-op.Line 100–103 can leave a stale requested plugin when
onMainTabClick('plugins')is consumed/no-op, causing later tab entries to jump unexpectedly.🔧 Suggested patch
onPluginsToolClick(pluginId) { const id = typeof pluginId === 'string' ? pluginId.trim() : ''; if (!id) return; this.pluginsRequestedId = id; if (typeof this.onMainTabClick === 'function') { this.onMainTabClick('plugins'); + if (this.pluginsRequestedId === id) { + this.pluginsRequestedId = ''; + } return; } if (typeof this.switchMainTab === 'function') { this.switchMainTab('plugins'); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/prompt-templates/methods.mjs` around lines 96 - 103, In onPluginsToolClick, calling this.onMainTabClick('plugins') can be a no-op and leave this.pluginsRequestedId stale; update the handler so that after detecting typeof this.onMainTabClick === 'function' you call this.onMainTabClick('plugins') and then clear this.pluginsRequestedId (e.g., set it to '' or null) before returning to avoid leaving a stale requested id that can cause later tab jumps.
🧹 Nitpick comments (1)
tests/e2e/test-web-ui-assets.js (1)
43-43: ⚡ Quick winPrefer stable asset markers over source-string assertions.
These checks are validating implementation details rather than the bundled behavior. Line 61 is the most brittle: a harmless template refactor can remove that exact expression without breaking the plugin. Please assert on a stable panel token from the emitted HTML/JS instead of raw source snippets.
Also applies to: 60-63, 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/test-web-ui-assets.js` at line 43, Replace brittle raw-string assertions in tests/e2e/test-web-ui-assets.js (the assert using rootPage.body.includes('svn-log-browser')) with checks for a stable emitted marker from the built bundle (e.g., a data attribute or container id/class that the bundler/plugin emits such as data-plugin-id="svn-log-browser" or an element id/class like "plugin-svn-log-browser" or the canonical panel token used by the UI). Locate the assertion around rootPage.body.includes and change it to parse the emitted HTML/JS for that stable token (use the same approach for the other occurrences at the nearby assertions on lines 60-63 and 93) so tests validate bundled output instead of internal template text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/svn-webdav.js`:
- Around line 355-356: The code currently applies normalizeText to both
params.username and params.password which trims surrounding whitespace and can
corrupt valid passwords; in getSvnInfo and getSvnLogs remove the normalizeText
call for params.password and use the raw params.password (i.e., leave the
password untrimmed) while keeping normalizeText for params.username if desired,
updating the assignments around username/password (references: normalizeText,
params.username, params.password, getSvnInfo, getSvnLogs).
- Around line 30-31: The current parsed.protocol check allows plain http even
when credentials are present; update the logic wherever that check appears (the
parsed.protocol !== 'http:' && parsed.protocol !== 'https:' block and the two
other occurrences referenced, and mirror the same behavior as getSvnLogs) so
that if parsed.protocol === 'http:' and username/password are supplied the
function rejects the URL (return '' or throw) unless an explicit insecure
override flag is set (use the existing config flag if one exists or add a clear
flag like ALLOW_INSECURE_HTTP), i.e., check for credentials (username/password)
together with parsed.protocol==='http:' and require either 'https:' or the
override before returning parsed.toString(); apply this change to all three
locations mentioned to prevent credential leakage.
- Line 375: The current calculation requestedLimit = page * pageSize can produce
a false positive at exact page boundaries; update the logic so requestedLimit =
(page + 1) * pageSize and use a strict greater-than check (logs.length >
requestedLimit) when computing hasMore (referencing requestedLimit, page,
pageSize and the logs.length check) to ensure hasMore is true only when an extra
item exists; apply the same change at the second occurrence around the other
check.
In `@web-ui/modules/svn-log-browser.methods.mjs`:
- Around line 55-59: The code is persisting plaintext SVN passwords when
remember is true; stop storing passwords in browser storage. Change the logic so
remember only controls persistence of svnLogBrowserUrl and svnLogBrowserUsername
(use svnLogBrowserMethodsNormalizeText on those), but do NOT persist
svnLogBrowserPassword—assign prefs.password only to the in-memory
this.svnLogBrowserPassword (no normalization) outside or independent of the
remember/persistence path so it is never written to localStorage or saved with
prefs; update places that serialize prefs to ensure prefs.password is omitted.
In `@web-ui/partials/index/panel-plugins.html`:
- Around line 259-319: The form labels for the SVN plugin inputs are not
associated with their inputs; add unique id attributes to the relevant inputs
(e.g., the URL input referenced by ref="svnLogBrowserUrlInput", username,
password, page size input svnLogBrowserPageSizeInput, and path filter
svnLogBrowserPathFilter) and set the corresponding label's for attribute to
match each id (or alternatively wrap each input inside its label). Also ensure
the keyword/search box (the search control around the lines that render a
standalone search input) has an explicit visible label or an aria-label so
screen readers receive an accessible name; update any checkbox label tied to
svnLogBrowserRemember similarly if needed.
- Around line 347-355: Replace the hard-coded English labels "path" and "rev" in
the SVN log browser panel with the i18n translation calls used elsewhere (use
the t(...) helper), e.g. change the static text in the elements currently
rendering "path" and "rev" to t('svn.path') and t('svn.rev') (or your project's
existing keys) so the labels in the svn-log-browser-info-row render localized
strings; update the translation resource files with those keys if they don't
exist.
---
Duplicate comments:
In `@plugins/prompt-templates/methods.mjs`:
- Around line 96-103: In onPluginsToolClick, calling
this.onMainTabClick('plugins') can be a no-op and leave this.pluginsRequestedId
stale; update the handler so that after detecting typeof this.onMainTabClick ===
'function' you call this.onMainTabClick('plugins') and then clear
this.pluginsRequestedId (e.g., set it to '' or null) before returning to avoid
leaving a stale requested id that can cause later tab jumps.
---
Nitpick comments:
In `@tests/e2e/test-web-ui-assets.js`:
- Line 43: Replace brittle raw-string assertions in
tests/e2e/test-web-ui-assets.js (the assert using
rootPage.body.includes('svn-log-browser')) with checks for a stable emitted
marker from the built bundle (e.g., a data attribute or container id/class that
the bundler/plugin emits such as data-plugin-id="svn-log-browser" or an element
id/class like "plugin-svn-log-browser" or the canonical panel token used by the
UI). Locate the assertion around rootPage.body.includes and change it to parse
the emitted HTML/JS for that stable token (use the same approach for the other
occurrences at the nearby assertions on lines 60-63 and 93) so tests validate
bundled output instead of internal template text.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8036f589-7156-4cac-85ad-f4f19c000492
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
cli.jslib/svn-webdav.jspackage.jsonplugins/prompt-templates/computed.mjsplugins/prompt-templates/methods.mjsplugins/svn-log-browser/overview.mjstests/e2e/test-plugin-svn-log-browser.jstests/e2e/test-web-ui-assets.jstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/modules/svn-log-browser.computed.mjsweb-ui/modules/svn-log-browser.methods.mjsweb-ui/partials/index/panel-plugins.htmlweb-ui/styles/plugins-panel.css
✅ Files skipped from review due to trivial changes (1)
- web-ui/app.js
📜 Review details
🔇 Additional comments (3)
package.json (1)
48-48: Dependency addition looks appropriate.
xml2jsis correctly added as a runtime dependency for the new WebDAV XML parsing flow.cli.js (1)
78-81: SVN handler import wiring looks correct.
getSvnInfo/getSvnLogsare imported with the expected names and used consistently in the new action handlers.tests/e2e/test-plugin-svn-log-browser.js (1)
125-161: Solid e2e coverage for cache + pagination + fallback behavior.This validates the critical API paths and includes reliable cleanup in
finally. Nice improvement in confidence for the new plugin.
| if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return ''; | ||
| return parsed.toString(); |
There was a problem hiding this comment.
Block credentialed requests over plain HTTP by default.
When username/password are provided, allowing http: risks credential disclosure in transit. Require HTTPS (or an explicit insecure override flag).
Suggested fix
async function getSvnInfo(params) {
const url = normalizeUrlCandidate(params && params.url);
if (!url) {
return { error: 'invalid url' };
}
const username = normalizeText(params && params.username);
- const password = normalizeText(params && params.password);
+ const password = params && params.password != null ? String(params.password) : '';
+ if (username && new URL(url).protocol !== 'https:') {
+ return { error: 'https required when credentials are provided' };
+ }Mirror the same check in getSvnLogs.
Also applies to: 350-357, 367-374
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/svn-webdav.js` around lines 30 - 31, The current parsed.protocol check
allows plain http even when credentials are present; update the logic wherever
that check appears (the parsed.protocol !== 'http:' && parsed.protocol !==
'https:' block and the two other occurrences referenced, and mirror the same
behavior as getSvnLogs) so that if parsed.protocol === 'http:' and
username/password are supplied the function rejects the URL (return '' or throw)
unless an explicit insecure override flag is set (use the existing config flag
if one exists or add a clear flag like ALLOW_INSECURE_HTTP), i.e., check for
credentials (username/password) together with parsed.protocol==='http:' and
require either 'https:' or the override before returning parsed.toString();
apply this change to all three locations mentioned to prevent credential
leakage.
| const username = normalizeText(params && params.username); | ||
| const password = normalizeText(params && params.password); |
There was a problem hiding this comment.
Do not trim password credentials before auth.
normalizeText strips surrounding whitespace, so valid passwords can be altered before Basic auth and fail unexpectedly.
Suggested fix
- const password = normalizeText(params && params.password);
+ const password = params && params.password != null ? String(params.password) : '';Apply the same change in both getSvnInfo and getSvnLogs.
Also applies to: 372-373
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/svn-webdav.js` around lines 355 - 356, The code currently applies
normalizeText to both params.username and params.password which trims
surrounding whitespace and can corrupt valid passwords; in getSvnInfo and
getSvnLogs remove the normalizeText call for params.password and use the raw
params.password (i.e., leave the password untrimmed) while keeping normalizeText
for params.username if desired, updating the assignments around
username/password (references: normalizeText, params.username, params.password,
getSvnInfo, getSvnLogs).
| const username = normalizeText(params && params.username); | ||
| const password = normalizeText(params && params.password); | ||
| const { page, pageSize } = pickPaging(params || {}); | ||
| const requestedLimit = page * pageSize; |
There was a problem hiding this comment.
hasMore can be a false positive at exact page boundaries.
With requestedLimit = page * pageSize, logs.length >= requestedLimit returns true even when there is no next page (exactly N items).
Suggested fix
- const requestedLimit = page * pageSize;
+ const requestedLimit = page * pageSize + 1;
@@
- const hasMore = logs.length >= requestedLimit;
+ const hasMore = logs.length > startIndex + pageSize;Also applies to: 398-398
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/svn-webdav.js` at line 375, The current calculation requestedLimit = page
* pageSize can produce a false positive at exact page boundaries; update the
logic so requestedLimit = (page + 1) * pageSize and use a strict greater-than
check (logs.length > requestedLimit) when computing hasMore (referencing
requestedLimit, page, pageSize and the logs.length check) to ensure hasMore is
true only when an extra item exists; apply the same change at the second
occurrence around the other check.
| if (remember) { | ||
| this.svnLogBrowserUrl = svnLogBrowserMethodsNormalizeText(prefs.url); | ||
| this.svnLogBrowserUsername = svnLogBrowserMethodsNormalizeText(prefs.username); | ||
| this.svnLogBrowserPassword = typeof prefs.password === 'string' ? prefs.password : ''; | ||
| } |
There was a problem hiding this comment.
Avoid persisting SVN passwords in localStorage.
Line 71 stores plaintext credentials in browser storage, which is accessible to injected scripts/extensions. Keep “remember” for URL/username, but do not persist password.
🔧 Suggested patch
restoreSvnLogBrowserPrefs() {
const prefs = svnLogBrowserMethodsLoadPrefs(typeof localStorage !== 'undefined' ? localStorage : null);
if (!prefs) return false;
const remember = prefs.remember === true;
this.svnLogBrowserRemember = remember;
if (remember) {
this.svnLogBrowserUrl = svnLogBrowserMethodsNormalizeText(prefs.url);
this.svnLogBrowserUsername = svnLogBrowserMethodsNormalizeText(prefs.username);
- this.svnLogBrowserPassword = typeof prefs.password === 'string' ? prefs.password : '';
+ this.svnLogBrowserPassword = '';
}
this.svnLogBrowserPageSizeInput = svnLogBrowserMethodsNormalizePositiveIntegerInput(prefs.pageSize, 25);
this.svnLogBrowserPathFilter = svnLogBrowserMethodsNormalizeText(prefs.pathFilter);
return true;
},
@@
persistSvnLogBrowserPrefs() {
const remember = this.svnLogBrowserRemember === true;
const prefs = {
remember,
url: remember ? svnLogBrowserMethodsNormalizeText(this.svnLogBrowserUrl) : '',
username: remember ? svnLogBrowserMethodsNormalizeText(this.svnLogBrowserUsername) : '',
- password: remember ? (typeof this.svnLogBrowserPassword === 'string' ? this.svnLogBrowserPassword : '') : '',
+ password: '',
pageSize: svnLogBrowserMethodsNormalizePositiveIntegerInput(this.svnLogBrowserPageSizeInput, 25),
pathFilter: svnLogBrowserMethodsNormalizeText(this.svnLogBrowserPathFilter)
};
return svnLogBrowserMethodsSavePrefs(typeof localStorage !== 'undefined' ? localStorage : null, prefs);
},Also applies to: 67-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/modules/svn-log-browser.methods.mjs` around lines 55 - 59, The code is
persisting plaintext SVN passwords when remember is true; stop storing passwords
in browser storage. Change the logic so remember only controls persistence of
svnLogBrowserUrl and svnLogBrowserUsername (use
svnLogBrowserMethodsNormalizeText on those), but do NOT persist
svnLogBrowserPassword—assign prefs.password only to the in-memory
this.svnLogBrowserPassword (no normalization) outside or independent of the
remember/persistence path so it is never written to localStorage or saved with
prefs; update places that serialize prefs to ensure prefs.password is omitted.
ymkiux
force-pushed
the
feat/svn-log-browser
branch
from
93ede54 to
69fbd03
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli.js`:
- Around line 10010-10017: The hostname-only check allows DNS-based SSRF; update
the non-loopback guard around parsedUrl/rawUrl (where requesterIsLoopback is
used and isPrivateNetworkHost(parsedUrl.hostname) is called) to resolve the
hostname to IP addresses and re-check each resolved address with
isPrivateNetworkHost before allowing access: use the Node dns promises API
(e.g., dns.promises.lookup or dns.promises.resolve* with all: true) to get both
IPv4/IPv6 results, treat any resolved private/loopback address as forbidden,
handle lookup errors/timeouts conservatively (fail-closed or treat as
forbidden), and apply the same change to the other identical block at the later
occurrence.
In `@plugins/prompt-templates/methods.mjs`:
- Line 86: The SVN password is being persisted in localStorage by
createSvnLogBrowserMethods via svnLogBrowserMethodsSavePrefs; change that so
only non-sensitive prefs (url, username, pageSize, pathFilter,
svnLogBrowserRemember) are written to localStorage and the password is written
to sessionStorage when svnLogBrowserRemember === true (and removed from
sessionStorage when false); update svnLogBrowserMethodsLoadPrefs to read the
password from sessionStorage (falling back to no password) while loading other
prefs from localStorage, and ensure that when toggling remember off any stored
password in localStorage is removed/migrated to sessionStorage appropriately so
no plaintext password remains in localStorage.
In `@web-ui/partials/index/panel-svn.html`:
- Around line 107-113: Replace the hardcoded English labels "path" and "rev" in
the SVN info rows (the divs using class svn-log-browser-info-key) with calls to
the translation helper used elsewhere (e.g., t('svn.path') and t('svn.rev')) and
add matching keys ("svn.path" and "svn.rev") to the i18n dictionary in
web-ui/modules/i18n.dict.mjs alongside the other SVN entries so
svnLogBrowserInfo.relativePath and svnLogBrowserInfo.revision render with
localized labels.
- Around line 133-147: The container div with class "svn-log-browser-list" uses
role="list" but its child v-for items (divs with class "svn-log-browser-item"
generated by v-for over svnLogBrowserFilteredItems) lack role="listitem"; update
those child divs to include role="listitem" (and also add role="listitem" to
each path entry div with class "svn-log-browser-item-path" if you keep the inner
paths as a list) or replace the outer div with a semantic <ul> and change the
v-for item divs to <li> (and path container to nested <ul>/<li>) to follow
best-practice semantics and ensure screen readers can announce and navigate list
items.
- Around line 17-77: The labels in the SVN panel template are not associated
with their inputs; add matching id attributes to the inputs and for attributes
to the corresponding labels so clicking labels focuses the inputs (use ids like
svn-url, svn-username, svn-password, svn-page-size, svn-path-filter) and keep
existing bindings (ref="svnLogBrowserUrlInput", v-model="svnLogBrowserPassword",
v-model.trim="svnLogBrowserUsername", v-model.trim="svnLogBrowserPageSizeInput",
v-model.trim="svnLogBrowserPathFilter") so the label-for pairs map to the exact
inputs in the diff.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff233256-6689-4913-b882-f93080188a16
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonplugins/svn-log-browser/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
cli.jslib/svn-webdav.jspackage.jsonplugins/prompt-templates/computed.mjsplugins/prompt-templates/index.mjsplugins/prompt-templates/manifest.mjsplugins/prompt-templates/methods.mjsplugins/registry.mjsplugins/svn-log-browser/config.jsplugins/svn-log-browser/index.mjsplugins/svn-log-browser/manifest.mjsplugins/svn-log-browser/overview.mjsplugins/svn-log-browser/package.jsonplugins/svn-log-browser/public/index.htmlplugins/svn-log-browser/server.jsplugins/svn-log-browser/test/e2e.mjstests/e2e/run.jstests/e2e/test-plugin-svn-log-browser.jstests/e2e/test-web-ui-assets.jstests/unit/web-ui-behavior-parity.test.mjstools/dev/generate-plugins-registry.jsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/i18n.dict.mjsweb-ui/modules/svn-log-browser.computed.mjsweb-ui/modules/svn-log-browser.methods.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-svn.htmlweb-ui/session-helpers.mjsweb-ui/styles/plugins-panel.cssweb-ui/styles/sessions-usage.css
✅ Files skipped from review due to trivial changes (11)
- plugins/svn-log-browser/config.js
- plugins/svn-log-browser/index.mjs
- plugins/svn-log-browser/package.json
- web-ui/styles/sessions-usage.css
- web-ui/index.html
- web-ui/app.js
- web-ui/modules/app.methods.navigation.mjs
- package.json
- web-ui/modules/i18n.dict.mjs
- plugins/svn-log-browser/public/index.html
- tests/unit/web-ui-behavior-parity.test.mjs
🚧 Files skipped from review as they are similar to previous changes (11)
- plugins/svn-log-browser/overview.mjs
- tests/e2e/test-web-ui-assets.js
- plugins/prompt-templates/manifest.mjs
- plugins/svn-log-browser/manifest.mjs
- tests/e2e/run.js
- web-ui/modules/svn-log-browser.methods.mjs
- web-ui/styles/plugins-panel.css
- plugins/prompt-templates/computed.mjs
- lib/svn-webdav.js
- plugins/svn-log-browser/test/e2e.mjs
- plugins/svn-log-browser/server.js
📜 Review details
🧰 Additional context used
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-svn.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (11)
plugins/prompt-templates/methods.mjs (1)
96-107: StalepluginsRequestedIdafter a no-op click navigation — see prior review.The race between
onPluginsToolClicksettingpluginsRequestedIdandonMainTabClickconsuming (and skipping) the pointer-nav commit without callingswitchMainTabwas already flagged with a proposed fix.web-ui/modules/app.computed.main-tabs.mjs (1)
146-146: LGTM — SVN tab label wiring is correct and complete.All three computed branches are symmetric with the existing if-chain, and the three i18n keys (
kicker.svn,title.svn,subtitle.svn) are confirmed inweb-ui/modules/i18n.dict.mjs.Also applies to: 158-158, 170-170
plugins/prompt-templates/index.mjs (1)
3-3: LGTM — rename topromptTemplatesPluginMetais consistent across manifest, index, and registry.tools/dev/generate-plugins-registry.js (1)
52-57: LGTM — generated import alias pattern (${camel}PluginMeta as ${camel}Meta) matches the manually-writtenplugins/registry.mjsand is verified for bothprompt-templatesandsvn-log-browserfolder names.plugins/registry.mjs (1)
1-1: LGTM — import rename tracks the manifest export change cleanly.web-ui/modules/svn-log-browser.computed.mjs (1)
1-38: LGTM — filtering logic is defensive and correctly handles empty keyword/path-filter edge cases.web-ui/session-helpers.mjs (1)
191-220: LGTM — both thepluginsandsvntab-entry blocks are correct.
pluginsRequestedIdis immediately cleared before use (Line 192), eliminating the stale-value window on real navigations. Thesvnblock's defensivetypeofchecks before callingrestoreSvnLogBrowserPrefsand$nextTickare consistent with the rest of the file.web-ui/partials/index/layout-header.html (1)
84-93: All i18n keys for the SVN tab are defined ini18n.dict.mjs.Verification confirms that
tab.svn,side.tools,side.svn.meta, andside.svn.hintare all present in the i18n dictionary.tests/e2e/test-plugin-svn-log-browser.js (3)
1-30: Utility helpers are correct and well-scoped.
listencorrectly uses port0to bind an ephemeral address (resolving the past hardcoded-port comment),closewraps the callback-basedserver.close()cleanly, andreadBodydrains the body viasetEncoding('utf-8')so chunks arrive as strings. No concerns here.
32-123: Mock server implementation is correct.A few things verified:
- The
/brokenPROPFIND intentionally omitsS:repository-rootandS:baseline-relative-path, which correctly triggers the!info.repositoryRootbranch inloadLogs(context snippet 2) →filterLogsByPathis invoked → revision 11 (/other/new.txt) is excluded while revisions 12 and 10 (/target/*) pass. ✓- The hardcoded
http://127.0.0.1/repoin the non-brokenS:repository-root(line 60) uses port 80, notsvnPort. This is intentional and safe:repositoryRootis only checked for truthiness inloadLogsto decide whether to skip path filtering, never used as an outbound request URL. The REPORT target is derived fromcheckedInHref, which resolves against the full request URL and therefore carries the correct ephemeral port. ✓- The mixed namespace prefixes across log item fields (
D:version-name,S:creator-displayname,S:date,D:comment,S:modified-path, etc.) are all consistent with xml2js's stripPrefix processor thatnormalizeLogItems/mapChangedPathsdepend on. ✓- The REPORT limit-slicing logic on line 108 (
limit ? logItems.slice(0, Math.min(limit, logItems.length)) : logItems) correctly returns all three items for large limits and a subset for small ones, matching the paginated server behaviour.
125-162: Test assertions are logically sound.
- Cache miss/hit sequence (lines 134/140) exercises the 60-second in-memory cache with identical params. ✓
- Page 1 (
pageSize: 2) returns 2 items withhasMore: true; page 2 returns the remaining 1 item withhasMore: false. The mock's 3-item corpus is the minimum needed to verify both sides of thehasMoreboundary in one pass — clean design. ✓- The broken-path assertions (lines 156–158) test three distinct conditions (revision present, revision present, revision absent) without coupling to ordering, which is the right level of specificity for a filter test. ✓
- The
finallyblock ensures the mock is always torn down even on assertion failure. ✓
| if (!requesterIsLoopback) { | ||
| try { | ||
| const parsedUrl = new URL(rawUrl); | ||
| if (isPrivateNetworkHost(parsedUrl.hostname || '')) { | ||
| result = { error: 'Refusing to access private network svn url from non-loopback request' }; | ||
| break; | ||
| } | ||
| } catch (_) {} |
There was a problem hiding this comment.
Block DNS-based private-network SSRF, not just literal private IP hostnames
For non-loopback callers, the guard only checks parsedUrl.hostname as a literal. A public hostname that resolves to RFC1918/loopback (or DNS-rebinds) will pass and still hit internal targets. This weakens the private-network restriction on both svn-info and svn-logs.
🔐 Suggested hardening (resolve hostname and re-check resolved addresses)
+const dns = require('dns').promises;
+
+async function resolvesToPrivateNetwork(hostname) {
+ const host = typeof hostname === 'string' ? hostname.trim() : '';
+ if (!host) return true;
+ if (isPrivateNetworkHost(host)) return true; // literal IP / localhost
+ if (net.isIP(host)) return false;
+ let records = [];
+ try {
+ records = await dns.lookup(host, { all: true, verbatim: true });
+ } catch (_) {
+ // Fail closed for non-loopback remote requests
+ return true;
+ }
+ return records.some((r) => isPrivateNetworkHost(r && r.address ? r.address : ''));
+}- if (isPrivateNetworkHost(parsedUrl.hostname || '')) {
+ if (await resolvesToPrivateNetwork(parsedUrl.hostname || '')) {
result = { error: 'Refusing to access private network svn url from non-loopback request' };
break;
}Also applies to: 10034-10041
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli.js` around lines 10010 - 10017, The hostname-only check allows DNS-based
SSRF; update the non-loopback guard around parsedUrl/rawUrl (where
requesterIsLoopback is used and isPrivateNetworkHost(parsedUrl.hostname) is
called) to resolve the hostname to IP addresses and re-check each resolved
address with isPrivateNetworkHost before allowing access: use the Node dns
promises API (e.g., dns.promises.lookup or dns.promises.resolve* with all: true)
to get both IPv4/IPv6 results, treat any resolved private/loopback address as
forbidden, handle lookup errors/timeouts conservatively (fail-closed or treat as
forbidden), and apply the same change to the other identical block at the later
occurrence.
|
|
||
| export function createPluginsMethods() { | ||
| return { | ||
| ...createSvnLogBrowserMethods({ api }), |
There was a problem hiding this comment.
SVN credentials persisted as plaintext in localStorage — scope the password to sessionStorage.
createSvnLogBrowserMethods (context snippet, web-ui/modules/svn-log-browser.methods.mjs:47–90) calls persistSvnLogBrowserPrefs(), which writes the SVN password verbatim into localStorage when svnLogBrowserRemember === true. localStorage is readable by any same-origin JavaScript (XSS), is visible in browser DevTools, and survives browser restarts — an unnecessarily wide exposure for a credential.
The password field should use sessionStorage (cleared on tab/session close) while the non-sensitive preferences (url, username, pageSize, pathFilter) remain in localStorage. The change is localized to svnLogBrowserMethodsSavePrefs / svnLogBrowserMethodsLoadPrefs in svn-log-browser.methods.mjs.
🔒 Suggested split-storage approach (applies to svn-log-browser.methods.mjs)
// in svnLogBrowserMethodsSavePrefs (or persistSvnLogBrowserPrefs)
-storage.setItem(PREFS_KEY, JSON.stringify({ remember, url, username, password, pageSize, pathFilter }));
+storage.setItem(PREFS_KEY, JSON.stringify({ remember, url, username, pageSize, pathFilter }));
+if (typeof sessionStorage !== 'undefined') {
+ remember
+ ? sessionStorage.setItem(PREFS_PWD_KEY, password)
+ : sessionStorage.removeItem(PREFS_PWD_KEY);
+}
// in svnLogBrowserMethodsLoadPrefs (or restoreSvnLogBrowserPrefs)
-this.svnLogBrowserPassword = prefs.password ?? '';
+this.svnLogBrowserPassword = (typeof sessionStorage !== 'undefined'
+ ? sessionStorage.getItem(PREFS_PWD_KEY) ?? ''
+ : '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/prompt-templates/methods.mjs` at line 86, The SVN password is being
persisted in localStorage by createSvnLogBrowserMethods via
svnLogBrowserMethodsSavePrefs; change that so only non-sensitive prefs (url,
username, pageSize, pathFilter, svnLogBrowserRemember) are written to
localStorage and the password is written to sessionStorage when
svnLogBrowserRemember === true (and removed from sessionStorage when false);
update svnLogBrowserMethodsLoadPrefs to read the password from sessionStorage
(falling back to no password) while loading other prefs from localStorage, and
ensure that when toggling remember off any stored password in localStorage is
removed/migrated to sessionStorage appropriately so no plaintext password
remains in localStorage.
| <label class="form-label">{{ t('plugins.svnLogBrowser.fields.url') }}</label> | ||
| <input | ||
| ref="svnLogBrowserUrlInput" | ||
| class="form-input mono" | ||
| type="text" | ||
| v-model.trim="svnLogBrowserUrl" | ||
| :placeholder="t('plugins.svnLogBrowser.placeholders.url')" | ||
| spellcheck="false" | ||
| autocomplete="off"> | ||
|
|
||
| <div class="svn-log-browser-cred-row"> | ||
| <div> | ||
| <label class="form-label">{{ t('plugins.svnLogBrowser.fields.username') }}</label> | ||
| <input | ||
| class="form-input mono" | ||
| type="text" | ||
| v-model.trim="svnLogBrowserUsername" | ||
| :placeholder="t('plugins.svnLogBrowser.placeholders.username')" | ||
| spellcheck="false" | ||
| autocomplete="username"> | ||
| </div> | ||
| <div> | ||
| <label class="form-label">{{ t('plugins.svnLogBrowser.fields.password') }}</label> | ||
| <input | ||
| class="form-input mono" | ||
| type="password" | ||
| v-model="svnLogBrowserPassword" | ||
| :placeholder="t('plugins.svnLogBrowser.placeholders.password')" | ||
| spellcheck="false" | ||
| autocomplete="current-password"> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="svn-log-browser-cred-actions"> | ||
| <label class="svn-log-browser-toggle"> | ||
| <input type="checkbox" v-model="svnLogBrowserRemember" @change="persistSvnLogBrowserPrefs"> | ||
| <span>{{ t('plugins.svnLogBrowser.remember') }}</span> | ||
| </label> | ||
| </div> | ||
|
|
||
| <div class="svn-log-browser-ctrl-row"> | ||
| <div> | ||
| <label class="form-label">{{ t('plugins.svnLogBrowser.fields.pageSize') }}</label> | ||
| <input | ||
| class="form-input mono" | ||
| type="number" | ||
| min="1" | ||
| max="200" | ||
| v-model.trim="svnLogBrowserPageSizeInput" | ||
| @change="normalizeSvnLogBrowserPageSize"> | ||
| </div> | ||
| <div> | ||
| <label class="form-label">{{ t('plugins.svnLogBrowser.fields.pathFilter') }}</label> | ||
| <input | ||
| class="form-input mono" | ||
| type="text" | ||
| v-model.trim="svnLogBrowserPathFilter" | ||
| :placeholder="t('plugins.svnLogBrowser.placeholders.pathFilter')" | ||
| spellcheck="false" | ||
| autocomplete="off"> | ||
| <div class="plugins-panel-note">{{ t('plugins.svnLogBrowser.hints.pathFilter') }}</div> |
There was a problem hiding this comment.
<label> elements are not programmatically associated with their inputs.
Lines 17, 29, 39, 59, and 69 each render a <label> with no for attribute, and their sibling <input> elements carry no matching id. Without explicit for/id pairing (or wrapping the input inside the label), assistive technologies cannot infer the label-control relationship, and clicking the label text will not move focus to the input.
♿ Proposed fix (URL field shown as representative example)
- <label class="form-label">{{ t('plugins.svnLogBrowser.fields.url') }}</label>
+ <label class="form-label" for="svn-url">{{ t('plugins.svnLogBrowser.fields.url') }}</label>
<input
ref="svnLogBrowserUrlInput"
class="form-input mono"
type="text"
+ id="svn-url"
v-model.trim="svnLogBrowserUrl"Apply the same for/id pattern to the username (svn-username), password (svn-password), page size (svn-page-size), and path filter (svn-path-filter) pairs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/partials/index/panel-svn.html` around lines 17 - 77, The labels in the
SVN panel template are not associated with their inputs; add matching id
attributes to the inputs and for attributes to the corresponding labels so
clicking labels focuses the inputs (use ids like svn-url, svn-username,
svn-password, svn-page-size, svn-path-filter) and keep existing bindings
(ref="svnLogBrowserUrlInput", v-model="svnLogBrowserPassword",
v-model.trim="svnLogBrowserUsername", v-model.trim="svnLogBrowserPageSizeInput",
v-model.trim="svnLogBrowserPathFilter") so the label-for pairs map to the exact
inputs in the diff.
| <div class="svn-log-browser-info-key mono">path</div> | ||
| <div class="svn-log-browser-info-value mono">{{ svnLogBrowserInfo.relativePath }}</div> | ||
| </div> | ||
| <div class="svn-log-browser-info-row"> | ||
| <div class="svn-log-browser-info-key mono">rev</div> | ||
| <div class="svn-log-browser-info-value mono">{{ svnLogBrowserInfo.revision }}</div> | ||
| </div> |
There was a problem hiding this comment.
Hardcoded "path" and "rev" labels bypass i18n.
Lines 107 and 111 embed raw English strings directly while every other label in the panel uses t(...). This means these two labels will not translate.
🌐 Proposed fix
- <div class="svn-log-browser-info-key mono">path</div>
+ <div class="svn-log-browser-info-key mono">{{ t('plugins.svnLogBrowser.info.path') }}</div>- <div class="svn-log-browser-info-key mono">rev</div>
+ <div class="svn-log-browser-info-key mono">{{ t('plugins.svnLogBrowser.info.rev') }}</div>Add the corresponding keys to web-ui/modules/i18n.dict.mjs alongside the other SVN entries.
📝 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.
| <div class="svn-log-browser-info-key mono">path</div> | |
| <div class="svn-log-browser-info-value mono">{{ svnLogBrowserInfo.relativePath }}</div> | |
| </div> | |
| <div class="svn-log-browser-info-row"> | |
| <div class="svn-log-browser-info-key mono">rev</div> | |
| <div class="svn-log-browser-info-value mono">{{ svnLogBrowserInfo.revision }}</div> | |
| </div> | |
| <div class="svn-log-browser-info-key mono">{{ t('plugins.svnLogBrowser.info.path') }}</div> | |
| <div class="svn-log-browser-info-value mono">{{ svnLogBrowserInfo.relativePath }}</div> | |
| </div> | |
| <div class="svn-log-browser-info-row"> | |
| <div class="svn-log-browser-info-key mono">{{ t('plugins.svnLogBrowser.info.rev') }}</div> | |
| <div class="svn-log-browser-info-value mono">{{ svnLogBrowserInfo.revision }}</div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/partials/index/panel-svn.html` around lines 107 - 113, Replace the
hardcoded English labels "path" and "rev" in the SVN info rows (the divs using
class svn-log-browser-info-key) with calls to the translation helper used
elsewhere (e.g., t('svn.path') and t('svn.rev')) and add matching keys
("svn.path" and "svn.rev") to the i18n dictionary in
web-ui/modules/i18n.dict.mjs alongside the other SVN entries so
svnLogBrowserInfo.relativePath and svnLogBrowserInfo.revision render with
localized labels.
| <div v-else class="svn-log-browser-list" role="list"> | ||
| <div v-for="item in svnLogBrowserFilteredItems" :key="'svn-log-' + item.revision" class="svn-log-browser-item"> | ||
| <div class="svn-log-browser-item-head"> | ||
| <div class="svn-log-browser-item-rev mono">r{{ item.revision }}</div> | ||
| <div class="svn-log-browser-item-author mono">{{ item.author || '—' }}</div> | ||
| <div class="svn-log-browser-item-date mono">{{ item.date || '—' }}</div> | ||
| </div> | ||
| <div class="svn-log-browser-item-message">{{ item.message || '—' }}</div> | ||
| <div v-if="item.paths && item.paths.length" class="svn-log-browser-item-paths"> | ||
| <div v-for="(p, idx) in item.paths" :key="'svn-log-path-' + item.revision + '-' + idx" class="svn-log-browser-item-path mono"> | ||
| <span class="svn-log-browser-item-action">{{ p.action }}</span> | ||
| <span class="svn-log-browser-item-path-text">{{ p.path }}</span> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
role="list" container is missing role="listitem" on its children.
The ARIA list role is normally used in conjunction with the listitem role, which identifies a list item contained inside the list. The v-for divs at Line 134 carry no role="listitem", so screen readers cannot announce the list count or let users navigate between entries.
♿ Proposed fix
- <div v-for="item in svnLogBrowserFilteredItems" :key="'svn-log-' + item.revision" class="svn-log-browser-item">
+ <div v-for="item in svnLogBrowserFilteredItems" :key="'svn-log-' + item.revision" class="svn-log-browser-item" role="listitem">Alternatively, replace the outer div role="list" with a <ul> and each item div with an <li>, which best practice dictates as the preferred approach over explicit ARIA roles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/partials/index/panel-svn.html` around lines 133 - 147, The container
div with class "svn-log-browser-list" uses role="list" but its child v-for items
(divs with class "svn-log-browser-item" generated by v-for over
svnLogBrowserFilteredItems) lack role="listitem"; update those child divs to
include role="listitem" (and also add role="listitem" to each path entry div
with class "svn-log-browser-item-path" if you keep the inner paths as a list) or
replace the outer div with a semantic <ul> and change the v-for item divs to
<li> (and path container to nested <ul>/<li>) to follow best-practice semantics
and ensure screen readers can announce and navigate list items.
c18e0f0 to
1a1605d
Compare
2 participants
Summary
Tests
Summary by CodeRabbit
New Features
Style
Tests