Skip to content

feat(skills/update-stack): drop ledger condition + auto-derive scan list#3778

Merged
PierreBrisorgueil merged 2 commits into
masterfrom
chore/update-stack-drop-ledger-condition
Jun 3, 2026
Merged

feat(skills/update-stack): drop ledger condition + auto-derive scan list#3778
PierreBrisorgueil merged 2 commits into
masterfrom
chore/update-stack-drop-ledger-condition

Conversation

@PierreBrisorgueil
Copy link
Copy Markdown
Contributor

@PierreBrisorgueil PierreBrisorgueil commented Jun 3, 2026

Closes #3777.

Summary

Two coupled changes to /update-stack step 3ter drift gate:

  1. Drop DOWNSTREAM_PATCHES.md ledger exception — block on ANY shared non-test file divergence vs devkit-node/master. No "declare and skip" path.
  2. Auto-derive scan from tree roots — replace hardcoded module list (modules/home auth users tasks uploads billing lib config/defaults) with modules lib config. Per-file git ls-tree on 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-stack will BLOCK on modules/core/doc/index.yml until that drift is resolved per the 3 alternatives. Other Node downstreams (comes, montaine, pierreb, ism) pending re-audit.

Test plan

  • Pre-push critical review: pending.
  • Logical inspection of new bash flow: drift detection now unconditional on hash mismatch; downstream-only filter via git ls-tree upstream preserved verbatim.
  • Will be exercised end-to-end by trawl_node + other downstreams' next /update-stack (expected: BLOCK on the surfaced drifts until resolved).

Cross-refs

Summary by CodeRabbit

  • New Features

    • Enhanced drift detection to automatically scan across additional modules and configuration areas, providing more comprehensive coverage of potential divergences from upstream.
  • Bug Fixes

    • Strengthened Phase 1 verification gate to enforce stricter alignment with upstream shared files, removing the previous workaround mechanism for declaring divergences.
  • Documentation

    • Updated skill documentation to reflect new drift-resolution workflow and confirm test file exclusions remain in place.

…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.
Copilot AI review requested due to automatic review settings June 3, 2026 06:48
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

Warning

Review limit reached

@PierreBrisorgueil, we couldn't start this review because you've reached your PR review rate limit.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 44aa3d75-8c58-4c55-9ffd-34fffa2a7e4a

📥 Commits

Reviewing files that changed from the base of the PR and between 33ee527 and b3041ce.

📒 Files selected for processing (1)
  • .claude/skills/update-stack/SKILL.md

Walkthrough

The update-stack skill's Phase 1 drift gate is hardened to block unconditionally on shared file divergence by removing the DOWNSTREAM_PATCHES.md exception ledger, broadening file discovery across modules, lib, and config, and requiring explicit remediation (revert, upstream PR, or downstream relocation) for any detected drift.

Changes

Drift gate policy and enforcement

Layer / File(s) Summary
Drift gate policy redefinition and enforcement logic
.claude/skills/update-stack/SKILL.md
Gate 3ter is renamed to "Block on drift" and the drift detection loop is reworked to scan modules, lib, and config (excluding tests), report drift on each shared file immediately, and remove all DOWNSTREAM_PATCHES.md exception handling in favor of explicit remediation instructions (revert, create upstream PR, or relocate to downstream-only modules/config).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • pierreb-devkit/Node#3760: Both PRs modify the update-stack Phase 1 /verify "3ter" drift gate to block on undeclared differences vs upstream while deprecating/ignoring DOWNSTREAM_PATCHES.md and adjusting the drift scan behavior for shared modules/config.
  • pierreb-devkit/Node#3177: Both PRs modify the update-stack skill documentation/flow in .claude/skills/update-stack/SKILL.md, with the retrieved PR restructuring the two-phase workflow and the main PR adjusting the Phase 1 /verify drift-check gate.

Suggested labels

Chore

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description includes a clear summary of changes, rationale, operational impact, and test plan, though it doesn't follow the provided template structure with explicit sections like 'Scope', 'Validation', and 'Guardrails check'. Consider restructuring the description to match the repository template with explicit sections (Scope, Validation, Guardrails check) to improve clarity and consistency.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main changes: dropping the ledger condition and auto-deriving the scan list for the update-stack skill.
Linked Issues check ✅ Passed The PR directly addresses both objectives from #3777: dropping the DOWNSTREAM_PATCHES.md ledger exception and auto-deriving the scan list from tree roots (modules lib config).
Out of Scope Changes check ✅ Passed All changes in the pull request are scoped to the /update-stack skill's Phase 1 verification gate, directly addressing the two coupled objectives in #3777 with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 chore/update-stack-drop-ledger-condition

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.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +110 to 111
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.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 lift

Expand 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 to modules/home|auth|users|tasks|uploads|billing (line 34), but the Phase 1 drift gate scans the full shared tree under modules lib config (lines 110-111). Upstream devkit-node/master includes additional shared stack modules modules/audit, modules/core, and modules/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

📥 Commits

Reviewing files that changed from the base of the PR and between 3086820 and 33ee527.

📒 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.
@PierreBrisorgueil
Copy link
Copy Markdown
Contributor Author

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 git ls-files vs upstream git ls-tree — missing-locally coverage gap) tracked as follow-up #3779. Out of scope for this PR (focused on no-ledger refactor), but it's a real gap and will land next.

@PierreBrisorgueil PierreBrisorgueil merged commit 4f505c1 into master Jun 3, 2026
3 checks passed
@PierreBrisorgueil PierreBrisorgueil deleted the chore/update-stack-drop-ledger-condition branch June 3, 2026 06:59
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.

feat(skills/update-stack): drop ledger condition + auto-derive scan from tree roots

2 participants