feat(acul-screen-generator): add post-generation build validation phase#79
Open
subhankarmaiti wants to merge 10 commits into
Open
feat(acul-screen-generator): add post-generation build validation phase#79subhankarmaiti wants to merge 10 commits into
subhankarmaiti wants to merge 10 commits into
Conversation
…nce CLI-first workflow details Co-authored-by: Copilot <copilot@github.com>
… validation phase Co-authored-by: Copilot <copilot@github.com>
✅ claudelint — All checks passedFull report |
…reen reference checks
Member
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated ACUL screen generator SKILL.md to v1.0.1: nine-phase workflow, Node ≥22 and auth0 login/config verification, samples-first CLI gating with a dummy-page fallback, local-first modify rules, three generation paths (A/B/C), and an expanded lint/typecheck/build iterative validation loop with a 5-iteration cap. ChangesACUL Screen Generator v1.0.1 Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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: 2
🤖 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.
Inline comments:
In `@plugins/auth0/skills/acul-screen-generator/SKILL.md`:
- Around line 21-43: The markdown in SKILL.md contains unlabeled fenced code
blocks (MD040); update both opening fences so they include a language identifier
(e.g., change ``` to ```text) for the large instructional block starting with
"1. Check auth0-acul-samples availability..." and the smaller fenced block that
lists the React/React-JS GitHub URLs (the block containing "React:
https://github.com/auth0-samples/..."); ensure both opening fences are labeled
(e.g., ```text) so markdownlint MD040 is satisfied.
- Line 455: Update the malformed import example in the "Common pitfalls" section
so it uses the correct module path and matching quotes: replace the incorrect
backticked/mismatched snippet that shows `@auth0/auth0-acul-react/login-id'`
with the correct import `import { useLoginId } from '@auth0/auth0-acul-react'`
(referencing useLoginId) ensuring quotes match and the sub-path is not used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3af5cfbf-ea1f-4555-a8ce-da6eaf801692
📒 Files selected for processing (1)
plugins/auth0/skills/acul-screen-generator/SKILL.md
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@plugins/auth0/skills/acul-screen-generator/SKILL.md`:
- Line 450: The documentation currently allows raising the iteration cap from 5
to 10 under certain conditions; change this to enforce a strict, non-negotiable
hard cap of 5 iterations by removing or replacing the clause that permits
increasing to 10 and any conditional language about extending iterations in the
"Iteration cap" section of SKILL.md; ensure the text reads that iteration cap
defaults to and must not exceed 5 iterations and that if errors plateau across 2
consecutive iterations or the cap is reached, work stops and remaining errors
are presented to the customer for guidance (reference the "Iteration cap"
heading for the edit).
- Line 256: Update the Phase 2C guidance in SKILL.md to remove the contradiction
by making the reference hierarchy match the updated workflow: change the text
that currently reads "CLI first → samples → SDK examples" to "samples first →
CLI → SDK examples" (or equivalent wording that explicitly prioritizes samples
before any CLI attempts) so the line for "Only fetch from GitHub references..."
aligns with the samples-first policy; ensure the change is made in the Phase 2C
paragraph mentioning fetching from GitHub and verify any nearby sentences that
reference the old order are updated to the new "samples → CLI → SDK examples"
order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d51a3b0-92df-4247-a536-587b3f14aee2
📒 Files selected for processing (1)
plugins/auth0/skills/acul-screen-generator/SKILL.md
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/auth0/skills/acul-screen-generator/SKILL.md (2)
353-353: ⚡ Quick winClarify Path B heading to avoid confusion.
The heading "Screen from auth0-acul-samples (only when CLI doesn't support the screen)" is contradictory—if a screen is in auth0-acul-samples, the CLI should support it per the Reference Hierarchy (lines 22-28).
Path B actually applies when:
- Screen is in samples but CLI failed for environmental reasons (auth, version mismatch), OR
- Using the samples architecture pattern (modular structure) populated with SDK example data
Consider revising to: "Path B — Modular architecture using samples pattern (when CLI is unavailable or fails)" to clarify that you're using the architectural approach, not the actual samples code.
🤖 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 `@plugins/auth0/skills/acul-screen-generator/SKILL.md` at line 353, Update the Path B heading in SKILL.md to remove the contradictory implication that the CLI doesn't support a screen that exists in auth0-acul-samples; change the heading text and brief description to indicate this path is for using the modular/samples architecture pattern or when the CLI is unavailable/fails (e.g., auth/version/env issues). Specifically, replace the current heading "Screen from auth0-acul-samples (only when CLI doesn't support the screen)" with a clearer label such as "Path B — Modular architecture using samples pattern (when CLI is unavailable or fails)" and update any adjacent sentence(s) that mention CLI support to reflect the two cases: (1) sample exists but CLI failed for environmental reasons, or (2) using the samples architecture pattern with SDK example data.
217-222: ⚡ Quick winConsider edge case: what if the target screen is login-id?
The dummy-page strategy uses
login-idas a temporary scaffold to capture the project structure. If the user is actually addinglogin-idas their target screen (or iflogin-idalready exists), the instruction to "delete thelogin-id/screen directory" could be problematic.Consider adding a conditional: "Choose a different dummy screen (e.g.,
signuporlogin-password) iflogin-idis your target screen or already exists in the project."🤖 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 `@plugins/auth0/skills/acul-screen-generator/SKILL.md` around lines 217 - 222, Update the dummy-page instructions to handle the edge case where the target screen is "login-id" or a "login-id" screen already exists: replace the fixed example "auth0 acul screen add login-id -d <project-dir>" and the subsequent "delete the `login-id/` screen directory" guidance with a conditional note that advises choosing an alternate dummy name (e.g., "signup" or "login-password"), or first checking for an existing "login-id" directory before creating/deleting; reference the command example string "auth0 acul screen add login-id -d <project-dir>", the directory name `login-id/`, and suggested alternates "signup" and "login-password" so readers know what to change.
🤖 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.
Inline comments:
In `@plugins/auth0/skills/acul-screen-generator/SKILL.md`:
- Line 398: Update the validation note that currently limits applyAuth0Theme()
to Path B so it applies to all modular architecture paths: change the constraint
text to require applyAuth0Theme(sdkInstance) to be invoked in the standard
index.tsx pattern for any modular scaffold (both Path A and Path B) and ensure
the SKILL.md guidance references the function name applyAuth0Theme(sdkInstance)
and index.tsx entrypoint as the canonical place to call it for all modular
paths.
---
Nitpick comments:
In `@plugins/auth0/skills/acul-screen-generator/SKILL.md`:
- Line 353: Update the Path B heading in SKILL.md to remove the contradictory
implication that the CLI doesn't support a screen that exists in
auth0-acul-samples; change the heading text and brief description to indicate
this path is for using the modular/samples architecture pattern or when the CLI
is unavailable/fails (e.g., auth/version/env issues). Specifically, replace the
current heading "Screen from auth0-acul-samples (only when CLI doesn't support
the screen)" with a clearer label such as "Path B — Modular architecture using
samples pattern (when CLI is unavailable or fails)" and update any adjacent
sentence(s) that mention CLI support to reflect the two cases: (1) sample exists
but CLI failed for environmental reasons, or (2) using the samples architecture
pattern with SDK example data.
- Around line 217-222: Update the dummy-page instructions to handle the edge
case where the target screen is "login-id" or a "login-id" screen already
exists: replace the fixed example "auth0 acul screen add login-id -d
<project-dir>" and the subsequent "delete the `login-id/` screen directory"
guidance with a conditional note that advises choosing an alternate dummy name
(e.g., "signup" or "login-password"), or first checking for an existing
"login-id" directory before creating/deleting; reference the command example
string "auth0 acul screen add login-id -d <project-dir>", the directory name
`login-id/`, and suggested alternates "signup" and "login-password" so readers
know what to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 924f0106-f315-4c11-8d65-ccaa9897ebc9
📒 Files selected for processing (1)
plugins/auth0/skills/acul-screen-generator/SKILL.md
…t/acul-build-validation-phase
… dependency installation
…tions in SKILL.md
✅ skillsaw — All checks passedFull report
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@plugins/auth0/skills/acul-screen-generator/SKILL.md`:
- Around line 414-420: The table and the priority list use inconsistent notation
for Bun lockfiles (one uses "bun.lockb / bun.lock", the other uses
"bun.lock(b)"); pick one style (recommend "bun.lockb" and optionally "bun.lock"
separated by " / ") and update both the lockfile table row (the entry currently
showing `bun.lockb` / `bun.lock`) and the priority list reference (currently
"bun.lock(b)") to use that exact same notation so both places match; update any
other occurrences in SKILL.md to the chosen notation for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 193af00b-2e27-4abf-b65c-521668ff9abb
📒 Files selected for processing (1)
plugins/auth0/skills/acul-screen-generator/SKILL.md
Comment on lines
+414
to
+420
| | Lockfile | Package manager | Install command | | ||
| |----------|-----------------|-----------------| | ||
| | `pnpm-lock.yaml` | pnpm | `pnpm install` | | ||
| | `yarn.lock` | Yarn | `yarn install` | | ||
| | `package-lock.json` | npm | `npm install` | | ||
| | `bun.lockb` / `bun.lock` | Bun | `bun install` | | ||
|
|
There was a problem hiding this comment.
Use consistent notation for bun lockfiles.
The table at line 419 uses bun.lockb / bun.lock (slash notation), but line 420 uses bun.lock(b) (parenthetical notation) in the priority list. Choose one notation style and apply it consistently.
📝 Proposed fix for consistent notation
-If multiple lockfiles exist, prefer in this priority: `pnpm-lock.yaml` → `yarn.lock` → `package-lock.json` → `bun.lock(b)`.
+If multiple lockfiles exist, prefer in this priority: `pnpm-lock.yaml` → `yarn.lock` → `package-lock.json` → `bun.lockb` / `bun.lock`.🤖 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 `@plugins/auth0/skills/acul-screen-generator/SKILL.md` around lines 414 - 420,
The table and the priority list use inconsistent notation for Bun lockfiles (one
uses "bun.lockb / bun.lock", the other uses "bun.lock(b)"); pick one style
(recommend "bun.lockb" and optionally "bun.lock" separated by " / ") and update
both the lockfile table row (the entry currently showing `bun.lockb` /
`bun.lock`) and the priority list reference (currently "bun.lock(b)") to use
that exact same notation so both places match; update any other occurrences in
SKILL.md to the chosen notation for consistency.
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
2 participants
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.
Adds a new Phase 7: Build Validation & Iterative Fix to the ACUL screen generator skill. This phase runs automatically after code generation (Phase 6) to catch and fix common issues before handing off to the developer.
Problem
When generating ACUL screen code, the skill sometimes produces:
{}import, or vice versa)These issues require manual debugging by the developer after generation.
Solution
A new mandatory validation phase that:
npm run lint) and TypeScript type checker (tsc --noEmit)npm run build)Includes a table of common error patterns mapped to root causes and fixes, plus a "common pitfalls" section documenting the specific issues observed in practice.
Summary by CodeRabbit