Add cross-surface skill parity workflow - #38
Conversation
📝 WalkthroughWalkthroughAdds the ChangesSkill surface parity
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
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: 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 @.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py:
- Around line 146-162: Update the changed-path collection in the surrounding
sync flow to preserve raw Git paths: make git() retain unstripped output, use
NUL-delimited commands with -z and split on NUL for committed, staged, unstaged,
and untracked paths, and add --no-renames so renames produce separate add/delete
paths. Include T in every relevant --diff-filter to capture type changes, while
keeping empty entries excluded before classification.
- Around line 115-116: Update normalize_github_origin to avoid
str.removesuffix() if Python versions older than 3.9 remain supported, or
declare Python 3.9+ in the repository’s existing Python-version configuration
when that is the intended minimum. Keep the URL normalization behavior
unchanged.
🪄 Autofix
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: Enterprise
Run ID: 766d956d-38ef-4ab8-8476-c25b1221a2f4
📒 Files selected for processing (5)
.agents/skills/sync-coderabbit-skill-surfaces/SKILL.md.agents/skills/sync-coderabbit-skill-surfaces/agents/openai.yaml.agents/skills/sync-coderabbit-skill-surfaces/references/surface-contract.md.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.pyAGENTS.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
{.agents/skills/**,AGENTS.md}
📄 CodeRabbit inference engine (AGENTS.md)
Treat
.agents/skills/**and thisAGENTS.mdas maintainer-only; they do not require plugin companion PRs.
Files:
.agents/skills/sync-coderabbit-skill-surfaces/agents/openai.yamlAGENTS.md.agents/skills/sync-coderabbit-skill-surfaces/SKILL.md.agents/skills/sync-coderabbit-skill-surfaces/references/surface-contract.md.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: coderabbitai/skills
Timestamp: 2026-08-12T21:23:35.088Z
Learning: Run the parity planner even when no companion change seems necessary.
Learnt from: CR
Repo: coderabbitai/skills
Timestamp: 2026-08-12T21:23:35.088Z
Learning: For audit, diagnosis, or review-only tasks, report the parity plan without external writes.
Learnt from: CR
Repo: coderabbitai/skills
Timestamp: 2026-08-12T21:23:35.088Z
Learning: Never merge, mark ready, close, or overwrite an existing target PR through the parity workflow.
Learnt from: CR
Repo: coderabbitai/skills
Timestamp: 2026-08-12T21:23:35.088Z
Learning: Preserve target-specific safety behavior and stop on overlapping human work or an unmapped public surface.
🪛 ast-grep (0.45.1)
.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py
[error] 96-103: Command coming from incoming request
Context: subprocess.run(
["git", *args],
cwd=repo,
text=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[warning] 122-122: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.fullmatch(pattern, value)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[info] 454-454: use jsonify instead of json.dumps for JSON output
Context: json.dumps(plan, indent=2, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py
[warning] 97-104: Prefer capture_output over sending stdout and stderr to PIPE
Replace with capture_output keyword argument
(UP022)
[error] 97-97: subprocess call: check for execution of untrusted input
(S603)
[error] 98-98: Starting a process with a partial executable path
(S607)
[warning] 107-107: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 136-136: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 143-143: Boolean-typed positional argument in function definition
(FBT001)
[warning] 322-324: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
.agents/skills/sync-coderabbit-skill-surfaces/SKILL.md (1)
1-142: LGTM!.agents/skills/sync-coderabbit-skill-surfaces/references/surface-contract.md (1)
1-132: LGTM!AGENTS.md (1)
1-12: LGTM!.agents/skills/sync-coderabbit-skill-surfaces/agents/openai.yaml (1)
1-4: LGTM!.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py (1)
1-112: LGTM!Also applies to: 129-145, 167-465
| def normalize_github_origin(url: str) -> str | None: | ||
| value = url.strip().removesuffix("/") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI 'pyproject.toml|setup.cfg|tox.ini|\.python-version|\.tool-versions|Dockerfile' . \
-x sh -c 'echo "--- $1"; rg -n "requires-python|python_requires|python-version|PYTHON_VERSION|3\.[0-9]+" "$1" || true' sh {}Repository: coderabbitai/skills
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- tracked Python/tooling files"
git ls-files | rg '(^|/)(pyproject\.toml|setup\.cfg|tox\.ini|\.python-version|\.tool-versions|Dockerfile|requirements[^/]*(\.txt)?|Pipfile|Makefile|package\.json|.*\.ya?ml)$' || true
echo "--- Python version references"
rg -n -i --glob '!**/node_modules/**' --glob '!**/.git/**' \
'requires-python|python_requires|python-version|PYTHON_VERSION|setup-python|python3(\.[0-9]+)?|3\.[0-9]+' . || true
echo "--- target script context"
sed -n '1,180p' .agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py
echo "--- repository metadata"
git ls-files -s .agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.pyRepository: coderabbitai/skills
Length of output: 6498
Declare the supported Python version or replace str.removesuffix().
The repository has no Python version declaration. Declare Python 3.9 or later, or replace str.removesuffix("/") if older versions remain supported.
🤖 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 @.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py around
lines 115 - 116, Update normalize_github_origin to avoid str.removesuffix() if
Python versions older than 3.9 remain supported, or declare Python 3.9+ in the
repository’s existing Python-version configuration when that is the intended
minimum. Keep the URL normalization behavior unchanged.
| committed = git( | ||
| repo, | ||
| "diff", | ||
| "--name-only", | ||
| "--diff-filter=ACMRD", | ||
| f"{base_ref}...{head_ref}", | ||
| ) | ||
| paths.update(line for line in committed.splitlines() if line) | ||
|
|
||
| if include_worktree: | ||
| for args in ( | ||
| ("diff", "--name-only", "--diff-filter=ACMRD"), | ||
| ("diff", "--cached", "--name-only", "--diff-filter=ACMRD"), | ||
| ("ls-files", "--others", "--exclude-standard"), | ||
| ): | ||
| output = git(repo, *args) | ||
| paths.update(line for line in output.splitlines() if line) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve every changed Git path before classification.
git diff --name-only C-quotes path names that contain control characters. git() also strips the command output. A changed path such as skills/new\nSKILL.md then does not match PUBLIC_ROOTS and is classified as internal.
Git rename detection can also hide the old public path from --name-only output. The planner can report fully_mapped: true instead of stopping for an unmapped public surface.
Use -z and split on NUL bytes. Preserve raw output for path commands. Add --no-renames so a rename is classified as an add and a delete.
Proposed fix
-def git(repo: Path, *args: str, check: bool = True) -> str:
+def git(
+ repo: Path, *args: str, check: bool = True, strip: bool = True
+) -> str:
result = subprocess.run(
["git", *args],
cwd=repo,
text=True,
- stdout=subprocess.PIPE,
- stderr=subprocess.PIPE,
+ capture_output=True,
check=False,
)
...
- return result.stdout.strip()
+ return result.stdout.strip() if strip else result.stdout- "--name-only",
+ "--name-only",
+ "-z",
+ "--no-renames",
"--diff-filter=ACMRD",
f"{base_ref}...{head_ref}",
)
- paths.update(line for line in committed.splitlines() if line)
+ paths.update(path for path in committed.split("\0") if path)Apply the same NUL-delimited handling to staged, unstaged, and untracked paths. Add T to the diff filter if type changes can affect a published skill surface.
🤖 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 @.agents/skills/sync-coderabbit-skill-surfaces/scripts/surface_sync.py around
lines 146 - 162, Update the changed-path collection in the surrounding sync flow
to preserve raw Git paths: make git() retain unstripped output, use
NUL-delimited commands with -z and split on NUL for committed, staged, unstaged,
and untracked paths, and add --no-renames so renames produce separate add/delete
paths. Include T in every relevant --diff-filter to capture type changes, while
keeping empty entries excluded before classification.
1 participant
Summary
$sync-coderabbit-skill-surfacesmaintainer skillAGENTS.mdThe workflow distinguishes required, review, not-required, and capability-gap states. It refuses publishing for unmapped public skills, overlapping human PRs, unverified source provenance, unexpected target paths, or weakened platform safeguards.
This PR is maintainer-only, so its own parity plan correctly requires no Cursor or Codex companion PR.
Validation
quick_validate.py .agents/skills/sync-coderabbit-skill-surfacesgit diff --checkSummary by CodeRabbit
New Features
Documentation