Skip to content

chore(skills): remove the stale test-pr-devnet skill - #564

Merged
MegaRedHand merged 1 commit into
mainfrom
chore/remove-test-pr-devnet-skill
Aug 4, 2026
Merged

chore(skills): remove the stale test-pr-devnet skill#564
MegaRedHand merged 1 commit into
mainfrom
chore/remove-test-pr-devnet-skill

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

Removes .claude/skills/test-pr-devnet and repoints the two CLAUDE.md references at the skills that replaced it.

Why

devnet-runner and devnet-log-review already cover what this skill did:

test-pr-devnet did now covered by
Start a multi-client local devnet devnet-runner (node roster, timeout, automatic log dump)
Patch client-cmds/*-cmd.sh image tags devnet-runner
pause/unpause sync testing devnet-runner/references/instability-testing.md
Status checks and log greps devnet-log-review scripts

What was left had drifted from the current local setup:

  • It hardcoded LEAN_QUICKSTART=/Users/mega/lean_consensus/lean-quickstart, while devnet-runner expects lean-quickstart at the repo root.
  • Its readiness loop waited for exactly four _0 containers and its checks grepped zeam_0/ream_0/qlean_0, so every health check and the whole result summary reported nothing against the all-ethlambda validator-config.yaml the local devnet uses now.
  • Its scripts predate the Docker log caps that the newer skill scripts set.

It was last touched functionally in #414; #532 only swept it for docs consistency.

Not carried over

The one capability with no replacement is test-branch.sh's build-tag-patch-restore step: build the current branch as ghcr.io/lambdaclass/ethlambda:<branch>, point ethlambda-cmd.sh at it, and restore the backup afterwards. make docker-build only produces :local. Left out deliberately; worth adding to devnet-runner as a small script if branch-image testing comes up again.

devnet-runner and devnet-log-review now cover everything this skill did
except building a branch-specific Docker image: node roster and image tag
selection, pause/unpause instability testing, and log analysis.

What remained had drifted from the current local setup. It hardcoded a
lean-quickstart path outside the repo, waited for exactly four `_0`
containers, and grepped zeam_0/ream_0/qlean_0, so every health check and
its whole result summary reported nothing against the all-ethlambda
validator config the local devnet uses now.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

This confirms the PR's claims accurately reflect devnet-runner's capabilities (validator config/node roster, image tags, timeout-based runs, log dumping) and instability-testing.md covers pause/unpause. The review is straightforward since this is a docs/skill-removal chore, not code.

Review: PR 564 — chore(skills): remove the stale test-pr-devnet skill

This is a documentation/tooling-only change (deletes an internal .claude/skills/ skill and its scripts, updates two CLAUDE.md references). No Rust source, consensus logic, networking, or storage code is touched, so the consensus-layer review checklist doesn't apply here.

Findings:

  1. Accurate replacement mapping. Verified locally that devnet-runner (SKILL.md) actually covers node roster selection via validator-config.yaml, image tag updates via client-cmds/*-cmd.sh, and timeout-based runs with automatic log dumping — matching the PR description's claims. devnet-runner/references/instability-testing.md exists and covers pause/unpause, so that claim also checks out.

  2. CLAUDE.md edit is correct and minimal. The removed line pointing at test-pr-devnet/scripts/test-branch.sh and the pointer under "Testing with Local Devnet" are both updated to the new skill paths (devnet-runner/SKILL.md, devnet-log-review/SKILL.md), which exist in the repo.

  3. Deletion is clean. All four files under .claude/skills/test-pr-devnet/ (SKILL.md, check-status.sh, cleanup.sh, test-branch.sh) are removed together — no orphaned references left elsewhere (only the two CLAUDE.md mentions existed, both updated).

  4. Gap is explicitly called out, not silently dropped. The PR description notes that test-branch.sh's build-tag-patch-restore step (building a branch-tagged Docker image and pointing ethlambda-cmd.sh at it) has no replacement yet, and defers it as future work rather than pretending full parity. That's the right call for a chore PR — no scope creep into building that feature now.

No correctness, security, or performance concerns — this is a well-scoped, low-risk cleanup with a clear rationale (stale hardcoded macOS path, drifted container-name assumptions) and accurate documentation updates. Looks good to merge.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

No findings.

This PR only removes the legacy .claude/skills/test-pr-devnet/* helper and updates CLAUDE.md to point at the replacement devnet-runner / devnet-log-review skills. I do not see any changes to Rust code, consensus logic, SSZ handling, signature verification, fork choice, or state transition paths, so there is no direct correctness, security, or performance impact on the client itself.

Residual risk is limited to developer workflow: if the new docs pointed to missing assets, local devnet testing would regress. I checked that .claude/skills/devnet-runner/SKILL.md and .claude/skills/devnet-log-review/SKILL.md exist, and I did not find stale references to the removed test-pr-devnet scripts in the repo. I did not execute the replacement devnet workflow, so that operational path remains unverified by this review.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

This PR removes the test-pr-devnet skill and updates documentation to reference replacement skills (devnet-runner and devnet-log-review). No consensus-critical code is modified.

Issues in Deleted Code (for awareness when migrating logic)

File: .claude/skills/test-pr-devnet/scripts/test-branch.sh

  • Line 67: Hardcoded absolute path /Users/mega/lean_consensus/lean-quickstart is not portable across developer environments. Use environment variables with fallbacks instead:

    LEAN_QUICKSTART="${LEAN_QUICKSTART:-${HOME}/lean_consensus/lean-quickstart}"
  • Lines 154-155: The sed -i.tmp pattern followed by rm "$ETHLAMBDA_CMD.tmp" works but creates a temporary backup file unnecessarily. Since you already create a .backup on line 152, use sed -i.bak and remove that, or use sed -i'' (portable empty backup) if you don't need the intermediate backup:

    sed -i.bak "s|ghcr.io/lambdaclass/ethlambda:[^ ]*|ghcr.io/lambdaclass/ethlambda:$BRANCH_NAME|" "$ETHLAMBDA_CMD"
    rm "$ETHLAMBDA_CMD.bak"
  • Line 229: Using xargs without arguments for string trimming is non-idiomatic and potentially unsafe if ERROR_COUNT contains spaces. Use bash parameter expansion instead:

    ERROR_COUNT=$(echo "$ERROR_COUNT" | tr -d '[:space:]')
    # Or simply:
    ERROR_COUNT="${ERROR_COUNT//[[:space:]]/}"

File: .claude/skills/test-pr-devnet/scripts/cleanup.sh

  • Line 5: Same hardcoded path issue as above.

CLAUDE.md Changes

The documentation updates (lines 92, 99-101) correctly remove the obsolete reference and point to the replacement skills. The new references assume devnet-runner and devnet-log-review skills exist in the repository structure.

Summary

  • Security: No security implications; only dev tooling removed
  • Correctness: Deletion is complete and consistent
  • Consensus impact: None (no Rust code modified)
  • Action: Verify that the replacement skills (devnet-runner, devnet-log-review) exist and don't replicate the hardcoded path portability issues noted above

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the stale test-pr-devnet skill and directs contributors to the existing replacement workflows.

  • Deletes the obsolete skill documentation and its status, cleanup, and branch-testing scripts.
  • Updates CLAUDE.md to reference devnet-runner for running devnets and devnet-log-review for log analysis.

Confidence Score: 5/5

The PR appears safe to merge because the deleted workflow has no remaining references and both replacement skill paths exist.

The change removes only stale development tooling and updates its two documentation references to replacement skills present in the committed HEAD tree.

Important Files Changed

Filename Overview
CLAUDE.md Removes the obsolete command and replaces its documentation link with valid references to the two replacement skills.
.claude/skills/test-pr-devnet/SKILL.md Deletes stale workflow documentation whose responsibilities are intentionally delegated to maintained skills.
.claude/skills/test-pr-devnet/scripts/test-branch.sh Removes the obsolete branch-image and devnet orchestration script without leaving tracked references to it.
.claude/skills/test-pr-devnet/scripts/check-status.sh Removes the stale status script now superseded by devnet log-review tooling.
.claude/skills/test-pr-devnet/scripts/cleanup.sh Removes cleanup logic coupled to the obsolete multi-client setup.

Reviews (1): Last reviewed commit: "chore(skills): remove the stale test-pr-..." | Re-trigger Greptile

@MegaRedHand
MegaRedHand merged commit 82b6c10 into main Aug 4, 2026
7 checks passed
@MegaRedHand
MegaRedHand deleted the chore/remove-test-pr-devnet-skill branch August 4, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants