feat(native): added React Native migration agent workflows and tooling - #3708
Conversation
Agent definitions, commands, rules, and templates for the React Native component migration workflow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…to plan agent - Add *.stories.tsx rows to the 2a file classification table in plan-rn-migration.md - Flag components excluded from RN Storybook registry for removal in execute phase - Add Storybook Stories section to rn-discovery-report.md template Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Plan agent: native-status check now classifies main + sub-components + styled/animated using genuine-stub detection (not bare throwBladeError string presence, which false-positives on Tabs/Avatar/AccordionButton) - Plan agent: add sub-component rows to 2a classification and Step 8 file plan - Execute agent: add Step 3b to create/replace sub-components, skipping any that already exist as real implementations; harden Step 4 main guard - Templates: add Sub-Components inventory to discovery report and a 3b sub-component row to the migration-plan file table Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ment Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove escape hatches that allowed skipping simulator/visual checks. Now boots emulator, starts Metro, and builds Storybook if needed. Uses yarn start:ios (includes patch script) and pod install without --repo-update. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
One-window curses app pairing a live RN-migration dashboard with switchable games (snake, RN trivia, mini crossword), plus standalone scripts and an osascript launcher. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a Status Reporting Protocol section that instructs the agent to emit a plain-English ▶ [Step N] status line before every significant action (boot, screenshot, click, scroll, browser open, fix, etc.) so operators can follow the pipeline without reading raw commands. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Every tool call now requires exactly one plain-English sentence summarising the intent of the whole command (including pipelines), not just significant actions. Examples show how to describe effect not syntax. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Step 4a now explicitly closes the RN Storybook bottom-sheet navigator (back / backdrop tap / swipe-down + snapshot verify) after selecting a story, so visual captures show the component instead of the navigator overlay. Adds a general dismiss-before-capture rule for steps 4b-4e. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Adds a Claude PreToolUse hook (on the Agent tool) that, before spawning any plan/execute/verify-rn-migration subagent, lists the on-disk artifacts for the referenced component and injects them as context so the orchestrator decides the next phase from disk state rather than turn history. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
- verify-rn-migration: agent-browser web comparison is mandatory even when agent-device screenshots look fine; web render is the source of truth — major UI issues (alignment, missing elements, colors, hierarchy) are P1 and block PASS; ambiguous diffs default to P1 not P2; verify agent must never push or create PRs - migrate-to-rn skill + orchestrator-guardrails: no commit/push/PR without explicit per-component human approval at the Final Gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove the Codex-specific RN migration skill, agents, rules, and templates. Co-authored-by: Cursor <cursoragent@cursor.com>
…re behavior A component's core interaction must be positively observed on-device; if it can't be, it's P0 (FAIL/BLOCKER) and may never be downgraded to P2 by blaming test automation or citing correct-looking wiring. Severity is scored on impact, not confidence. Co-authored-by: Cursor <cursoragent@cursor.com>
Each verify agent now gets a dedicated Metro port, iOS simulator device, and agent-device session so multiple components can be verified in parallel without resource conflicts. The orchestrator allocates slots (port/device/ session) and builds the Storybook app once, then installs it on all simulators. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds Step 0.8 to write RCT_jsLocation per simulator via xcrun simctl spawn, preventing reloads from fetching bundles from port 8081 during parallel visual verification. Also adds a hard gate and pre-flight checklist before Verify phase to ensure app build isn't skipped. Co-authored-by: Cursor <cursoragent@cursor.com>
|
(Review Cancelled - Superseded by a new run) |
| For each component that passed: | ||
|
|
||
| ```bash | ||
| cd {Worktree} && git add -A |
There was a problem hiding this comment.
git add -A here stages everything in the worktree, including the migration artifacts written to {Worktree}/.claude/artifacts/{Name}/ — the discovery/plan/verification reports and all screenshots/*.png. Neither .claude/artifacts/ nor .claude/worktrees/ is in .gitignore (git check-ignore returns nothing for them on this branch), so every component PR would carry its internal reports and screenshots.
Suggest either adding these paths to .gitignore, or staging only the component source explicitly, e.g.:
cd {Worktree} && git add packages/blade/src/components/{Name}| ### 0.3 Check Native Status | ||
|
|
||
| ```bash | ||
| grep -rl "throwBladeError" packages/blade/src/components/{Name}/ --include="*.native.tsx" | head -5 |
There was a problem hiding this comment.
This coarse gate treats any .native.tsx that contains throwBladeError as a stub → "needs migration". But plan-rn-migration.md explicitly warns that already-implemented components (Tabs.native.tsx, Avatar.native.tsx, AccordionButton.native.tsx) contain throwBladeError purely for input validation and are not stubs.
So a fully-migrated component can be mis-classified here as needing migration, and the orchestrator would spin up a worktree + Plan agent for it unnecessarily. Consider reusing the plan agent's own "genuine-stub detection" (body-shape / line-count heuristic) instead of bare string presence, so the entry gate and the Plan phase agree.
There was a problem hiding this comment.
updated the skill to reuse plan agents own "genuine-stub detection"
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "input=$(cat); sub=$(printf \"%s\" \"$input\" | jq -r \".tool_input.subagent_type // \\\"\\\"\"); printf \"%s\" \"$sub\" | grep -qE \"(plan|execute|verify)-rn-migration\" || exit 0; prompt=$(printf \"%s\" \"$input\" | jq -r \".tool_input.prompt // \\\"\\\"\"); listing=$(printf \"%s\" \"$prompt\" | grep -oE \"[^ \\\"]*/\\.claude/artifacts/[A-Za-z0-9_-]+\" | sort -u | while read -r d; do [ -d \"$d\" ] && { printf \"=== %s ===\\n\" \"$d\"; ls -1 \"$d\" 2>/dev/null; printf \"\\n\"; }; done); [ -n \"$listing\" ] || exit 0; printf \"%s\" \"$listing\" | jq -Rs '{hookSpecificOutput:{hookEventName:\"PreToolUse\",additionalContext:(\"Existing RN migration artifacts already on disk — READ these before spawning; decide the next phase from disk state, not from turn history:\\n\\n\" + .)}}'", |
There was a problem hiding this comment.
This registers a repo-level PreToolUse hook that fires on every Agent tool call for anyone who opens blade in Claude Code — not only during an RN migration. It early-exits for non-*-rn-migration subagents, but it still shells out to jq + grep on each Agent spawn and assumes jq is installed on the contributor's machine (a missing jq makes the hook error rather than no-op).
Two things to confirm:
- Is an always-on, shared hook the intent, vs. shipping it alongside the
migrate-to-rnskill/plugin so it's opt-in for people actually running the migration? - If
jqis a hard dependency, worth documenting it (and guarding with acommand -v jqcheck so the hook degrades gracefully).
There was a problem hiding this comment.
1)kept it in the shared .claude/settings.json because Claude Code doesn't give a clean way to attach a hook to a skill/plugin lifecycle — a repo-level hook is the only reliable place for it to fire during orchestration.
2)I'll add a command -v jq >/dev/null 2>&1 || exit 0 guard at the very top so the hook degrades gracefully, and document jq as a prerequisite for the migration workflow.
Step 0.3 classified any .native.tsx containing throwBladeError as a stub, mis-flagging validation-only files (Tabs, Avatar, AccordionButton) as needing migration. Switch to a line-count based genuine-stub check so the entry gate agrees with the Plan agent, and handle the partial-migration case. Co-authored-by: Cursor <cursoragent@cursor.com>
|
(Review Cancelled - Superseded by a new run) |
The Agent PreToolUse hook piped input through jq before any check, so a contributor without jq installed hit an error on every Agent spawn. Add a `command -v jq || exit 0` guard up front so the hook degrades to a no-op. Co-authored-by: Cursor <cursoragent@cursor.com>
|
(Review Cancelled - Superseded by a new run) |
Step 6 used `git add -A`, which staged the .claude/artifacts/{Name}/ reports
and screenshots into every component PR. Stage only the component source, and
add .claude/artifacts/ and .claude/worktrees/ to .gitignore (matching the
existing .cursor/.agents entries) as a safety net.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
(Review Cancelled - Superseded by a new run) |
There was a problem hiding this comment.
✨ Agentic PR Review ✨
UI Review
✅ 1 passed
Passing checks (1)
| Check | Screenshot |
|---|---|
| ✅ Storybook loads successfully | ![]() |
| if (lineEnd < 0) continue; | ||
| const line = content.substring(lineStart, lineEnd); | ||
| // Case 2: CASE_SPLIT_PATTERN (var or const, in any file) | ||
| content = content.replace(/var CASE_SPLIT_PATTERN\s*=\s*\/[^\n;]+;/g, CASE_SPLIT_REPLACEMENT_VAR); |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 6/10
Problem: The CASE_SPLIT_PATTERN regex replacement only matches var and const declarations, but not let. The old code used content.indexOf('CASE_SPLIT_PATTERN') which matched any declaration type. If a dependency uses let CASE_SPLIT_PATTERN = /...\p{Lu}.../;, the Unicode property escapes won't be patched, causing Hermes to crash with "InvalidRegExp" at runtime. The comment on line 91 explicitly says "var or const" confirming the gap.
Suggestion: Add a third regex for let: content = content.replace(/let CASE_SPLIT_PATTERN\s*=\s*\/[^\n;]+;/g, CASE_SPLIT_REPLACEMENT_VAR); (using var in the replacement is functionally safe).
|
|
||
| ```bash | ||
| # 1. Close any existing browser session | ||
| npx agent-browser close |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 7/10
Problem: Step 4e uses agent-browser commands (close, open, screenshot) without any --session flag, unlike all agent-device commands which use --session {SessionName}. In a parallel batch, multiple verify agents reaching Step 4e concurrently will race: one agent's npx agent-browser close kills another agent's browser session. This breaks the PR's claim of "fully parallel verification" — the mandatory web comparison step is not parallelizable as written.
Suggestion: Either add session support to the agent-browser commands (if supported), or document that Step 4e must be serialized across slots (e.g., use a file-based lock or queue the web comparison so only one agent runs it at a time).
| 5. **Open one PR per component** via `git push` + `gh pr create`, run inside the worktree. | ||
| 5. **Open one PR per component** via `git push` + `gh pr create`, run inside the worktree — **only after explicit human approval at the Final Gate**. | ||
|
|
||
| 6. **Update** `.claude/artifacts/batch-status.md` (in the **main checkout**) after every state change. |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 8/10
Problem: The guardrail (which the SKILL.md explicitly includes via Read tool) says to update .claude/artifacts/batch-status.md, but the SKILL.md Step 7 says to write .claude/artifacts/rn-batch-status.md. The orchestrator reads both files, creating conflicting instructions about where to write batch status. This could result in batch status being split across two files or written to the wrong one.
Suggestion: Update the guardrail to use a generic placeholder like .claude/artifacts/{pipeline}-batch-status.md, or update the SKILL.md to match the guardrail's batch-status.md filename.
| const progress = useSharedValue(isActive ? 1 : 0); | ||
|
|
||
| const motionDuration = getIn(theme.motion, 'duration.moderate'); | ||
| const motionEasing = getIn(theme.motion, 'easing.standard.effective'); |
There was a problem hiding this comment.
🔴 [CRITICAL] · code-quality-critique · confidence: 9/10
Problem: The animation pattern uses getIn(theme.motion, 'easing.standard.effective') but the token path easing.standard.effective does not exist in the Blade token system. The actual token is easing.standard which returns a makeBezier() result. On native, makeBezier returns an EasingFactoryFn (not an array), so Easing.bezier(...motionEasing) would spread the function object, producing invalid arguments and a runtime crash. Existing native components like AnimatedThumb.native.tsx and AnimatedButtonContent.native.tsx use the easing value directly: easing: easing not easing: Easing.bezier(...easing).
Suggestion: Change to const motionEasing = getIn(theme.motion, 'easing.standard'); and use easing: motionEasing directly (not Easing.bezier(...motionEasing)). The value from getIn is already an EasingFactoryFn on native via makeBezier.native.ts. Remove the Easing import if no longer needed.
| const progress = useSharedValue(isActive ? 1 : 0); | ||
|
|
||
| const motionDuration = duration ?? getIn(theme.motion, 'duration.moderate'); | ||
| const motionEasing = getIn(theme.motion, 'easing.standard.effective'); |
There was a problem hiding this comment.
🔴 [CRITICAL] · code-quality-critique · confidence: 9/10
Problem: Same incorrect animation pattern as rn-migration.md: uses getIn(theme.motion, 'easing.standard.effective') (non-existent token path) and Easing.bezier(...motionEasing) (spreading an EasingFactoryFn). This is the code template that the execute agent will copy into actual .native.tsx files, so every component migrated using this agent will have a runtime crash in its animation code.
Suggestion: Change to const motionEasing = getIn(theme.motion, 'easing.standard'); and easing: motionEasing (no spread, no Easing.bezier wrapper). The Easing import from react-native-reanimated can be removed if not used elsewhere.
| | `ease` | `Easing.ease` | | ||
| | `ease-in-out` | `Easing.inOut(Easing.ease)` | | ||
| | `cubic-bezier(a,b,c,d)` | `Easing.bezier(a,b,c,d)` | | ||
| | Theme `easing.standard.effective` | `Easing.bezier(...getIn(theme.motion, 'easing.standard.effective'))` | |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 9/10
Problem: The easing mapping table shows Easing.bezier(...getIn(theme.motion, 'easing.standard.effective')) as the reanimated equivalent for theme easing. This propagates the same incorrect token path and incorrect spread pattern to every migration plan generated from this template.
Suggestion: Change to getIn(theme.motion, 'easing.standard') and note that the value is already an EasingFactoryFn on native — use it directly as easing: motionEasing in withTiming.
| @@ -23,7 +24,10 @@ const localNodeModules = (() => { | |||
| })(); | |||
| const rootNodeModules = path.resolve(__dirname, '../../../node_modules'); | |||
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 8/10
Problem: rootNodeModules is not resolved with fs.realpathSync like localNodeModules is. The comment on line 17 says 'Always resolve symlinks so grep -R works correctly on macOS', but only localNodeModules gets symlink resolution. If rootNodeModules is also a symlink (common in monorepo hoisted node_modules), grep -R will fail to follow it on macOS, missing files that need patching.
Suggestion: Apply the same realpathSync fallback pattern to rootNodeModules: const rootNodeModules = (() => { try { return fs.realpathSync(path.resolve(__dirname, '../../../node_modules')); } catch { return path.resolve(__dirname, '../../../node_modules'); } })();
| if (!fs.existsSync(nodeModules)) continue; | ||
| try { | ||
| const found = execSync( | ||
| `grep -Rl "\\\\p{" "${nodeModules}" --include="*.js" --include="*.mjs" 2>/dev/null || true`, |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 9/10
Problem: The CASE_SPLIT_PATTERN regex /var CASE_SPLIT_PATTERN\s*=\s*\/[^\n;]+;/g uses [^\n;]+ which stops at any semicolon. If the regex literal contains a semicolon inside a character class (e.g., /[#;]/g), the match is truncated at the semicolon inside the character class, producing a corrupted replacement that leaves trailing characters in the file. The old code checked line.includes('\\p{') before replacing, which acted as a safety gate.
Suggestion: Use a regex that properly handles character classes, or add back the \\p{ content check before replacing to avoid corrupting regex literals that don't contain Unicode property escapes.
| .filter(Boolean); | ||
| files.push(...found); | ||
| found.forEach((f) => allFiles.add(f)); | ||
| } catch { |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 8/10
Problem: The combined identifierStartRegex+identifierContinueRegex regex uses [^;]+ between the two declarations. If there are intermediate comma-separated variable declarations between identifierStartRegex and identifierContinueRegex (e.g., var identifierStartRegex = new RegExp(...), foo = 'bar', identifierContinueRegex = new RegExp(...);), the greedy [^;]+ match includes and silently drops the intermediate foo = 'bar' declaration in the replacement.
Suggestion: Make the combined regex more restrictive: use [^;,]+ instead of [^;]+ for the middle portion, or use a non-greedy match. Alternatively, rely solely on the separate fallback regexes (lines 74-79) and remove the combined regex.
| } | ||
| } | ||
|
|
||
| let patched = 0; |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 7/10
Problem: The fallback regexes for identifierStartRegex and identifierContinueRegex always replace with var even if the original declaration used let or const. This changes the scoping semantics from block-scoped to function-scoped. While unlikely to cause issues in bundled code, it is technically incorrect.
Suggestion: Capture the original declaration keyword and preserve it in the replacement, or use a callback function in the replace call to dynamically construct the replacement string.
|
|
||
| ```bash | ||
| # Find the built .app from the Xcode derived data | ||
| APP_PATH=$(find ~/Library/Developer/Xcode/DerivedData -name "blade.app" -path "*/Build/Products/Debug-iphonesimulator/*" -maxdepth 5 2>/dev/null | head -1) |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 7/10
Problem: The find command uses -maxdepth 5 at the end of the option list. On macOS BSD find (the primary platform for RN development), -maxdepth must appear before other primaries. This could cause the command to silently fail on macOS, resulting in an empty APP_PATH and failing to install the app on secondary simulators.
Suggestion: Move -maxdepth 5 to immediately after the path: find ~/Library/Developer/Xcode/DerivedData -maxdepth 5 -name "blade.app" -path "*/Build/Products/Debug-iphonesimulator/*" 2>/dev/null | head -1
|
|
||
| it('should handle onPress when not disabled', () => { | ||
| const onPress = jest.fn(); | ||
| const { getByText } = renderWithTheme( |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 6/10
Problem: The test pattern in the execute agent uses getByText('Press me') to find and press the component, while the rules file (rn-migration.md) testing pattern uses getByRole('button'). getByText can match non-interactive Text nodes, leading to false positive tests where fireEvent.press fires on a non-pressable element.
Suggestion: Use getByRole('button') consistently in the execute agent's test template, matching the pattern in rn-migration.md. If the component doesn't have role='button', use getByTestId or the appropriate role.
| } | ||
| // Case 3: identifierStartRegex + identifierContinueRegex (jsdoc-type-pratt-parser, bundled into chunks) | ||
| content = content.replace( | ||
| /(?:let|var|const)\s+identifierStartRegex\s*=\s*new RegExp\([^;]+identifierContinueRegex\s*=\s*new RegExp\([^;]+;/g, |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 8/10
Problem: The combined identifierStartRegex+identifierContinueRegex regex uses [^;]+ between the two declarations. Since [^;] matches newlines, if the two new RegExp(...) calls are separated by other code that lacks semicolons (ASI in non-minified JS), the regex greedily matches across all intermediate lines and replaces them, destroying code. For example, let identifierStartRegex = new RegExp(...)\nlet foo = "bar"\nlet identifierContinueRegex = new RegExp(...) (no semicolons) would have foo deleted.
Suggestion: Use [^;\n]+ to restrict matching to a single line, or remove the combined regex entirely and rely on the individual fallback regexes (lines 103-108) which are safer. The combined regex is an optimization for the same-line case; the fallbacks already handle the separate-lines case.
|
|
||
| // Case 4: string-built Unicode class references (\p{Lu}, \p{Ll}) | ||
| content = content.replace( | ||
| /const rUnicodeUpper\s*=\s*['"]\\\\p\{Lu\}['"];/g, |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 9/10
Problem: The rUnicodeUpper and rUnicodeLower replacement regexes (lines 113-120) only match const declarations: /const rUnicodeUpper\s*=.../g. If a dependency uses var rUnicodeUpper or let rUnicodeUpper, the replacement won't apply and the Unicode property escape will remain, causing the Hermes crash the script is meant to prevent.
Suggestion: Change const to (?:const|var|let) in both regex patterns, matching the pattern already used in Case 3 for identifierStartRegex.
| if (lineEnd < 0) continue; | ||
| const line = content.substring(lineStart, lineEnd); | ||
| // Case 2: CASE_SPLIT_PATTERN (var or const, in any file) | ||
| content = content.replace(/var CASE_SPLIT_PATTERN\s*=\s*\/[^\n;]+;/g, CASE_SPLIT_REPLACEMENT_VAR); |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 7/10
Problem: The CASE_SPLIT_PATTERN regex uses \/[^\n;]+; to match the regex literal. If the regex literal itself contains a semicolon (e.g., inside a character class like /[/;]/g), [^\n;]+ stops at the first ; inside the character class. The replacement is inserted before that ;, leaving the remainder (e.g., ]/g;) appended after, producing invalid JS. The current es-toolkit pattern doesn't have this, but a future version could.
Suggestion: Use a more precise pattern that matches to the closing / followed by flags and ;, e.g.: /var CASE_SPLIT_PATTERN\s*=\s*\/.+?\/[gimusy]*;/g with non-greedy .+?.
| // Skip files not related to es-toolkit or storybook string utils | ||
| if (!file.includes('es-toolkit') && !file.includes('storybook')) continue; | ||
| for (const file of allFiles) { | ||
| if (file.includes('.d.ts') || file.includes('.map') || file.includes('/.cache/')) continue; |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 7/10
Problem: The old code filtered to only es-toolkit and storybook packages (if (!file.includes('es-toolkit') && !file.includes('storybook')) continue;). This filter was removed, so the script now reads and attempts to patch ALL .js/.mjs files in node_modules containing \p{. While the regex patterns target specific variable names (CASE_SPLIT_PATTERN, identifierStartRegex, etc.), identifierStartRegex and identifierContinueRegex are generic enough to appear in other packages. A false-positive match would corrupt an unrelated dependency with no easy recovery short of yarn install.
Suggestion: Either restore a filter list that includes the known affected packages (es-toolkit, storybook, jsdoc-type-pratt-parser), or add a log message when a file outside known packages is patched so unexpected modifications are visible.
| Alternative (faster check): | ||
| ```bash | ||
| cd {Worktree}/packages/blade && grep -r "from '\.\/" src/components/{Name}/ --include="*.native.tsx" | while read line; do | ||
| file=$(echo "$line" | sed "s/:.*from '\.\/\(.*\)'.*/\1/") |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 8/10
Problem: The sed command s/:.*from '\.\/\(.*\)'.*/\1/ uses greedy .* in the capture group. If an import line contains additional single-quoted strings after the import path (e.g., a comment like import { X } from './Foo' // 'bar'), the greedy \(.*\) captures Foo' // 'bar instead of just Foo, producing an incorrect path for the file-existence check and a false 'UNRESOLVED' result.
Suggestion: Replace \(.*\) with \([^']*\) to stop at the first closing single-quote: s/:.*from '\.\/\([^']*\)'.*/\1/. Also use IFS= read -r line instead of read line to preserve whitespace and backslashes.
| # Genuine stub = small file whose body is essentially just a throwBladeError (no real render tree/hooks). | ||
| # Validation-only files that use throwBladeError are large, so a line-count threshold separates them. | ||
| grep -rl "throwBladeError" packages/blade/src/components/{Name}/ --include="*.native.tsx" | while read -r f; do | ||
| if [ "$(wc -l < "$f")" -lt 40 ]; then echo "STUB: $f"; else echo "REAL: $f"; fi |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 7/10
Problem: The orchestrator's pre-flight stub detection uses a hardcoded 40-line threshold: if [ "$(wc -l < "$f")" -lt 40 ]. This disagrees with the Plan agent's more sophisticated 'genuine-stub detection' (which examines the file body for real render trees/hooks). A genuine stub file with >40 lines (e.g., verbose imports + throwBladeError + comments) would be classified as REAL by the orchestrator, causing it to skip the component entirely — the Plan agent never runs.
Suggestion: Either increase the threshold to a safer value (e.g., 80-100 lines) since genuine stubs are 'essentially just throwBladeError + trivial return' and rarely exceed 20 lines even with imports, or add a secondary check: grep -c 'return\|<\|useEffect\|useState\|useMemo' "$f" to detect whether the file has real render/logic beyond throwBladeError.
- Replace `npx agent-device`/`npx agent-browser` with direct binary calls - Prefer `swipe` over unreliable `agent-device scroll` for navigation - Tap by `@ref` (device points) instead of eyeballed pixel coords - Document dismissing error/warning toasts via coordinate tap Co-authored-by: Cursor <cursoragent@cursor.com>
|
(Review Cancelled - Superseded by a new run) |
Reverts the npx -> direct-binary edits in verify-with-browser/SKILL.md; the direct-binary/headed guidance stays in the RN agent + migrate-to-rn skill. Co-authored-by: Cursor <cursoragent@cursor.com>
|
(Review Cancelled - Superseded by a new run) |
Document jq, agent-device, and agent-browser checks before Step 0 and have the orchestrator install anything missing. Assumes normal Blade dev setup is already in place. Co-authored-by: Cursor <cursoragent@cursor.com>
|
(Review Cancelled - Superseded by a new run) |
…ght tool checks [resolved by agent] Co-authored-by: admin <admin>
|
🤖 Slash AI Review has been triggered. View execution logs |

Summary
plan-rn-migration,execute-rn-migration, andverify-rn-migrationagents (both.claudeand.agentsvariants) plus themigrate-to-rnskill for orchestrating component migrations.claude/rules/rn-migration.md), report/plan/patch-request templates, and orchestrator guardrail updatespatch-storybook-hermes.jsfor the native Storybook workflow🤖 Generated with Claude Code