Skip to content

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

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#4232
PierreBrisorgueil merged 2 commits into
masterfrom
chore/update-stack-drop-ledger-condition

Conversation

@PierreBrisorgueil
Copy link
Copy Markdown
Collaborator

Closes #4231.

Summary

Three 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-vue/master. No "declare and skip" path.
  2. Auto-derive scan from tree roots — replace hardcoded module list (src/modules/home auth users tasks core app secure) with src/modules src/lib src/config.
  3. Drop stale src/modules/secure — never existed on devkit Vue origin/master.

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 src/modules/admin, src/modules/app, src/modules/billing, src/modules/core, src/modules/legal, src/modules/organizations AND included a non-existent src/modules/secure. Re-audit on trawl_vue with corrected scan surfaced 13 undeclared drifts (~700 LOC) — none declared in either trawl_vue or trawl_node ledger.

Mirrors pierreb-projects/infra#37 (PRF Phase 0.5 gate, merged 2026-06-03).

⚠️ Operational impact — READ

src/modules/app/app.router.js historically diverges on EVERY downstream Vue project because every project embeds custom routes there. Under no-ledger this BLOCKS on every Vue downstream /update-stack.

The right fix is to refactor: keep app.router.js stack-iso, register downstream routes from a downstream-only module via an extension hook. That refactor is separate work and should precede this gate landing safely — OR this PR ships as the forcing function (intended consequence of the no-ledger policy: BLOCK until each downstream's app.router.js + 12 other drifts are addressed).

Merging this PR means every Vue downstream's next /update-stack blocks on ~13 files (varies per project) until cleanup. That's the explicit intent of the no-ledger policy, but worth being explicit.

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_vue + other downstreams' next /update-stack (expected: BLOCK on surfaced drifts).

Cross-refs

…ist (#4231)

Three coupled changes to step 3ter drift gate:

1. Drop `DOWNSTREAM_PATCHES.md` ledger exception — block on ANY shared
   non-test file divergence vs `devkit-vue/master`. User decision
   2026-06-02 (memory `feedback_no_dev_in_shared_modules`). Resolution
   path becomes revert / promote-up / relocate.

2. Replace hardcoded module list (`src/modules/home auth users tasks core
   app secure`) with `src/modules src/lib src/config`. Old enum silently
   missed `src/modules/admin`, `app`, `billing`, `core`, `legal`,
   `organizations` AND included a non-existent `src/modules/secure`.
   Re-audit on trawl_vue with corrected scan surfaced 13 undeclared
   drifts (~700 LOC).

3. Drop stale `src/modules/secure` entry — never existed on devkit Vue
   origin/master.

Operational note: `src/modules/app/app.router.js` historically diverges on
every downstream (downstream routes). Under no-ledger it MUST be refactored
to keep `app.router.js` stack-iso. Until then the gate will block every
Vue downstream `/update-stack` — forcing function as intended.

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

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 5 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: cf91d096-4b1e-41a3-ba53-df2f5fc83b87

📥 Commits

Reviewing files that changed from the base of the PR and between 0560aab and 85829db.

📒 Files selected for processing (1)
  • .claude/skills/update-stack/SKILL.md
✨ 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 skill documentation to tighten Phase 1 “3ter” drift gating by removing the DOWNSTREAM_PATCHES.md ledger exception and broadening the scan scope so shared-file drift is always blocked.

Changes:

  • Removes the “declare and skip” ledger path and blocks on any shared non-test divergence vs devkit-vue/master.
  • Replaces the hardcoded module scan list with a broader scan over src/modules, src/lib, and src/config.
  • Updates the “3ter” operator messaging to reflect the new no-ledger policy and remediation paths.

Comment on lines 102 to 107
drift_found=0
while IFS= read -r f; do
upstream_blob=$(git ls-tree devkit-vue/master -- "$f" 2>/dev/null | awk '{print $3}')
[ -z "$upstream_blob" ] && continue # downstream-only file — skip
local_blob=$(git rev-parse "HEAD:$f" 2>/dev/null)
if [ "$upstream_blob" != "$local_blob" ]; then
Comment thread .claude/skills/update-stack/SKILL.md Outdated
Comment on lines +123 to +124
- Scan covers the full stack tree (`src/modules`, `src/lib`, `src/config`) — auto-discovers every shared module. Per-file `git ls-tree` on upstream filters downstream-only files.
- Test files (`/tests*/`, `/__tests__/`, `*.test.*`, `*.spec.*`) are excluded — downstream test adaptations are acceptable.
Comment thread .claude/skills/update-stack/SKILL.md Outdated
- 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 (`src/modules`, `src/lib`, `src/config`) — auto-discovers every shared module. Per-file `git ls-tree` on upstream filters downstream-only files.
- Test files (`/tests*/`, `/__tests__/`, `*.test.*`, `*.spec.*`) are excluded — downstream test adaptations are acceptable.
- `src/modules/app/app.router.js` historically diverged on every downstream (downstream routes). Under no-ledger this must be refactored: keep `app.router.js` stack-iso and register downstream routes from a downstream-only module (e.g. `src/modules/{project}/{project}.router.js` plugged via `app.router.js`'s extension hook). Until refactored, this gate will BLOCK on every Vue downstream `/update-stack`.
- Rewrite Phase 1 'Stack modules' line + 3bis 'stack code' line to point
  at auto-discovery (no hardcoded enumeration).
- Tighten test-files exclusion description to match the actual regex.
- Add Rules bullet noting e2e helpers under src/lib/helpers/e2e/ are
  scanned (stack-managed). Surfaced by critical-review fallback.
- Refine app.router.js Rules text: clarify that the extension hook
  needed for downstream route registration does NOT currently exist;
  the refactor must add it. Surfaced by Copilot review.

Separate follow-up issue will be filed for switching the scan source
from downstream `git ls-files` to upstream `git ls-tree` — closes
the coverage gap for stack files missing on downstream (Copilot review
finding, out of scope for this PR).
@PierreBrisorgueil
Copy link
Copy Markdown
Collaborator Author

Thanks Copilot + critical-review fallback — addressed in commit 85829db:

  • 'Stack modules' Phase 1 line + 3bis stack-code line rewritten to point at auto-discovery (no hardcoded enumeration).
  • Test-file regex description tightened.
  • app.router.js Rules text clarified: the extension hook does NOT currently exist; the refactor needs to add one. (Copilot was right that the previous wording implied it existed.)
  • Added Rules bullet noting e2e helpers under src/lib/helpers/e2e/ are scanned (stack-managed). (Surfaced by critical-review fallback.)

Coverage gap re. scan source (downstream git ls-files vs upstream git ls-tree) tracked as follow-up #4233. Out of scope for this PR (focused on no-ledger refactor) but is a real gap and will land next.

@PierreBrisorgueil PierreBrisorgueil merged commit 7be1220 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