Repository navigation
feat(tools/vcs): implement the Subversion backend - #1609
Merged
Merged
Conversation
`magpie-vcs` detected SVN working copies but every operation raised, so skills could not drive the svn.apache.org source trees or any other SVN checkout through the source-control capability. SubversionBackend now binds each abstract operation to `svn`, following the centralized model already documented in tools/asf-svn/source-control.md: branches are server-side copies under the trunk/branches/tags layout, `stage` schedules adds and deletes, `commit` writes every working-copy change straight to the server (there is no staging area), `fetch` is a no-op because history reads query the server, and `push` refuses because the commit already published. `log` normalises to one `r<rev> <subject>` line per revision, and `cat` pegs the URL so moved or deleted paths resolve. All svn calls run `--non-interactive` so a missing credential fails instead of blocking on a prompt. With no extension-point backend left, `_UnimplementedBackend` is removed, and the docs that listed SVN as pending now describe the binding. Generated-by: Claude Code (Opus 5.5)
The svn backend relies on `info --show-item` and `cleanup --remove-unversioned` (Subversion 1.9+); an older svn now reports as unavailable and every operation fails with an actionable error instead of an opaque command failure. `new-branch` takes one path segment, rejects empty, `.` and `..` names, and URL-quotes the name; `switch` applies the same segment check to layout-relative refs. The README now says commit sends modified and staged paths, not unversioned files. Generated-by: Claude Code (Opus 5.5)
A project nested under a directory named branches or tags (e.g. ^/branches/projects/foo/trunk) was split at the first marker, so new-branch and switch targeted the wrong server path. The rightmost marker now wins. The README states the supported layout, the one-segment branch names and recovery from a partial new-branch, and the svn tests skip unless the backend reports svn 1.9+ available. Generated-by: Claude Code (Opus 5.5)
zeroshade
added
enhancement
github-actions
Bot
added
contract:tracker
… names The rightmost layout scan matched `trunk` even when it was the name of a branch or tag, so a working copy on branches/trunk reported `trunk` and a later new-branch targeted branches/branches/<name>. A segment directly after branches/ or tags/ is now always the line name. Name-based switch now refuses a nested bare name, matching new-branch and the README. Generated-by: Claude Code (Opus 5.5)
zeroshade
removed
contract:tracker
github-actions
Bot
added
contract:tracker
svn info returns relative-url URL-encoded, so current_branch reported branches/fix%201 for a branch created as "fix 1", and passing that back to switch quoted it again (fix%25201). current_branch now decodes the name; URL construction keeps quoting exactly once. Generated-by: Claude Code (Opus 5.5)
zeroshade
removed
contract:tracker
potiuk
reviewed
Oct 13, 2026
potiuk
left a comment
potiuk
left a comment
Member
There was a problem hiding this comment.
The SVN mapping is sound and well scoped (the trunk/branches/tags parse, the version gate, refusing push, the documented commit-everything decision), and the new commit already fixes the branches/trunk misparse; the comments below are small and none blocks.
Smaller observations
- See inline comments on
tools/vcs/src/magpie_vcs/__init__.py:735,tools/vcs/README.md:126,tools/vcs/src/magpie_vcs/__init__.py:649,tools/vcs/src/magpie_vcs/__init__.py:675,tools/vcs/README.md:133.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The findings
below are observations, not blockers; an Apache Magpie
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.More on how Apache Magpie handles maintainer review:
Contributing guide.
- stage: test paths with os.path.lexists, so a versioned symlink whose target is missing is not scheduled for deletion. - diff --base: the svn backend takes a revision; a line-of-work name (trunk, main, a branch) now fails with a clear VCSError instead of svn's revision syntax error. - README: on svn the change goes public at commit / new-branch, so pre-publish gates belong there; -n does not bound the server request when log filters are set; CLI callers detect the centralized model via `magpie-vcs detect` printing svn. Generated-by: Claude Code (Opus 5.5)
cat pegged <url>/<path>@<rev>, so on a branch a revision from before the branch existed failed (E200009): the branch directory did not exist at that revision. cat now asks svn for the working-copy path at the operative revision, which follows copy history back through trunk, and falls back to the pegged URL only when the path is no longer in the working copy. Generated-by: Claude Code (Opus 5.5)
For a path no longer in the working copy, cat fell back to <branch-url>/<path>@<rev>, which cannot reach a revision from before the branch existed. It now finds the commit on this line of work that deleted the path (or a parent) and reads it through the revision before that, so copy history is still followed back through trunk. Generated-by: Claude Code (Opus 5.5)
stage only checked whether each argument existed, so `stage pkg` or `stage .` left versioned files deleted inside the directory as missing (!) instead of scheduling their deletion, unlike `git add <dir>`. stage now reads `svn status --xml` for the present paths and runs svn delete on every missing entry before svn add. Generated-by: Claude Code (Opus 5.5)
github-actions
Bot
added
contract:tracker
potiuk
pushed a commit
to Kaap10/magpie
that referenced
this pull request
Oct 16, 2026
* feat(tools/vcs): implement the Subversion backend (apache#602) `magpie-vcs` detected SVN working copies but every operation raised, so skills could not drive the svn.apache.org source trees or any other SVN checkout through the source-control capability. SubversionBackend now binds each abstract operation to `svn`, following the centralized model already documented in tools/asf-svn/source-control.md: branches are server-side copies under the trunk/branches/tags layout, `stage` schedules adds and deletes, `commit` writes every working-copy change straight to the server (there is no staging area), `fetch` is a no-op because history reads query the server, and `push` refuses because the commit already published. `log` normalises to one `r<rev> <subject>` line per revision, and `cat` pegs the URL so moved or deleted paths resolve. All svn calls run `--non-interactive` so a missing credential fails instead of blocking on a prompt. With no extension-point backend left, `_UnimplementedBackend` is removed, and the docs that listed SVN as pending now describe the binding. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): check svn version and validate branch names The svn backend relies on `info --show-item` and `cleanup --remove-unversioned` (Subversion 1.9+); an older svn now reports as unavailable and every operation fails with an actionable error instead of an opaque command failure. `new-branch` takes one path segment, rejects empty, `.` and `..` names, and URL-quotes the name; `switch` applies the same segment check to layout-relative refs. The README now says commit sends modified and staged paths, not unversioned files. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): resolve svn layout at the rightmost marker A project nested under a directory named branches or tags (e.g. ^/branches/projects/foo/trunk) was split at the first marker, so new-branch and switch targeted the wrong server path. The rightmost marker now wins. The README states the supported layout, the one-segment branch names and recovery from a partial new-branch, and the svn tests skip unless the backend reports svn 1.9+ available. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): read branches/trunk as a branch; refuse nested switch names The rightmost layout scan matched `trunk` even when it was the name of a branch or tag, so a working copy on branches/trunk reported `trunk` and a later new-branch targeted branches/branches/<name>. A segment directly after branches/ or tags/ is now always the line name. Name-based switch now refuses a nested bare name, matching new-branch and the README. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): report svn branch names decoded svn info returns relative-url URL-encoded, so current_branch reported branches/fix%201 for a branch created as "fix 1", and passing that back to switch quoted it again (fix%25201). current_branch now decodes the name; URL construction keeps quoting exactly once. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): address apache#1609 review comments - stage: test paths with os.path.lexists, so a versioned symlink whose target is missing is not scheduled for deletion. - diff --base: the svn backend takes a revision; a line-of-work name (trunk, main, a branch) now fails with a clear VCSError instead of svn's revision syntax error. - README: on svn the change goes public at commit / new-branch, so pre-publish gates belong there; -n does not bound the server request when log filters are set; CLI callers detect the centralized model via `magpie-vcs detect` printing svn. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): make svn cat follow copy history cat pegged <url>/<path>@<rev>, so on a branch a revision from before the branch existed failed (E200009): the branch directory did not exist at that revision. cat now asks svn for the working-copy path at the operative revision, which follows copy history back through trunk, and falls back to the pegged URL only when the path is no longer in the working copy. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): svn cat follows history for paths deleted on a branch For a path no longer in the working copy, cat fell back to <branch-url>/<path>@<rev>, which cannot reach a revision from before the branch existed. It now finds the commit on this line of work that deleted the path (or a parent) and reads it through the revision before that, so copy history is still followed back through trunk. Generated-by: Claude Code (Opus 5.5) * fix(tools/vcs): svn stage schedules deletions under a staged directory stage only checked whether each argument existed, so `stage pkg` or `stage .` left versioned files deleted inside the directory as missing (!) instead of scheduling their deletion, unlike `git add <dir>`. stage now reads `svn status --xml` for the present paths and runs svn delete on every missing entry before svn add. Generated-by: Claude Code (Opus 5.5)
Merged
12 of 14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
magpie-vcsdetected SVN working copies but every operation raised, so skills could not drivesvn.apache.orgtrees (or any SVN checkout) through the source-control capability.SubversionBackendnow binds every abstract operation tosvn, following the centralized model already documented intools/asf-svn/source-control.md.trunk/branches/<name>/tags/<name>(rightmost marker wins);stage=svn add/svn delete;commitwrites every modified or staged path straight to the server (no staging area);fetchis a no-op (history reads query the server);pushrefuses (the commit already published).logemitsr<rev> <subject>,catpegs the URL so moved/deleted paths resolve. All calls run--non-interactive; SVN 1.9+ is enforced._UnimplementedBackendis removed (no extension-point backend left); docs and the adapters spec that listed SVN as pending now describe the binding. The rest of feat(tools/subversion): Apache Subversion (SVN) support — ASF svn.apache.org / dist.apache.org #602 (dist.apache.org helpers, svn recipes) already lives intools/asf-svn/.Type of change
tools/*/withpyproject.toml)tools/<system>/*.md)docs/,README.md,CONTRIBUTING.md)Test plan
tools/vcs:uv run pytest(40 passed; 4 hg tests skip withouthg),ruff check,ruff format --check,mypypass. SVN tests run against localfile://repos built withsvnadminmagpie-vcsdetect/branch/log -n/log --since/clean/cat/pushagainst a realsvn.apache.orgcheckout (comdev/projects.apache.org/trunk)container-gateway/symlink-lint, fixed separately in test: isolate container-gateway and symlink-lint fixtures from the host #1608RFC-AI-0004 compliance
new-branch,stage,commit) stay gated in the calling skill; the tool adds no prompt, as with the other backendstools/vcsREADME (svnlog/cat/diff --base/new-branch/switch/commitreach the server)Linked issues
Closes #602
Notes for reviewers
commitsends the whole working copy (SVN has no staging area; changelists break on new files in new directories and on deletions);fetchno-op andpusherror rather than both silent.blamehas no abstract operation for any backend (docs point atsvn blame); externals, sparse and mixed-revision working copies are not specially handled; nested branch names are refused.tools/spec-loop/.last-syncis 44 commits behindmain;specs/adapters.mdis updated here but.last-syncis not bumped, since that would claim the other commits are synced.Generative AI disclosure
Drafted with Claude Code (Opus 5.5) and reviewed by the author; each commit carries a
Generated-by:trailer.