feat(skills/update-stack): drop ledger condition + auto-derive scan list#3778
Conversation
…ist (#3777) Two coupled changes to step 3ter drift gate: 1. Drop `DOWNSTREAM_PATCHES.md` ledger exception — block on ANY shared non-test file divergence vs `devkit-node/master`. User decision 2026-06-02 (memory `feedback_no_dev_in_shared_modules`): drift in shared files must never happen, not be documented. Resolution path becomes revert / promote-up / relocate. 2. Replace hardcoded module list (`modules/home auth users tasks uploads billing lib config/defaults`) with `modules lib config`. Old enum silently missed `modules/audit`, `modules/core`, `modules/organizations`. Re-audit on trawl_node with corrected scan surfaced 1 undeclared drift (`modules/core/doc/index.yml`). Mirrors infra#37 (PRF Phase 0.5 gate) for `/update-stack`-time enforcement.
|
Warning Review limit reached
More reviews will be available in 54 minutes and 11 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe ChangesDrift gate policy and enforcement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the /update-stack “Phase 1 / step 3ter” drift gate documentation to remove the DOWNSTREAM_PATCHES.md exception and to broaden the scan scope so drift is detected across the whole stack tree rather than a hardcoded module list.
Changes:
- Removes the “declare and skip” (
DOWNSTREAM_PATCHES.md) workflow; any shared non-test divergence blocks. - Switches the scan roots from a fixed list to
modules lib config, relying on upstream tree presence to skip downstream-only files. - Updates the block message guidance to the new “revert / promote-up / relocate” resolution paths.
| done < <(git ls-files modules lib config 2>/dev/null \ | ||
| | grep -vE "/(tests|__tests__)/" | grep -vE "\.(test|spec)\.(js|jsx|ts|tsx)$") |
| - Declare diverging paths in `DOWNSTREAM_PATCHES.md` as `'path/to/file'` (single-quoted) — the gate matches on the quoted token to avoid substring collisions. | ||
| - Downstream-only files (new modules, helpers, lib additions) are not scanned — the sweep only covers the stack directories listed above. | ||
| - Block on ANY shared-file divergence. No "declare and skip" path — the `DOWNSTREAM_PATCHES.md` ledger model was abandoned 2026-06-02 (memory `feedback_no_dev_in_shared_modules`). | ||
| - Scan covers the full stack tree (`modules`, `lib`, `config`) — auto-discovers every shared module. Per-file `git ls-tree` on upstream filters downstream-only files. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/update-stack/SKILL.md (1)
20-20:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftExpand Phase 1 conflict-resolution rules to cover all upstream stack modules
In
.claude/skills/update-stack/SKILL.md, Phase 1’s “Resolve conflicts” rule only applies tomodules/home|auth|users|tasks|uploads|billing(line 34), but the Phase 1 drift gate scans the full shared tree undermodules lib config(lines 110-111). Upstreamdevkit-node/masterincludes additional shared stack modulesmodules/audit,modules/core, andmodules/organizations, so merges may be resolved incorrectly before the drift gate blocks.Update the stack modules list (line 20) and extend the line 34 rule to include
audit|core|organizations(and keep it aligned with the drift gate’s scope).🤖 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 @.claude/skills/update-stack/SKILL.md at line 20, Update the stack modules list and the Phase 1 "Resolve conflicts" rule in SKILL.md so they include the additional upstream shared modules `audit`, `core`, and `organizations`; specifically, modify the module enumeration on the top summary (the `Stack modules:` line that currently lists `home, auth, users, tasks, uploads, billing`) to add `audit`, `core`, and `organizations`, and extend the Phase 1 rule text that currently limits conflict resolution to `modules/home|auth|users|tasks|uploads|billing` so it also includes `audit|core|organizations`, ensuring this rule matches the drift gate’s scanned scope over `modules lib config`.
🤖 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.
Outside diff comments:
In @.claude/skills/update-stack/SKILL.md:
- Line 20: Update the stack modules list and the Phase 1 "Resolve conflicts"
rule in SKILL.md so they include the additional upstream shared modules `audit`,
`core`, and `organizations`; specifically, modify the module enumeration on the
top summary (the `Stack modules:` line that currently lists `home, auth, users,
tasks, uploads, billing`) to add `audit`, `core`, and `organizations`, and
extend the Phase 1 rule text that currently limits conflict resolution to
`modules/home|auth|users|tasks|uploads|billing` so it also includes
`audit|core|organizations`, ensuring this rule matches the drift gate’s scanned
scope over `modules lib config`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f625748f-6f5f-490b-bec6-4805b681db6c
📒 Files selected for processing (1)
.claude/skills/update-stack/SKILL.md
- Rewrite Phase 1 'Stack modules' line to point at auto-discovery (no
hardcoded enumeration that drifts with new modules).
- Rewrite 3bis 'stack code' line same way.
- Tighten test-files exclusion description to match the actual regex
(paths containing /tests/ or /__tests__/; *.test.{js,jsx,ts,tsx} /
*.spec.{js,jsx,ts,tsx}). The previous '/tests*/' wording was sloppy.
|
Thanks Copilot — both catches addressed in commit b3041ce (doc fixes for the 'Stack modules' line + test-file regex description). The third concern (scan source from local |
Closes #3777.
Summary
Two coupled changes to
/update-stackstep 3ter drift gate:DOWNSTREAM_PATCHES.mdledger exception — block on ANY shared non-test file divergence vsdevkit-node/master. No "declare and skip" path.modules/home auth users tasks uploads billing lib config/defaults) withmodules lib config. Per-filegit ls-treeon upstream filters downstream-only files.Why
User decision 2026-06-02 (memory
feedback_no_dev_in_shared_modules): drift in shared files must never happen, not be documented. Resolution path: revert / promote-up / relocate.Hardcoded scan list silently missed
modules/audit,modules/core,modules/organizations. Re-audit on trawl_node with corrected scan surfaced 1 undeclared drift (modules/core/doc/index.yml— trawl-specific OpenAPI tags), not in either ledger.Mirrors pierreb-projects/infra#37 (PRF Phase 0.5 gate, merged 2026-06-03).
Operational impact
trawl_node/update-stackwill BLOCK onmodules/core/doc/index.ymluntil that drift is resolved per the 3 alternatives. Other Node downstreams (comes, montaine, pierreb, ism) pending re-audit.Test plan
git ls-tree upstreampreserved verbatim./update-stack(expected: BLOCK on the surfaced drifts until resolved).Cross-refs
feedback_no_dev_in_shared_modulesinfra/docs/superpowers/plans/2026-05-30-trawl-devkit-perfect-alignment.mdTask E.2Summary by CodeRabbit
New Features
Bug Fixes
Documentation