Prevent hanging in svn+ssh#1230
Conversation
|
Warning Review limit reached
More reviews will be available in 15 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughCentralizes SVN non-interactive SSH handling by injecting BatchMode=yes into SVN_SSH, adding SshHostKeyError detection with ssh-keyscan/ssh -T hints, routing svn calls through _run_svn/_run_svn_raw across SvnRemote and SvnRepo, and updating tests, docs, CI, and related git/submodule behavior. ChangesNon-Interactive SVN SSH Mode
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs:
Suggested labels: Suggested reviewers:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@CHANGELOG.rst`:
- Line 23: Update the CHANGELOG line that currently reads "* Run ``svn+ssh://``
connections in non-interactive mode to prevent hanging (`#0`)" to reference the
correct issue/PR number; replace the placeholder "(`#0`)" with "(`#1230`)" so the
entry reads "...prevent hanging (`#1230`)". Ensure only the numeric reference is
changed and formatting (backticks and punctuation) is preserved.
In `@dfetch/vcs/svn.py`:
- Around line 37-55: The remediation message currently uses the output of
_ssh_target_from_url (which returns "user@host") for both commands; change
_raise_if_ssh_host_key_error to parse the URL (urlparse(url)) locally, build
host_only = parsed.hostname or url for the ssh-keyscan suggestion, and keep
target = _ssh_target_from_url(url) (user@host) for the ssh -T suggestion so the
ssh-keyscan line uses only the hostname/IP while the ssh -T line still shows
user@host.
In `@example/dfetch.yaml`:
- Line 10: Update the example near the url-base entry (url-base:
svn+ssh://svn.code.sf.net/p/) to add a brief note explaining that SourceForge
requires users to register an SSH public key in their account and to accept the
host's SSH key (add guidance to add the host key to ~/.ssh/known_hosts or run
ssh-keyscan for the host), and include a short smoke-test instruction to verify
the repository is reachable via svn+ssh (e.g., attempt an svn list/ls of the
tortoisesvn/code path) so reviewers know how to validate the ssh-key/known-hosts
setup non-interactively.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: a4056c4b-b3e0-4ab1-b83c-6b6a4029179a
📒 Files selected for processing (4)
CHANGELOG.rstdfetch/vcs/svn.pyexample/dfetch.yamltests/test_svn.py
spoorcc
force-pushed
the
svn-ssh-non-onteractive
branch
from
16d4f3a to
81387c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/test_svn.py`:
- Around line 569-583: Add an autouse pytest fixture in this test module that
centralizes clearing the lru_cache for _extend_env_for_non_interactive_mode:
create a fixture (autouse=True) which imports
dfetch.vcs.svn._extend_env_for_non_interactive_mode and calls
_extend_env_for_non_interactive_mode.cache_clear() before each test (and
optionally after) so the cached process-global state is reset; then remove the
repetitive per-test imports and explicit
_extend_env_for_non_interactive_mode.cache_clear() calls from tests like
test_svn_ssh_env_has_batch_mode, test_svn_ssh_env_preserves_existing_batch_mode
and the *_passes_batch_mode_env tests.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 86c26433-10d8-4f73-b002-421224f1f319
📒 Files selected for processing (3)
CHANGELOG.rstdfetch/vcs/svn.pytests/test_svn.py
spoorcc
force-pushed
the
svn-ssh-non-onteractive
branch
2 times, most recently
from
e470253 to
d798bd0
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@dfetch/project/svnsubproject.py`:
- Around line 165-169: The except block catching SubprocessCommandError should
preserve and log the underlying failure details; change the handler to capture
the exception (e.g., "except SubprocessCommandError as e:") and include the
revision and the exception/return-code information in the logger.warning call
for the externals-report skip (use the existing complete_path and the local
revision variable and e or e.returncode/str(e) as available), or include
exc_info=True to preserve traceback.
In `@dfetch/util/cmdline.py`:
- Around line 49-51: The inline Bandit suppression should be removed and the
root cause fixed in run_on_cmdline: stop relying on "# nosec" by ensuring
subprocess.run is invoked safely (use shell=False and a sequence for cmd or
parse cmd with shlex.split), validate or sanitize the cmd input (or enforce a
whitelist of allowed commands) before calling subprocess.run, and keep existing
arguments (env, capture_output, check, timeout) while removing the "# nosec"
comment next to subprocess.run so Bandit no longer flags an unsafe shell
invocation of cmd.
- Around line 52-58: The TimeoutExpired handler in run_on_cmdline currently
drops captured output; change the except block to extract exc.stdout and
exc.stderr (they may be bytes or None), decode bytes to str using
errors="replace", default to "" when None, and pass those decoded stdout/stderr
into the SubprocessCommandError constructor (retain the timeout message and exit
code 124) so partial output is preserved for diagnostics; reference
run_on_cmdline and SubprocessCommandError to locate and update the handler.
In `@dfetch/vcs/svn.py`:
- Around line 239-261: externals_from_url should handle SSH host-key
verification errors like other SVN helpers: wrap the run_on_cmdline call in
externals_from_url in a try/except that catches SubprocessCommandError and calls
_raise_if_ssh_host_key_error(logger, err) (or re-raise if not an SSH host-key
issue) before proceeding; ensure you still decode result.stdout and call
SvnRepo._normalize_url_prefix and SvnRepo._parse_externals after successful
run_on_cmdline.
- Around line 25-35: The cached function _extend_env_for_non_interactive_mode
currently returns a mutable dict which lru_cache will reuse across calls; change
it to return an immutable mapping so callers cannot mutate the cached object:
keep the caching but wrap the computed env in types.MappingProxyType (import
MappingProxyType from types) and update the return type annotation to
Mapping[str, str] (or similar typing.Mapping) so the cached value is an
immutable view of the environment.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e6748593-13b3-4b61-a828-c15382bb17b7
📒 Files selected for processing (9)
CHANGELOG.rstdfetch/project/svnsubproject.pydfetch/util/cmdline.pydfetch/vcs/svn.pydoc/explanation/threat_model_usage.rstsecurity/tm_usage.pytests/test_cmdline.pytests/test_svn.pytests/test_svnsubproject.py
| except SubprocessCommandError: | ||
| logger.warning( | ||
| "Could not retrieve SVN externals from '%s' — skipping report.", | ||
| complete_path, | ||
| ) |
There was a problem hiding this comment.
Preserve the subprocess failure details in this warning.
This warning drops the reason externals reporting was skipped. Please include at least the revision plus the underlying SVN error/return code so operators can tell an expected timeout/host-key case from an unexpected repository/configuration failure.
🤖 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 `@dfetch/project/svnsubproject.py` around lines 165 - 169, The except block
catching SubprocessCommandError should preserve and log the underlying failure
details; change the handler to capture the exception (e.g., "except
SubprocessCommandError as e:") and include the revision and the
exception/return-code information in the logger.warning call for the
externals-report skip (use the existing complete_path and the local revision
variable and e or e.returncode/str(e) as available), or include exc_info=True to
preserve traceback.
| proc = subprocess.run( | ||
| cmd, env=env, capture_output=True, check=True, timeout=timeout | ||
| ) # nosec |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Remove the inline Bandit suppression from the subprocess wrapper.
This change still relies on # nosec in production code. Since run_on_cmdline is already the single subprocess boundary, fix the underlying Bandit concern here once instead of suppressing the call inline.
As per coding guidelines, dfetch/**/*.py: "Avoid inline lint suppressions (# noqa, # type: ignore, # pylint: disable, # pyright: ignore) without fixing the root cause; exception permitted for module-level tool headers (lines 1–5) in test files".
🤖 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 `@dfetch/util/cmdline.py` around lines 49 - 51, The inline Bandit suppression
should be removed and the root cause fixed in run_on_cmdline: stop relying on "#
nosec" by ensuring subprocess.run is invoked safely (use shell=False and a
sequence for cmd or parse cmd with shlex.split), validate or sanitize the cmd
input (or enforce a whitelist of allowed commands) before calling
subprocess.run, and keep existing arguments (env, capture_output, check,
timeout) while removing the "# nosec" comment next to subprocess.run so Bandit
no longer flags an unsafe shell invocation of cmd.
| except subprocess.TimeoutExpired as exc: | ||
| raise SubprocessCommandError( | ||
| list(exc.cmd) if isinstance(exc.cmd, (list, tuple)) else [str(exc.cmd)], | ||
| "", | ||
| f"Command timed out after {exc.timeout:.0f}s", | ||
| 124, | ||
| ) from exc |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby context
FILE="dfetch/util/cmdline.py"
echo "== $FILE =="
nl -ba "$FILE" | sed -n '1,140p'
echo
# Locate SubprocessCommandError definition
echo "== SubprocessCommandError definition/usages =="
rg -n "class SubprocessCommandError|SubprocessCommandError\(" -S dfetch/util/cmdline.py dfetch -g'*.py'
# Show the exact SubprocessCommandError class if found
matches=$(rg -n "class SubprocessCommandError" -S dfetch -g'*.py' || true)
if [ -n "$matches" ]; then
echo
echo "== Showing class SubprocessCommandError block =="
# Print file sections around each match
echo "$matches" | while read -r line; do
f=$(echo "$line" | cut -d: -f1)
ln=$(echo "$line" | cut -d: -f2)
start=$((ln-20))
end=$((ln+80))
nl -ba "$f" | sed -n "${start},${end}p"
echo
done
fi
# Check whether os is already imported in cmdline.py
echo "== Imports in dfetch/util/cmdline.py =="
rg -n "^import|^from" "$FILE"Repository: dfetch-org/dfetch
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="dfetch/util/cmdline.py"
echo "== File exists?"
test -f "$FILE" && echo "OK: $FILE" || (echo "MISSING: $FILE" && exit 1)
echo
echo "== $FILE (lines 1-140)"
cat -n "$FILE" | sed -n '1,140p'
echo
echo "== TimeoutExpired branch (exact matches)"
rg -n "except subprocess\.TimeoutExpired as exc" -n "$FILE" || true
rg -n "TimeoutExpired" "$FILE" || true
echo
echo "== SubprocessCommandError definition/constructor"
rg -n "class SubprocessCommandError|def __init__\(|SubprocessCommandError\(" -S "$FILE" || true
# Print block around the first class definition match, if present
class_line=$(rg -n "class SubprocessCommandError" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${class_line:-}" ]; then
start=$((class_line-20))
end=$((class_line+120))
echo
echo "== Showing around SubprocessCommandError at ~line $class_line"
cat -n "$FILE" | sed -n "${start},${end}p"
fi
echo
echo "== Imports in $FILE"
rg -n "^(import|from )" "$FILE" || true
echo
echo "== SubprocessCommandError usages elsewhere (show a few)"
rg -n "SubprocessCommandError\(" -S dfetch -g'*.py' | head -n 20Repository: dfetch-org/dfetch
Length of output: 6117
🌐 Web query:
Python subprocess.TimeoutExpired attributes output stdout stderr capture_output run
💡 Result:
In Python, when using subprocess.run with a timeout parameter, a subprocess.TimeoutExpired exception is raised if the process does not terminate within the specified time [1][2]. Important details regarding this exception and output capture include: 1. Accessing Captured Output: When a TimeoutExpired exception is raised, you can access the output captured up to the point of the timeout via the exception's attributes [3]. Specifically: - stdout: Contains the standard output captured [1][2]. - stderr: Contains the standard error captured [1][2]. - output: An alias for stdout, provided for symmetry with stderr [1][2]. 2. Data Format: Regardless of whether you set text=True or encoding in your subprocess.run call, the stdout and stderr attributes on the TimeoutExpired exception will always be in bytes [1][2][4]. They will be None if no output was captured or if capture_output=True (or stdout=PIPE/stderr=PIPE) was not used [1][2]. 3. Usage Pattern: To access this data, you must catch the exception [3]. For example: try: result = subprocess.run(["your_command"], capture_output=True, text=True, timeout=5) except subprocess.TimeoutExpired as e: # Access bytes output here captured_stdout = e.stdout captured_stderr = e.stderr # If text=True was used, you may need to decode manually: # decoded_stdout = e.stdout.decode if e.stdout else None 4. Behavior with capture_output: When you use capture_output=True in subprocess.run, Python automatically configures stdout and stderr as pipes (stdout=PIPE and stderr=PIPE) [5]. If a timeout occurs, the subprocess library ensures that whatever data was already buffered in these pipes is attached to the exception [1][2]. Note: Before Python 3.5, the stdout and stderr attributes were not present on the TimeoutExpired exception [1][2]. Always ensure your code accounts for these attributes being bytes, even when operating in text mode [6][1].
Citations:
- 1: https://docs.python.org/3.10/library/subprocess.html
- 2: https://docs.python.org/3/library/subprocess.html
- 3: https://stackoverflow.com/questions/64640506/capture-output-from-subprocess-with-a-timeout-set
- 4: https://github.com/python/cpython/blob/0274d830/Doc/library/subprocess.rst
- 5: https://docs.python.org/release/3.11.5/library/subprocess.html
- 6: Subprocess timeout causes output to be returned as bytes in text mode python/cpython#87597
Preserve captured stdout/stderr on subprocess timeouts for better diagnostics.
run_on_cmdline uses subprocess.run(..., capture_output=True), and subprocess.TimeoutExpired can include the buffered stdout/stderr from the timed-out process. The current handler replaces stdout with "" and only keeps the timeout message, dropping the most useful partial output.
File: dfetch/util/cmdline.py
Lines: 52-58
except subprocess.TimeoutExpired as exc:
raise SubprocessCommandError(
list(exc.cmd) if isinstance(exc.cmd, (list, tuple)) else [str(exc.cmd)],
"",
f"Command timed out after {exc.timeout:.0f}s",
124,
) from exc
Suggested fix
except subprocess.TimeoutExpired as exc:
+ stdout = exc.output.decode(errors="replace").strip() if isinstance(exc.output, bytes) else (exc.output or "")
+ stderr = exc.stderr.decode(errors="replace").strip() if isinstance(exc.stderr, bytes) else (exc.stderr or "")
+ timeout_msg = f"Command timed out after {exc.timeout:g}s"
raise SubprocessCommandError(
- list(exc.cmd) if isinstance(exc.cmd, (list, tuple)) else [str(exc.cmd)],
- "",
- f"Command timed out after {exc.timeout:.0f}s",
+ list(exc.cmd) if isinstance(exc.cmd, (list, tuple)) else [str(exc.cmd)],
+ stdout,
+ f"{stderr}{os.linesep if stderr else ''}{timeout_msg}",
124,
) from exc🤖 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 `@dfetch/util/cmdline.py` around lines 52 - 58, The TimeoutExpired handler in
run_on_cmdline currently drops captured output; change the except block to
extract exc.stdout and exc.stderr (they may be bytes or None), decode bytes to
str using errors="replace", default to "" when None, and pass those decoded
stdout/stderr into the SubprocessCommandError constructor (retain the timeout
message and exit code 124) so partial output is preserved for diagnostics;
reference run_on_cmdline and SubprocessCommandError to locate and update the
handler.
d798bd0 to
f6f8068
Compare
spoorcc
force-pushed
the
svn-ssh-non-onteractive
branch
from
f6f8068 to
ce9644c
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/test_svn.py (1)
577-584: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winVerify that existing SSH options are preserved.
The test correctly validates that
BatchMode=yesis not duplicated, but it doesn't verify that other SSH options (like-i /my/key) are preserved in the result. Consider adding an assertion to ensure the full original value is maintained.🔍 Suggested assertion
monkeypatch.setenv("SVN_SSH", "ssh -o BatchMode=yes -i /my/key") _extend_env_for_non_interactive_mode.cache_clear() env = _extend_env_for_non_interactive_mode() assert env["SVN_SSH"].count("BatchMode=yes") == 1 + assert "-i /my/key" in env["SVN_SSH"]🤖 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 `@tests/test_svn.py` around lines 577 - 584, The test test_svn_ssh_env_preserves_existing_batch_mode should also assert that existing SSH options are preserved; after calling _extend_env_for_non_interactive_mode(), add an assertion that env["SVN_SSH"] still contains the original key option (e.g. assert "-i /my/key" in env["SVN_SSH"] or assert env["SVN_SSH"].startswith("ssh -o BatchMode=yes -i /my/key") to ensure the full original value is maintained) while keeping the existing BatchMode=yes duplication check.dfetch/vcs/svn.py (1)
389-404:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFinish applying the centralized SSH host-key handling to the remaining remote entry points.
get_last_changed_revision()for remote targets,export(), andfiles_in_path()all passBatchMode=yes, but they still surface rawSubprocessCommandErroron host-key verification failures. Wrap theserun_on_cmdline()calls the same way asget_info_from_target()so callers get the same remediation path everywhere.Also applies to: 443-449, 454-460
🤖 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 `@dfetch/vcs/svn.py` around lines 389 - 404, get_last_changed_revision(), export(), and files_in_path() still call run_on_cmdline() directly and will raise raw SubprocessCommandError on SSH host-key verification failures; update each to mirror get_info_from_target() by wrapping their run_on_cmdline() calls in the same try/except used there (catch SubprocessCommandError), invoke the same remediation/host-key handling helper used by get_info_from_target(), and re-run or re-raise as appropriate so callers receive the centralized remediation path instead of raw subprocess errors; locate the run_on_cmdline invocations in get_last_changed_revision(), export(), and files_in_path() and apply the identical exception-handling pattern found in get_info_from_target().
♻️ Duplicate comments (4)
dfetch/vcs/svn.py (2)
48-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a host-only target in the
ssh-keyscanhint.
_ssh_target_from_url()can returnuser@host, butssh-keyscanexpects the host portion. Keepuser@hostonly for thessh -Texample.Does `ssh-keyscan` accept targets in `user@host` form, or only hostnames/IP addresses?🤖 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 `@dfetch/vcs/svn.py` around lines 48 - 54, The ssh hint uses _ssh_target_from_url(url) which may return "user@host" but ssh-keyscan requires just the host; update the code to extract a host-only value (e.g., call or add a helper to parse host from url or strip the user prefix from the result of _ssh_target_from_url) and use that host-only string in the ssh-keyscan line, while keeping the original target (user@host) for the ssh -T example; adjust the two formatted f-strings in the RuntimeError construction around _ssh_target_from_url(url) accordingly (use host_only for the ssh-keyscan hint and the original target for the ssh -T hint).
238-241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle host-key failures in
externals_from_url().This
svn propgetcall now runs with the non-interactive SSH env, but it still skips_raise_if_ssh_host_key_error(), so missing known-hosts state leaks a rawSubprocessCommandErrorinstead of the new remediation message.♻️ Minimal fix
- result = run_on_cmdline(logger, cmd, env=_extend_env_for_non_interactive_mode()) + try: + result = run_on_cmdline( + logger, + cmd, + env=_extend_env_for_non_interactive_mode(), + ) + except SubprocessCommandError as exc: + _raise_if_ssh_host_key_error(url, exc) + raise🤖 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 `@dfetch/vcs/svn.py` around lines 238 - 241, The svn propget call in externals_from_url (the run_on_cmdline invocation) runs with non-interactive SSH but doesn't check for SSH host-key failures; after capturing the subprocess result, call SvnRepo._raise_if_ssh_host_key_error(result, url) (or the existing helper that inspects the subprocess result) before calling SvnRepo._normalize_url_prefix and SvnRepo._parse_externals so host-key issues produce the remediation message instead of a raw SubprocessCommandError.CHANGELOG.rst (1)
24-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate placeholder issue reference.
The changelog entry still references
(#0), which should be(#1230)based on the PR objectives. This issue was previously flagged and marked as addressed in commit 81387c8, but remains in the current code.📝 Proposed fix
-* Run ``svn+ssh://`` connections in non-interactive mode to prevent hanging (`#0`) +* Run ``svn+ssh://`` connections in non-interactive mode to prevent hanging (`#1230`)🤖 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 `@CHANGELOG.rst` at line 24, Update the placeholder issue reference in the changelog entry that reads "* Run ``svn+ssh://`` connections in non-interactive mode to prevent hanging (`#0`)" to use the correct issue number "(`#1230`)"; locate the changelog line containing the phrase "Run ``svn+ssh://`` connections in non-interactive mode to prevent hanging" and replace the "(`#0`)" token with "(`#1230`)" so the entry reflects the proper issue reference.example/dfetch.yaml (1)
10-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVerify SSH prerequisites for the svn+ssh example.
The change from
svn://tosvn+ssh://demonstrates the non-interactive SSH mode feature. However, documentation explaining SSH key registration and known-hosts setup for SourceForge was previously requested and marked as addressed in commit 81387c8, but is not visible in the current code.🤖 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 `@example/dfetch.yaml` at line 10, The example changed the config key value url-base: svn+ssh://svn.code.sf.net/p/ to use non-interactive SSH but lacks documentation on SSH prerequisites; add a short doc/README note referenced from the example that explains registering your SSH public key with SourceForge, adding SourceForge's host key to users' known_hosts (or using a secured ssh config entry), and testing non-interactive SSH access (ssh -T or svn list) before using the url-base value, and link or mention this guidance next to the example so users of the url-base: svn+ssh://... snippet know the required SSH setup steps.
🤖 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 @.claude/settings.json:
- Around line 5-31: Split the unrelated expanded Claude assistant permission
entries (e.g., "Bash(git stash *)", "Bash(pip install *)", "Bash(pytest *)",
"Bash(dfetch add *)", etc.) out of the current commit into a separate PR focused
on dev-tooling/config changes, leaving only the svn+ssh hang fix in this PR;
alternatively, if any of these entries are required to test or validate the
svn+ssh change, add a short note in the PR description explaining which specific
permission(s) (reference the exact entries above) are required and why so
reviewers understand their relevance.
In `@tests/test_svn.py`:
- Around line 542-549: Add a new parametrized test in tests/test_svn.py that
mirrors test_get_info_from_target_raises_hint_on_ssh_host_key_failure but covers
SvnRepo.externals_from_url, SvnRepo.get_last_changed_revision,
SvnRepo("...").export, and SvnRepo.files_in_path: for each method call patch
dfetch.vcs.svn.run_on_cmdline to raise SubprocessCommandError with stderr "Host
key verification failed." and assert the call raises a RuntimeError matching
"known hosts"; reference the SvnRepo class and the four method names
(externals_from_url, get_last_changed_revision, export, files_in_path) so the
test targets the updated non-interactive SSH behavior.
- Around line 525-540: The parameterized test
test_svn_remote_raises_hint_on_ssh_host_key_failure is missing coverage for
SvnRemote.ls_tree; update the param list in the pytest.mark.parametrize tuple to
include ("ls_tree", "svn+ssh://svn.code.sf.net/project") so ls_tree is exercised
the same way as is_svn, list_of_tags, and list_of_branches and will raise the
same SSH host-key hint on SubprocessCommandError.
---
Outside diff comments:
In `@dfetch/vcs/svn.py`:
- Around line 389-404: get_last_changed_revision(), export(), and
files_in_path() still call run_on_cmdline() directly and will raise raw
SubprocessCommandError on SSH host-key verification failures; update each to
mirror get_info_from_target() by wrapping their run_on_cmdline() calls in the
same try/except used there (catch SubprocessCommandError), invoke the same
remediation/host-key handling helper used by get_info_from_target(), and re-run
or re-raise as appropriate so callers receive the centralized remediation path
instead of raw subprocess errors; locate the run_on_cmdline invocations in
get_last_changed_revision(), export(), and files_in_path() and apply the
identical exception-handling pattern found in get_info_from_target().
In `@tests/test_svn.py`:
- Around line 577-584: The test test_svn_ssh_env_preserves_existing_batch_mode
should also assert that existing SSH options are preserved; after calling
_extend_env_for_non_interactive_mode(), add an assertion that env["SVN_SSH"]
still contains the original key option (e.g. assert "-i /my/key" in
env["SVN_SSH"] or assert env["SVN_SSH"].startswith("ssh -o BatchMode=yes -i
/my/key") to ensure the full original value is maintained) while keeping the
existing BatchMode=yes duplication check.
---
Duplicate comments:
In `@CHANGELOG.rst`:
- Line 24: Update the placeholder issue reference in the changelog entry that
reads "* Run ``svn+ssh://`` connections in non-interactive mode to prevent
hanging (`#0`)" to use the correct issue number "(`#1230`)"; locate the changelog
line containing the phrase "Run ``svn+ssh://`` connections in non-interactive
mode to prevent hanging" and replace the "(`#0`)" token with "(`#1230`)" so the
entry reflects the proper issue reference.
In `@dfetch/vcs/svn.py`:
- Around line 48-54: The ssh hint uses _ssh_target_from_url(url) which may
return "user@host" but ssh-keyscan requires just the host; update the code to
extract a host-only value (e.g., call or add a helper to parse host from url or
strip the user prefix from the result of _ssh_target_from_url) and use that
host-only string in the ssh-keyscan line, while keeping the original target
(user@host) for the ssh -T example; adjust the two formatted f-strings in the
RuntimeError construction around _ssh_target_from_url(url) accordingly (use
host_only for the ssh-keyscan hint and the original target for the ssh -T hint).
- Around line 238-241: The svn propget call in externals_from_url (the
run_on_cmdline invocation) runs with non-interactive SSH but doesn't check for
SSH host-key failures; after capturing the subprocess result, call
SvnRepo._raise_if_ssh_host_key_error(result, url) (or the existing helper that
inspects the subprocess result) before calling SvnRepo._normalize_url_prefix and
SvnRepo._parse_externals so host-key issues produce the remediation message
instead of a raw SubprocessCommandError.
In `@example/dfetch.yaml`:
- Line 10: The example changed the config key value url-base:
svn+ssh://svn.code.sf.net/p/ to use non-interactive SSH but lacks documentation
on SSH prerequisites; add a short doc/README note referenced from the example
that explains registering your SSH public key with SourceForge, adding
SourceForge's host key to users' known_hosts (or using a secured ssh config
entry), and testing non-interactive SSH access (ssh -T or svn list) before using
the url-base value, and link or mention this guidance next to the example so
users of the url-base: svn+ssh://... snippet know the required SSH setup steps.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: ee5c747c-98e6-4f39-abc2-48501f388bab
📒 Files selected for processing (5)
.claude/settings.jsonCHANGELOG.rstdfetch/vcs/svn.pyexample/dfetch.yamltests/test_svn.py
spoorcc
force-pushed
the
svn-ssh-non-onteractive
branch
from
ce9644c to
5d550c7
Compare
spoorcc
force-pushed
the
svn-ssh-non-onteractive
branch
from
5d550c7 to
1b58c1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@dfetch/vcs/svn.py`:
- Line 71: Replace the redundant str() wrapper around the decoded stdout: change
the return of "return str(result.stdout.decode())" to simply return the decoded
string (i.e., return result.stdout.decode()). This removes the unnecessary
conversion while preserving existing behavior in the function containing that
return expression.
- Around line 135-138: list_of_tags() currently calls _run_svn directly and can
raise SubprocessCommandError if the tags/ path is missing; update list_of_tags
to mirror list_of_branches by wrapping the _run_svn call in a try/except
SubprocessCommandError block (catch the exception and return an empty list),
keeping the same output parsing logic (strip "/\r" and split by "\n") and
referencing list_of_tags, list_of_branches, _run_svn, and SubprocessCommandError
to locate where to add the handler.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 57862597-74b9-4d2e-a123-1c822ff2b03b
📒 Files selected for processing (5)
.readthedocs.ymlCHANGELOG.rstdfetch/vcs/svn.pytests/test_svn.pytests/test_svn_vcs.py
spoorcc
force-pushed
the
svn-ssh-non-onteractive
branch
from
1b58c1d to
0cc082e
Compare
|
@coderabbitai resume |
|
✅ Action performedReviews resumed. Review finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_svn.py (1)
538-671:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd Google-style docstrings to the new test functions.
The newly added tests in this block are missing function docstrings, which violates the test-style guideline.
📝 Example pattern
def test_ssh_hint_includes_hostname(): + """Raises a known-hosts hint that includes the parsed SSH hostname.""" stderr = "Host key verification failed."As per coding guidelines, “
tests/**/*.py: Docstrings in test functions should follow Google style convention.”🤖 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 `@tests/test_svn.py` around lines 538 - 671, Several new test functions lack Google-style docstrings; add a one- to two-sentence Google-style docstring to each test function (e.g., test_svn_remote_raises_hint_on_ssh_host_key_failure, test_get_info_from_target_raises_hint_on_ssh_host_key_failure, test_svn_repo_raises_hint_on_ssh_host_key_failure, test_ssh_hint_includes_hostname, test_ssh_hint_includes_user_when_present_in_url, test_svn_ssh_env_has_batch_mode, test_svn_ssh_env_preserves_existing_batch_mode, test_run_svn_passes_non_interactive_env_to_subprocess, test_ssh_hint_on_authenticity_of_host_message, test_ssh_hint_without_url_omits_hostname_commands, test_browse_tree_raises_hint_on_ssh_host_key_failure, test_create_diff_handles_non_utf8_diff_output) that describes the test purpose and any important conditions; follow Google docstring style (short summary, optional blank line, and brief notes on the scenario/assertions) and place it immediately under each def line.Source: Coding guidelines
🤖 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 @.claude/settings.json:
- Around line 23-24: The allowlist entries "Bash(pip install *)" and
"Bash(pytest *)" (and the similar wildcard entries around lines 28–31) are too
permissive; replace these star-based patterns with least-privilege shapes by
enumerating allowed package/flag patterns or using constrained argument regexes
instead of "*" — e.g., restrict "pip install" to specific package
names/versions, constrain "pytest" to specific test paths/flags, and tighten any
"dfetch" or other Bash(...) entries to explicit, validated argument forms so
arbitrary installs/tests/commands cannot be executed.
In @.readthedocs.yml:
- Line 22: The post_checkout step currently suppresses all errors by running
"git fetch --unshallow || true"; change it to only run git fetch --unshallow
when the repo is actually shallow (e.g., test git rev-parse
--is-shallow-repository or inspect .git/shallow) so real fetch failures are not
hidden; update the post_checkout sequence to conditionally execute "git fetch
--unshallow" when shallow, otherwise skip it, keeping the step name
(post_checkout) and the command string ("git fetch --unshallow") so reviewers
can find and verify the change.
In `@tests/test_svn.py`:
- Around line 543-544: Update the host-key failure tests to assert the specific
SshHostKeyError instead of a generic RuntimeError: replace
pytest.raises(RuntimeError, match="known hosts") with
pytest.raises(SshHostKeyError, match="known hosts") for the calls invoking
getattr(SvnRemote(url), method)(*call_args) (and apply the same change to the
other test occurrences referenced), ensuring the tests import or reference the
SshHostKeyError class used by the SvnRemote implementation.
---
Outside diff comments:
In `@tests/test_svn.py`:
- Around line 538-671: Several new test functions lack Google-style docstrings;
add a one- to two-sentence Google-style docstring to each test function (e.g.,
test_svn_remote_raises_hint_on_ssh_host_key_failure,
test_get_info_from_target_raises_hint_on_ssh_host_key_failure,
test_svn_repo_raises_hint_on_ssh_host_key_failure,
test_ssh_hint_includes_hostname,
test_ssh_hint_includes_user_when_present_in_url,
test_svn_ssh_env_has_batch_mode, test_svn_ssh_env_preserves_existing_batch_mode,
test_run_svn_passes_non_interactive_env_to_subprocess,
test_ssh_hint_on_authenticity_of_host_message,
test_ssh_hint_without_url_omits_hostname_commands,
test_browse_tree_raises_hint_on_ssh_host_key_failure,
test_create_diff_handles_non_utf8_diff_output) that describes the test purpose
and any important conditions; follow Google docstring style (short summary,
optional blank line, and brief notes on the scenario/assertions) and place it
immediately under each def line.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 52bb39b5-6274-45b5-9c87-fd00a910db43
📒 Files selected for processing (8)
.claude/settings.json.readthedocs.ymlCHANGELOG.rstdfetch/vcs/svn.pydoc/howto/troubleshooting.rstfeatures/check-svn-repo.featuretests/test_svn.pytests/test_svn_vcs.py
| "Bash(pip install *)", | ||
| "Bash(pytest *)", |
There was a problem hiding this comment.
Restrict wildcard Bash permissions to least privilege.
These *-based entries materially widen what the assistant can execute and weaken the allowlist boundary. For high-risk commands (pip install, broad pytest, and unconstrained dfetch args), prefer explicit command/arg shapes.
🔧 Suggested tightening example
- "Bash(pip install *)",
- "Bash(pytest *)",
+ "Bash(pip install -e .[docs])",
+ "Bash(pytest tests/*)",Also applies to: 28-31
🤖 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 @.claude/settings.json around lines 23 - 24, The allowlist entries "Bash(pip
install *)" and "Bash(pytest *)" (and the similar wildcard entries around lines
28–31) are too permissive; replace these star-based patterns with
least-privilege shapes by enumerating allowed package/flag patterns or using
constrained argument regexes instead of "*" — e.g., restrict "pip install" to
specific package names/versions, constrain "pytest" to specific test
paths/flags, and tighten any "dfetch" or other Bash(...) entries to explicit,
validated argument forms so arbitrary installs/tests/commands cannot be
executed.
0cc082e to
5fd2026
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@dfetch/vcs/git.py`:
- Around line 447-454: The current logic treats a globbed src (e.g. "vendor/*")
as a literal when strip_glob_prefix returns the same path, causing matched-root
submodules like "vendor/libfoo" to be added to to_remove; fix by detecting when
src is a glob pattern and, in that case, test the submodule path against the
glob (using fnmatch.fnmatch or Path.match) and treat a match as within_src
instead of removal. Update the block around strip_glob_prefix so that when
new_path == submodule.path you check "if is_glob(src) and
fnmatch.fnmatch(submodule.path, src): within_src.append(submodule) else if
Path(src).is_relative_to(Path(submodule.path)): continue else:
to_remove.add(submodule.path)"; keep existing names strip_glob_prefix,
submodule.path, within_src, to_remove and reuse Path/ fnmatch APIs.
In `@pyproject.toml`:
- Line 49: The dependency lines "cyclonedx-python-lib==11.10.0" and
"cyclonedx-python-lib[json-validation]==11.10.0" are correctly pinned but v11
has a known JSON-validation performance issue; update the repo to document this
risk and prevent CI regressions by either (a) adding a short comment next to
those dependency entries in pyproject.toml noting the JSON-validation
performance caveat, and/or (b) changing CI/test job(s) that install the
json-validation extra to skip or limit large-JSON validation workloads (or
install the plain package instead) so tests won’t be impacted.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 7c37646c-3122-4507-b2d2-2757ad0e98cc
📒 Files selected for processing (17)
.github/workflows/build.yml.readthedocs.ymlCHANGELOG.rstdfetch/log.pydfetch/util/license.pydfetch/vcs/git.pydfetch/vcs/svn.pydoc/conf.pydoc/dfetch_preamble.incdoc/howto/troubleshooting.rstfeatures/check-svn-repo.featurefeatures/fetch-git-repo-with-submodule.featurefeatures/report-sbom.featurepyproject.tomltests/test_git_vcs.pytests/test_svn.pytests/test_svn_vcs.py
| new_path = strip_glob_prefix(submodule.path, src) | ||
| if new_path != submodule.path: | ||
| submodule.path = new_path | ||
| within_src.append(submodule) | ||
| else: | ||
| if Path(src).is_relative_to(Path(submodule.path)): | ||
| continue | ||
| to_remove.add(submodule.path) |
There was a problem hiding this comment.
Glob src roots can be incorrectly deleted before promotion.
When src is a glob (for example vendor/*) and a submodule is exactly at a matched root (vendor/libfoo), strip_glob_prefix returns unchanged, then Line 452 treats the glob literally and falls through to to_remove. That removes an in-scope directory before _move_src_folder_up runs.
💡 Proposed fix
+import fnmatch
import glob
@@
new_path = strip_glob_prefix(submodule.path, src)
if new_path != submodule.path:
submodule.path = new_path
within_src.append(submodule)
else:
+ # Keep submodule directories that match the src glob root itself.
+ # They are needed for _move_src_folder_up promotion.
+ if fnmatch.fnmatch(submodule.path, src):
+ continue
if Path(src).is_relative_to(Path(submodule.path)):
continue
to_remove.add(submodule.path)📝 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.
| new_path = strip_glob_prefix(submodule.path, src) | |
| if new_path != submodule.path: | |
| submodule.path = new_path | |
| within_src.append(submodule) | |
| else: | |
| if Path(src).is_relative_to(Path(submodule.path)): | |
| continue | |
| to_remove.add(submodule.path) | |
| import fnmatch | |
| import glob | |
| new_path = strip_glob_prefix(submodule.path, src) | |
| if new_path != submodule.path: | |
| submodule.path = new_path | |
| within_src.append(submodule) | |
| else: | |
| # Keep submodule directories that match the src glob root itself. | |
| # They are needed for _move_src_folder_up promotion. | |
| if fnmatch.fnmatch(submodule.path, src): | |
| continue | |
| if Path(src).is_relative_to(Path(submodule.path)): | |
| continue | |
| to_remove.add(submodule.path) |
🤖 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 `@dfetch/vcs/git.py` around lines 447 - 454, The current logic treats a globbed
src (e.g. "vendor/*") as a literal when strip_glob_prefix returns the same path,
causing matched-root submodules like "vendor/libfoo" to be added to to_remove;
fix by detecting when src is a glob pattern and, in that case, test the
submodule path against the glob (using fnmatch.fnmatch or Path.match) and treat
a match as within_src instead of removal. Update the block around
strip_glob_prefix so that when new_path == submodule.path you check "if
is_glob(src) and fnmatch.fnmatch(submodule.path, src):
within_src.append(submodule) else if
Path(src).is_relative_to(Path(submodule.path)): continue else:
to_remove.add(submodule.path)"; keep existing names strip_glob_prefix,
submodule.path, within_src, to_remove and reuse Path/ fnmatch APIs.
| "semver==3.0.4", | ||
| "patch-ng==1.19.1", | ||
| "cyclonedx-python-lib==11.8.0", | ||
| "cyclonedx-python-lib==11.10.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest version of cyclonedx-python-lib and are there any known issues with version 11.10.0?
💡 Result:
As of June 13, 2026, the latest released version of cyclonedx-python-lib is 11.10.0, which was released on June 11, 2026 [1]. Regarding version 11.10.0, there are no specific, widely documented critical issues or regressions identified as of today. However, users of the v11.x series should be aware of a long-standing performance concern regarding JSON validation for very large files [2]. This performance issue was documented in issue #862 [2], which noted that validation speed significantly decreased in version 11 due to a change in the underlying JSON validator library [2]. While ongoing maintenance and updates have occurred throughout the 11.x release cycle, it is recommended to monitor the official CycloneDX GitHub repository for any bug reports or performance updates specific to the latest release [3][2].
Citations:
- 1: https://www.piwheels.org/project/cyclonedx-python-lib/
- 2: [PERFORMANCE]: v11 JSON validation with large files is utterly slow CycloneDX/cyclonedx-python-lib#862
- 3: https://github.com/CycloneDX/cyclonedx-python-lib/blob/main/CHANGELOG.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant sections around the referenced lines
sed -n '35,120p' pyproject.toml | nl -ba | sed -n '10,90p'Repository: dfetch-org/dfetch
Length of output: 104
🌐 Web query:
cyclonedx-python-lib 11.10.0 changelog json-validation issue
💡 Result:
As of June 13, 2026, version 11.10.0 of cyclonedx-python-lib has been released [1]. There are no specific reports or changelog entries in the project's official documentation or issue tracker documenting a new "json-validation issue" introduced specifically in version 11.10.0 [2][3][4][5]. The library has a known, long-standing performance issue regarding JSON validation for large files, which was tracked in issue #862 [6]. This performance degradation was noted as originating in the v11 release series following a transition to different JSON validation libraries [6]. If you are experiencing a validation error in version 11.10.0, it may be related to: 1. The known performance overhead when validating large JSON files containing numerous format-checked fields (e.g., iri-reference) [6]. 2. Limitations in the abstraction of validation error messages, which has been a reported concern for developers trying to present clear error feedback to end-users [7]. For further investigation, you can review the official repository's issues and release notes at https://github.com/CycloneDX/cyclonedx-python-lib/issues and https://github.com/CycloneDX/cyclonedx-python-lib/releases [5][6].
Citations:
- 1: https://www.piwheels.org/project/cyclonedx-python-lib/
- 2: https://github.com/CycloneDX/cyclonedx-python-lib/blob/main/CHANGELOG.md
- 3: https://cyclonedx-python-library.readthedocs.io/en/stable/changelog.html
- 4: https://cyclonedx-python-library.readthedocs.io/en/latest/changelog.html
- 5: https://github.com/CycloneDX/cyclonedx-python-lib/releases/tag/v11.7.0
- 6: [PERFORMANCE]: v11 JSON validation with large files is utterly slow CycloneDX/cyclonedx-python-lib#862
- 7: Validation errors are hard to present safely to the user (missing abstraction) CycloneDX/cyclonedx-python-lib#827
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
path="pyproject.toml"
start=40
end=110
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i}:{line.rstrip()}")
PYRepository: dfetch-org/dfetch
Length of output: 2568
🌐 Web query:
cyclonedx-python-lib 11.10.0 release notes known issues json-validation
💡 Result:
As of June 13, 2026, cyclonedx-python-lib version 11.10.0 has been released [1]. Official release notes detailing specific changes for version 11.10.0 were not indexed in public repositories as of the time of this search; however, the library continues to address ongoing validation improvements [2]. Regarding JSON validation, the library has experienced a known performance limitation since the transition to non-GPL JSON validators in the v11 release series [3]. Specifically, validating large JSON files can be slow due to the performance characteristics of the underlying validator library when processing certain format checks (such as iri-reference) [3]. To address usability concerns related to schema validation, recent updates (including those leading up to v11.10.0) have focused on improving the presentation of validation errors [4][2]. Notable efforts include: - Enriching ValidationError objects with clearer message and path properties [2][5]. - Implementing mechanisms to squeeze or limit the size of error messages for better readability [5]. - Providing capabilities to iterate over all validation errors rather than just reporting the first failure [5]. - Normalizing validation failure reporting across different formats (JSON and XML) [2]. For the most accurate and up-to-date information on 11.10.0-specific changes, it is recommended to check the official GitHub releases page [6].
Citations:
- 1: https://www.piwheels.org/project/cyclonedx-python-lib/
- 2: feat(validation): improve user-facing schema validation errors CycloneDX/cyclonedx-python-lib#940
- 3: [PERFORMANCE]: v11 JSON validation with large files is utterly slow CycloneDX/cyclonedx-python-lib#862
- 4: Validation errors are hard to present safely to the user (missing abstraction) CycloneDX/cyclonedx-python-lib#827
- 5: feat: schema based validation may return iterable of all errors CycloneDX/cyclonedx-python-lib#834
- 6: https://github.com/CycloneDX/cyclonedx-python-lib/releases
cyclonedx-python-lib 11.10.0 — released; main known risk is v11 JSON-validation performance
pyproject.tomlalready pinscyclonedx-python-lib==11.10.0(line 49) andcyclonedx-python-lib[json-validation]==11.10.0(line 102).11.10.0is published/released; no widely documented critical issues specific to11.10.0were found in public release/changelog/issue references.- Still, v11.x has a known JSON-validation performance issue for large JSON inputs (tracked as CycloneDX issue
#862), so ensure your CI/test workloads won’t be materially impacted when using thejson-validationextra.
🤖 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 `@pyproject.toml` at line 49, The dependency lines
"cyclonedx-python-lib==11.10.0" and
"cyclonedx-python-lib[json-validation]==11.10.0" are correctly pinned but v11
has a known JSON-validation performance issue; update the repo to document this
risk and prevent CI regressions by either (a) adding a short comment next to
those dependency entries in pyproject.toml noting the JSON-validation
performance caveat, and/or (b) changing CI/test job(s) that install the
json-validation extra to skip or limit large-JSON validation workloads (or
install the plain package instead) so tests won’t be impacted.
spoorcc
force-pushed
the
svn-ssh-non-onteractive
branch
from
92d25cd to
98b653b
Compare
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores