Skip to content

feat(skills): add 4090:verify — W0 substrate artifact check - #1601

Merged
POWERFULMOVES merged 1 commit into
mainfrom
feat/node-4090-verify-skill
May 25, 2026
Merged

POWERFULMOVES merged 1 commit into
mainfrom
feat/node-4090-verify-skill

Conversation

@POWERFULMOVES

@POWERFULMOVES POWERFULMOVES commented May 25, 2026

Copy link
Copy Markdown
Owner

Summary

Test plan

  • Run the embedded bash block from repo root on main — expect 3 lines and ALL CHECKS PASSED
  • Confirm skill appears in skills list as node-4090-verify

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added comprehensive verification runbook for workflow validation procedures, documenting all required validation checks, automated helper utilities, detailed expected output specifications, step-by-step troubleshooting guides, and comprehensive fix recipes that map each failure scenario to specific remediation actions, enabling teams to efficiently manage operational compliance and successfully resolve validation issues.

Review Change Stack

7-point verification skill: SKILL.md no BLOCKED notices, unifi_topology
wired in json-to-profile.py, TAC probe-wire status=done, hydrated
credentials for GH_PAT_PUBLISH/ANTHROPIC_API_KEY/GHCR_USERNAME, and
local secrets bundle exists at APPDATA host config dir.

Codifies the post-PR-1599 + post-secrets-funnel verification workflow
so any W0 merge or funnel run can be confirmed in one invocation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR adds a verification SKILL runbook for the 4090:verify workflow. The new file defines three artifact-level checks to confirm required deliverables are present on main after a W0 PR merge: absence of BLOCKED notices in the node-4090-probe SKILL.md, unifi_topology wiring in json-to-profile.py, and probe-wire status completion in the TAC config. It includes bash-based check logic, expected output documentation, and a fix recipes table for remediation.

Changes

4090 Verification Runbook

Layer / File(s) Summary
Artifact verification checks and fix recipes
.claude/skills/node-4090-verify/SKILL.md
Verification SKILL runbook defining three post-merge artifact checks with bash check() helper logic, expected passing output, and fix recipes table mapping failure modes to remediation actions.

Possibly related PRs

  • POWERFULMOVES/PMOVES.AI#1599: Introduces and updates the unifi_topology wiring, BLOCKED removal in node-4090-probe SKILL.md, and probe-wire: status: done TAC entry that are verified by this runbook.
  • POWERFULMOVES/PMOVES.AI#1536: Introduces the node-4090-probe deliverable and 4090 laptop TAC probe wiring that this verification SKILL explicitly targets.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A runbook hops through checks so neat,
Three artifacts make the merge complete,
With bash and recipes, no BLOCKED in sight,
The 4090 node is verified right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description covers the summary and test plan sections from the template. However, it lacks required sections: Testing with command outputs, Required Checks checklist, Review Coordination, and Follow-up Tasks. Complete the missing required sections: add Testing section with actual command outputs, check Required Checks items, specify if Codex/Copilot reviews are needed, and add Follow-up Tasks if applicable.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: adding a new skill called '4090:verify' for W0 substrate artifact verification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/node-4090-verify-skill

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@POWERFULMOVES
POWERFULMOVES merged commit 1ab8298 into main May 25, 2026
12 of 15 checks passed
@POWERFULMOVES
POWERFULMOVES deleted the feat/node-4090-verify-skill branch May 25, 2026 06:27

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 169ec7c5ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}

# 1. node-4090-probe SKILL.md: no BLOCKED notice
BLOCKED=$(grep -c "BLOCKED" .claude/skills/node-4090-probe/SKILL.md 2>/dev/null || echo "99")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make BLOCKED count check succeed on zero matches

The first verification check is currently a false-negative in the healthy case: grep -c prints 0 when no BLOCKED lines are present but exits with status 1, so the || echo "99" branch also runs and BLOCKED becomes a multi-line value (0\n99). That makes [ "$BLOCKED" = "0" ] fail even when the probe skill has no BLOCKED notice, so this script reports failure on valid main branches.

Useful? React with 👍 / 👎.


# 3. TAC: probe-wire status=done (PR #1599)
check "TAC: probe-wire status=done" \
"grep -A8 'probe-wire' pmoves/configs/tac_trees/node-4090-laptop.tac.yaml | grep -q 'status: done'"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Search full probe-wire block for status instead of -A8

The TAC check can fail even when probe-wire is done because it only scans 8 lines after the probe-wire match. In pmoves/configs/tac_trees/node-4090-laptop.tac.yaml, status: done is farther down due the multiline instruction block, so this command misses it and emits a false failure. As written, the skill’s expected “ALL CHECKS PASSED” output is not achievable on the current valid TAC layout.

Useful? React with 👍 / 👎.

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.

1 participant