chore(update-stack): rework skill as 2-phase ISO merge, remove stack-maintainer agent#3177
Merged
PierreBrisorgueil merged 6 commits intomasterfrom Mar 2, 2026
Merged
Conversation
…-maintainer agent - Phase 1: ISO merge with explicit --theirs rules for stack modules and lib - Phase 2: project alignment via MIGRATION.md + tasks reference module - Remove stack-maintainer agent (guardrails covered by CLAUDE.md + skill) - Update CLAUDE.md: remove agent reference
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoves the Stack Maintainer Agent docs and refactors the update-stack skill into a two‑phase ISO-merge + project-alignment workflow, updating CLAUDE.md to drop the embedded agent reference and reflect only embedded settings and skills. (45 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant LocalRepo as "Local Repo"
participant Upstream
participant Verifier
rect rgba(200,230,255,0.5)
Maintainer->>LocalRepo: start Phase 1 (ISO merge)
LocalRepo->>Upstream: fetch guarded remote + merge (--theirs rules)
Upstream-->>LocalRepo: upstream files
LocalRepo->>LocalRepo: apply per-file conflict rules (stack files vs downstream)
LocalRepo-->>Maintainer: merge result (zero-downstream logic)
end
rect rgba(200,255,200,0.5)
Maintainer->>LocalRepo: start Phase 2 (project alignment)
LocalRepo->>LocalRepo: apply MIGRATION.md and drift fixes
LocalRepo->>Verifier: run /verify (pre-checks, lint, tests, final summary)
Verifier-->>Maintainer: verification report
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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)
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: 1
🧹 Nitpick comments (1)
.claude/skills/update-stack/SKILL.md (1)
28-28: Use an unambiguous module path pattern in the conflict rule.Line 28’s
modules/home\|auth\|users\|tasks\|uploadsis easy to misread and may match unintended paths when applied manually. Prefer explicit paths or grouped form likemodules/(home|auth|users|tasks|uploads).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/update-stack/SKILL.md at line 28, The conflict-rule path pattern `modules/home\|auth\|users\|tasks\|uploads` is ambiguous; update the SKILL.md entry replacing that string with an explicit grouped form like `modules/(home|auth|users|tasks|uploads)` (or list each full path) so the rule is unambiguous when applied manually—locate the exact pattern text in the table row and replace it with the grouped or explicit paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/update-stack/SKILL.md:
- Line 14: The doc claims `config/defaults/` is stack core but also requires
`config/defaults/<project>.js` to be kept `--ours`, creating a contradiction
that can break Phase 1 merges; update SKILL.md to clearly state ownership: mark
the directory `config/defaults/` as stack core except for per-downstream
override files `config/defaults/<project>.js` which should be treated as
downstream-owned and resolved `--ours` during merges; specifically edit the line
referencing `Stack core: config/defaults/` and the line about
`config/defaults/<project>.js` so they consistently declare that the directory
is core while individual `<project>.js` files are downstream overrides and must
be preserved `--ours` to avoid merge conflicts.
---
Nitpick comments:
In @.claude/skills/update-stack/SKILL.md:
- Line 28: The conflict-rule path pattern
`modules/home\|auth\|users\|tasks\|uploads` is ambiguous; update the SKILL.md
entry replacing that string with an explicit grouped form like
`modules/(home|auth|users|tasks|uploads)` (or list each full path) so the rule
is unambiguous when applied manually—locate the exact pattern text in the table
row and replace it with the grouped or explicit paths.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.claude/agents/stack-maintainer.md.claude/skills/update-stack/SKILL.mdCLAUDE.md
💤 Files with no reviewable changes (1)
- .claude/agents/stack-maintainer.md
There was a problem hiding this comment.
Pull request overview
Reworks the Claude Code /update-stack skill documentation into an explicit two-phase workflow (ISO merge → project alignment) and removes the now-redundant embedded stack-maintainer agent documentation/file.
Changes:
- Updates
update-stackskill to a two-phase process with explicit conflict-resolution rules and/verifycheckpoints. - Removes the
stack-maintainerembedded agent (and its mention inCLAUDE.md). - Adjusts
CLAUDE.mdwording to reflect the updated.claude/contents.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
CLAUDE.md |
Removes references to embedded agents and updates .claude/ description accordingly. |
.claude/skills/update-stack/SKILL.md |
Replaces the flat merge checklist with a two-phase “ISO merge” + “project alignment” workflow and adds a conflict rules table. |
.claude/agents/stack-maintainer.md |
Deletes the embedded agent definition (agent removed from the repo). |
- Clarify config/defaults ownership (stack-owned files ISO, project.js downstream-only) - Add git add + merge --continue step after conflict resolution - Full commands in all table rows (no --ours shorthand) - Fix MIGRATION.md row to include file path - Remove backslash escaping in table pipe pattern
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3177 +/- ##
=======================================
Coverage 89.87% 89.87%
=======================================
Files 52 52
Lines 1166 1166
Branches 235 235
=======================================
Hits 1048 1048
Misses 107 107
Partials 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/update-stack/SKILL.md:
- Line 47: Replace the documented pre-merge command "git add ." with "git add
-A" in the SKILL.md content so conflict-resolution steps stage deletions as well
as additions/modifications before calling "git merge --continue"; locate the
line that currently shows the "git add ." command and update it to use the "-A"
flag to ensure all tracked changes (including removals) are staged.
- Line 35: The doc has a contradiction between the Phase 1 rule that “lib” must
exist in ISO and be identical to upstream and the table row that treats “lib
additions” as downstream-only kept with --ours; resolve by editing the table
cell text “Downstream-only new files (new modules, helpers, lib additions,
scripts)” to remove “lib additions” (or explicitly mark “lib” as
upstream-owned/exception), and update the Phase 1 wording to clearly state that
“lib” is upstream-owned and must be identical to upstream; ensure the keywords
“lib”, “Phase 1”, and the table row “Downstream-only new files” are updated so
the two rules no longer conflict.
- Replace git add . with git add -u (no untracked staging) - Clarify lib ownership: existing files ISO, new additions downstream
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.claude/skills/update-stack/SKILL.md (1)
12-12:⚠️ Potential issue | 🟠 MajorResolve conflicting
libownership rules between Phase 1 goal and conflict table.Line 12 requires
libto end Phase 1 identical to upstream, but Line 35 still treatslib additionsas downstream-owned (--ours). This can leave downstream logic inliband break ISO guarantees.Suggested doc adjustment
-| Downstream-only new files (new modules, helpers, lib additions, scripts) | Never delete — these do not exist in the stack, `git checkout --ours <file>` if flagged | +| Downstream-only new files (new modules, helpers, scripts) | Never delete — these do not exist in the stack, `git checkout --ours <file>` if flagged | +| Downstream-only `lib/` additions | Not allowed in Phase 1 ISO. Move/refactor outside `lib/` in Phase 2 if still needed. |Based on learnings: Keep the stack mergeable for downstream projects.
Also applies to: 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/update-stack/SKILL.md at line 12, Update the SKILL.md to make `lib` ownership consistent: change the conflict-table rule that currently marks `lib additions` as downstream-owned (`--ours`) so it matches the Phase 1 goal that `lib` must exit identical to upstream; locate the entries referencing `lib` and the `--ours` conflict strategy and replace them with an upstream-preferred strategy (or remove downstream-only ownership), and add a short clarifying sentence tying the `lib` rule to the Phase 1 goal to prevent downstream logic in `lib` and preserve ISO/merge guarantees.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.claude/skills/update-stack/SKILL.md:
- Line 12: Update the SKILL.md to make `lib` ownership consistent: change the
conflict-table rule that currently marks `lib additions` as downstream-owned
(`--ours`) so it matches the Phase 1 goal that `lib` must exit identical to
upstream; locate the entries referencing `lib` and the `--ours` conflict
strategy and replace them with an upstream-preferred strategy (or remove
downstream-only ownership), and add a short clarifying sentence tying the `lib`
rule to the Phase 1 goal to prevent downstream logic in `lib` and preserve
ISO/merge guarantees.
… row in update-stack
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.claude/skills/update-stack/SKILL.md (1)
35-35:⚠️ Potential issue | 🟠 Major
lib additionsrule still conflicts with Phase 1 ISO goal.Keeping downstream
lib additionswith--ourscontradicts the Phase 1 requirement thatlibexits identical to upstream (Line 12). This can leave downstream logic inlibimmediately after ISO merge.Suggested doc fix
-| Downstream-only new files (new modules, helpers, lib additions, scripts) | Never delete — these do not exist in the stack, `git checkout --ours <file>` if flagged | +| Downstream-only new files (new modules, helpers, scripts) | Never delete — these do not exist in the stack, `git checkout --ours <file>` if flagged | +| Downstream-only new files under `lib/` | Do not preserve in Phase 1 ISO; relocate/refactor during Phase 2 if still needed |Based on learnings: Keep the stack mergeable for downstream projects; Flag security or mergeability risks explicitly in reviews.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/update-stack/SKILL.md at line 35, Update the table rule that currently allows keeping "lib additions" downstream with `--ours`: change the "Downstream-only new files (new modules, helpers, lib additions, scripts)" entry so it no longer permits retaining `lib` changes that conflict with the Phase 1 ISO requirement of identical `lib` to upstream; instead instruct reviewers to flag downstream `lib` additions as merge risks and require either upstreaming, a security/mergeability review, or explicit approval before using `git checkout --ours`. Reference the "lib additions" rule and the "Phase 1 ISO goal" text so reviewers see the conflict and the new requirement to escalate downstream `lib` diffs.
🧹 Nitpick comments (1)
.claude/skills/update-stack/SKILL.md (1)
19-21: Harden remote setup to avoid merging from a staledevkit-nodeURL.If
devkit-nodealready exists but points elsewhere, this flow still proceeds. Consider setting/verifying URL explicitly before fetch/merge.Based on learnings: Keep changes minimal and merge-friendly for downstream projects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/update-stack/SKILL.md around lines 19 - 21, Ensure the local remote named "devkit-node" points to the expected repo before fetching/merging: check the current URL with git remote get-url devkit-node, and if it exists but differs from "https://github.com/pierreb-devkit/Node.git" update it via git remote set-url devkit-node <expected-url>; if it does not exist add it with git remote add devkit-node <expected-url>; then run git fetch devkit-node and git merge devkit-node/master. This keeps the existing flow (git fetch/devkit-node and git merge devkit-node/master) but guarantees the remote URL is correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.claude/skills/update-stack/SKILL.md:
- Line 35: Update the table rule that currently allows keeping "lib additions"
downstream with `--ours`: change the "Downstream-only new files (new modules,
helpers, lib additions, scripts)" entry so it no longer permits retaining `lib`
changes that conflict with the Phase 1 ISO requirement of identical `lib` to
upstream; instead instruct reviewers to flag downstream `lib` additions as merge
risks and require either upstreaming, a security/mergeability review, or
explicit approval before using `git checkout --ours`. Reference the "lib
additions" rule and the "Phase 1 ISO goal" text so reviewers see the conflict
and the new requirement to escalate downstream `lib` diffs.
---
Nitpick comments:
In @.claude/skills/update-stack/SKILL.md:
- Around line 19-21: Ensure the local remote named "devkit-node" points to the
expected repo before fetching/merging: check the current URL with git remote
get-url devkit-node, and if it exists but differs from
"https://github.com/pierreb-devkit/Node.git" update it via git remote set-url
devkit-node <expected-url>; if it does not exist add it with git remote add
devkit-node <expected-url>; then run git fetch devkit-node and git merge
devkit-node/master. This keeps the existing flow (git fetch/devkit-node and git
merge devkit-node/master) but guarantees the remote URL is correct.
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
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
Scope
.claude/skills/update-stack,.claude/agents(deleted),CLAUDE.mdnonelowValidation
npm run lintnpm testGuardrails check
Notes for reviewers
Summary by CodeRabbit