chore(skills): integrate React Doctor into prod-readiness + audit-code-quality (#243)

[thomasluizon]

What & why

Make the React Doctor React-correctness gate (already a required CI check via .github/workflows/react-doctor.yml, but there --scope changed) a first-class part of the launch-readiness skills, so the full standing backlog is surfaced and driven to zero — not just the per-PR delta.

Changes

• prod-readiness workflow (prod-readiness.mjs): new React phase runs a scoped full-repo scan (react-doctor@0.7.6 --project apps/web,apps/mobile,packages/shared + hermetic flags matching CI; excludes design/handoff/**, whose vendored *.jsx mockups emit ~1054 false jsx-no-undef) and returns structured findings in the result.
• prod-readiness SKILL: React Doctor is binding inventory item fix: onboarding finish, level-up re-trigger, fresh start focus #10; errors → High, warnings → Low/Info; coverage-table row + the GO verdict now require zero react-doctor errors.
• audit-code-quality SKILL: new Phase 2.5 runs the same scan on any frontend scope.

Tooling-only change (.claude/**); no app code touched. Baseline full-scan backlog (excl. design/handoff): 128 errors + 1094 warnings — burned down separately.

Refs #243

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
orbit-ui-mobile-web	Ignored		Jul 12, 2026 11:13pm

[claude]

Code Review: PR #460 (chore(skills): integrate React Doctor into prod-readiness + audit-code-quality)

Scope: Tooling-only diff: .claude/skills/audit-code-quality/SKILL.md (+23), .claude/skills/prod-readiness/SKILL.md (+25/-8), .claude/workflows/prod-readiness.mjs (+57). No apps/*, packages/shared, or orbit-api files touched.
Recommendation: APPROVE

Summary

This wires the React Doctor React-correctness scanner into the prod-readiness and audit-code-quality skills as a full-repo (not --scope changed) pass, adding a 10th binding inventory item to prod-readiness with its own GO-gating rule (zero react-doctor errors). The mechanics (schema, prompt, fallback defaults, verdict wiring) are sound and consistent with the file's existing patterns. One real correctness gap in the new gating logic in prod-readiness.mjs, Medium severity — not merge-blocking.

Findings

Critical

None.

High

None.

Medium

[MEDIUM] React Doctor phase mis-gates a path-scoped, orbit-api-only prod-readiness run
· dimension: 1. Correctness
· location: .claude/workflows/prod-readiness.mjs:159 (`if (scope !== 'api') { ... }`)
· issue: prod-readiness's own argument-hint (and SKILL.md Phase 1's resolution table) supports scope = an arbitrary path, not just the literal tokens 'ui'/'api'/'both'. The new React phase only skips when `scope` is exactly the string `'api'`. A very plausible real input — e.g. `/prod-readiness orbit-api` (a whole-repo-name path, not the literal `'api'` token) or any path under the orbit-api checkout — falls through to `scope !== 'api'` being true, so the phase still launches a FULL `apps/web,apps/mobile,packages/shared` react-doctor scan even though the requested scope is backend-only and has no React surface at all.
· risk: the consolidated report gets a spurious, irrelevant "item 10" result (frontend findings mixed into what the caller framed as a backend-only readiness sweep), wastes the scan's ~360s budget, and the coverage table's "ui/both scope only" framing in prod-readiness/SKILL.md is silently violated for this input shape.
· fix: mirror the existing `resolveSurfaces`/`surfaceRepos` pattern already in `.claude/workflows/audit.mjs:130-145` — treat any scope that resolves to an api-only surface (literal `'api'`/`'backend'`, or a path under the orbit-api root) as excluded, e.g. `const isApiOnlyScope = (s) => ['api','backend'].includes(s) || /orbit-api/i.test(s)`, and gate on that instead of the strict `!== 'api'` string check.
· reference: CLAUDE.md rule 1 (root cause) / rubric dimension 1 (Correctness) — the workflow's own existing scope-normalization convention in audit.mjs shows the intended pattern this file should have reused.

Low / Info

None posted (Signal gate — Low/Info are not PR-review findings).

Subagents

Agent	Verdict
parity-checker	N/A — no apps/web/** or apps/mobile/** file changed
i18n-syncer	N/A — no user-facing strings or packages/shared/src/i18n/* changed
contract-aligner	N/A — no packages/shared/src/types/* / endpoints.ts, no orbit-api
security-reviewer	N/A — no orbit-api code changed
design-reviewer	N/A — no apps/*/orbit-landing-page UI files changed

Validation

Check	Result
Lint	N/A — skipped per CI adaptation (this PR's own required checks — Build/Unit Tests/SonarCloud — cover it; also no workspace's ESLint config scopes .claude/**)
Type check	N/A — no TS source changed
Tests	N/A — no testable app code changed
Build (api)	N/A — orbit-api untouched, not checked out

Deferred — N/A dimensions & files not verdicted

• Dimension 8 (DESIGN.md/AI-slop) — N/A, no apps/* UI files in the diff.
• Dimension 9 (Parity) — N/A, no apps/web/apps/mobile files in the diff.
• Dimension 10 (i18n) — N/A, no locale/user-facing-string changes.
• Dimension 11 (Contract drift + backward-compat) — N/A, no shared-types/DTO changes; also orbit-api not checked out in this CI context, so this is doubly not verifiable here.
• Dimension 13 (Backend hard rules) — N/A, orbit-api not touched.
• Dimension 14 (FEATURES.md parity) — N/A, tooling-only change with no user-facing feature surface.
• All 3 changed files received a verdict above — nothing left un-reviewed.

What's good

• The new REACT_DOCTOR_SCHEMA + reactDoctorPrompt() correctly reuse the file's existing conventions: pinned tool version (react-doctor@0.7.6, not floating), an explicit ran:false fallback for both the api-scope skip and an agent-death case, and general-purpose (not audit-readonly) chosen deliberately since this finder needs Bash to actually run the CLI.
• Correctly distinguishes the CI gate's --scope changed (delta-only) from this full-repo sweep, and documents the design/handoff/** false-positive exclusion (~1054 false jsx-no-undef) in both touched SKILL.md files consistently.
• prod-readiness/SKILL.md's GO-verdict update is coherent: item 10 is folded into the binding-inventory count (9→10), the unified ladder maps react-doctor error/warning onto the existing High/Low-Info tiers without inventing new vocabulary, and the "ran:false for ui/both is a Deferred gap, not clean" distinction is stated explicitly.

Recommendation

Merge as-is; the one Medium finding (path-scope mis-gating) is a real but non-blocking edge case — worth a fast follow-up fix (the one-line pattern already exists in audit.mjs to copy from) but doesn't justify holding this PR, since the dominant call shapes (ui/api/both) are all handled correctly.

Severity	Count
Critical (incl. ⚠️ old-client breaks)	0
High	0
Medium	1
Low / Info	0 (posted)

[claude]

Code Review: PR #460 — chore(skills): integrate React Doctor into prod-readiness + audit-code-quality

Scope: PR #460 in thomasluizon/orbit-ui-mobile (tooling-only, .claude/**; no app code touched)
Recommendation: NEEDS WORK

Summary

This wires a full-repo react-doctor@0.7.6 scan into prod-readiness.mjs (new reactDoctor phase) and documents it as inventory item #10 in prod-readiness/SKILL.md and a new Phase 2.5 in audit-code-quality/SKILL.md. The idea and CI-flag parity are sound, but the launch-gating aggregate counts this PR introduces are not validated against the itemized findings, and it bypasses this exact codebase's existing pattern for that problem.

Findings

High

[High] errorCount/warningCount that gate the launch verdict are unvalidated LLM self-reports, not derived from findings — and this bypasses the codebase's own established fix for that problem

• location: .claude/workflows/prod-readiness.mjs — REACT_DOCTOR_SCHEMA (required: ['ran', 'findings']) and the phase('React') block that consumes reactDoctor as returned by agent(), unmodified.
• issue: errorCount/warningCount are declared in the schema's properties but are not in required, so the sub-agent's structured output can omit or mis-report them, and nothing in this file cross-checks them against the findings array it also returns. prod-readiness/SKILL.md makes these two fields the hard gate: "GO requires the app-code error backlog at zero" and the report skeleton prints {errorCount errors / warningCount warnings} straight from the field rather than re-deriving it.
• risk: a mismatched or missing errorCount (e.g., the model reports 0 while findings actually contains severity: "error" items) silently produces a false GO on what this skill explicitly bills as the pre-launch decision document. This is exactly the codebase's own definition of a High finding — "missing validation."
• This is a regression from an existing in-repo pattern, not a novel judgment call: the sibling audit.mjs (same engine) already solves this exact problem with a countBy(findings) helper that deterministically derives severity counts from a returned findings array (used for counts: countBy(kept)). This PR trusts a raw, self-reported aggregate instead of reusing that helper for react-doctor.
• fix: derive errorCount/warningCount in prod-readiness.mjs from reactDoctor.findings (e.g. findings.filter(f => f.severity === 'error').length, mirroring audit.mjs's countBy) instead of trusting the agent's self-reported aggregate, and/or add both fields to REACT_DOCTOR_SCHEMA's required array so a non-conforming response fails loudly rather than silently.
• reference: CLAUDE.md rule 10 (DRY at the right level — reuse countBy rather than re-inventing an unvalidated variant); rubric.md severity ladder, High — "missing validation."
• verdict: CONFIRMED by an independent adversarial skeptic pass, which additionally located the countBy precedent in audit.mjs that strengthens the finding (this is a regression from an existing pattern, not a pre-existing accepted risk). The skeptic noted a mitigating factor — the actual CI-blocking gate (.github/workflows/react-doctor.yml, --scope changed) is a separate deterministic check unaffected by this bug, so only the advisory full-repo prod-readiness report is tainted — and offered Medium as a defensible alternate severity. Kept at High because the SKILL.md explicitly bills this report as the pre-launch decision document and the fix (reuse countBy) is trivial.

Medium

[Medium] The "identical" react-doctor invocation is duplicated with drifted, platform-incompatible temp-path syntax across the two files that describe it

• location: .claude/workflows/prod-readiness.mjs (reactDoctorPrompt(), the --json-out "%TEMP%/rd-prodreadiness.json" line) vs. .claude/skills/audit-code-quality/SKILL.md Phase 2.5 (--json-out "$TMPDIR/rd-audit.json").
• issue: the PR body describes this as "the same scan" with matching CI-hermetic flags, and every other flag is byte-identical between the two invocations — but the temp-output path uses Windows cmd/PowerShell syntax (%TEMP%) in one file and POSIX shell syntax ($TMPDIR) in the other. Neither is a no-op in the other's shell: %TEMP% is not expanded by bash (it would be passed through as a literal path segment), and $TMPDIR is frequently unset in a bash environment, expanding to an empty string ("/rd-audit.json", a root-level path most environments cannot write to).
• risk: whichever convention doesn't match the shell actually executing it, --json-out writes to (or fails to write to) a bogus path, and the following "Read the JSON report file" step in both prompts has nothing valid to read — degrading (best case) to a silent ran:false/empty findings, or (worse) an agent that reports success against stale/nonexistent data.
• fix: pick one portable convention and use it in both places — e.g., have each .mjs/skill instruction resolve an absolute temp path itself (Node's os.tmpdir() inside prod-readiness.mjs; a mktemp call or an explicit repo-relative scratch path in the audit-code-quality instruction) rather than embedding a shell-specific env-var reference inside a prompt string that's interpreted by whatever shell the executing agent actually has.
• reference: CLAUDE.md rule 10 (DRY at the right level — this is the exact "extract on the third real use" case: the same command, written out independently three times across this PR, already drifted on the first pass).

Subagents

Agent	Verdict
parity-checker	N/A — no apps/web/** or apps/mobile/** file changed
i18n-syncer	N/A — no user-facing strings or i18n JSON changed
contract-aligner	N/A — no packages/shared/src/types/* / endpoints.ts / orbit-api DTOs changed
security-reviewer	N/A — no orbit-api code changed
design-reviewer	N/A — no apps/* or landing-page UI file changed

Validation

Check	Result
Lint	N/A — .claude/** is outside every workspace's ESLint config (apps/web, apps/mobile, packages/shared only)
Type check	N/A — no TS/C# source changed
Tests	N/A — no test-covered source changed
Build (api)	N/A — orbit-api not touched

Deferred — N/A dimensions & files not verdicted

• Dimensions 8 (DESIGN.md/AI-slop), 9 (Parity), 10 (i18n), 11 (Contract drift/backward-compat), 13 (Backend hard rules), 14 (FEATURES.md parity) — N/A, diff never touches apps/* UI, packages/shared/src/types, orbit-api, or the user-facing feature surface (tooling-only per the PR's own description, confirmed by reading the diff).
• Dimension 12 (Security) — reviewed manually (no orbit-api change to gate security-reviewer in): no injection surface, no secrets, no auth-relevant code; the new shell-command strings are static (no user input flows into them). Clean.
• All 3 changed files (.claude/skills/audit-code-quality/SKILL.md, .claude/skills/prod-readiness/SKILL.md, .claude/workflows/prod-readiness.mjs) were read in full diff context and given a verdict above. Nothing deferred among changed files.

What's good

• The React Doctor version is pinned (@0.7.6) and matches the CI gate's pin exactly (.github/workflows/react-doctor.yml), avoiding ruleset drift between the ad-hoc full-repo scan and the required PR check — a real, deliberate design choice, not an oversight.
• --project apps/web,apps/mobile,packages/shared correctly and verifiably excludes the vendored design/handoff/**/*.jsx mockups (confirmed those files exist at that path) that would otherwise flood the scan with ~1054 false jsx-no-undef positives — the stated rationale checks out.
• The "GO requires zero react-doctor errors" gate and the ran:false → Deferred-coverage-gap distinction (vs. api-scope's legitimate skip) are well-specified and internally consistent in the SKILL.md documentation.
• Errors vs. warnings are cleanly separated onto the existing High / Low-Info tiers rather than inventing a new severity vocabulary.

Recommendation

Fix the High finding before merge: derive errorCount/warningCount from reactDoctor.findings (reusing audit.mjs's countBy pattern) instead of trusting an unvalidated LLM-self-reported aggregate that gates a real launch decision. The Medium (temp-path inconsistency) is worth fixing in the same pass since it's a two-line change in each file and the same PR already touches both.

---

🤖 Automated review via /pr-review

[thomasluizon]

Both findings addressed (HEAD 6d537446):

High — counts derived from findings, not self-reported. prod-readiness.mjs now derives react-doctor errorCount/warningCount from reactDoctor.findings using the same deterministic countBy pattern as audit.mjs, and overwrites them on every path (ran / skipped / null-fallback). Removed the two fields from REACT_DOCTOR_SCHEMA.properties (schema is additionalProperties:false, so the agent can no longer self-report them) and dropped the aggregate ask from the prompt. The SKILL report skeleton and the GO gate now consume the derived values. Smoke-checked: a self-reported 0/0 against 2 error + 3 warning findings is corrected to 2/3, so a mis-report can no longer produce a false GO.

Medium — one portable temp path. Both invocations now use a shell-agnostic path: prod-readiness.mjs resolves os.tmpdir() (via dynamic await import('node:os'), required because the workflow runner strips the export and async-wraps the body) into a forward-slash absolute path; audit-code-quality/SKILL.md uses ${TMPDIR:-/tmp}/rd-audit.json so an unset TMPDIR no longer writes to /rd-audit.json.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #460

Scope: PR #460 in orbit-ui-mobile (chore(skills): integrate React Doctor into prod-readiness + audit-code-quality (#243))
Recommendation: APPROVE

Summary

Tooling-only change (.claude/skills/audit-code-quality/SKILL.md, .claude/skills/prod-readiness/SKILL.md, .claude/workflows/prod-readiness.mjs) that wires the existing react-doctor required-CI-check tool into the two launch-readiness skills as a full-repo scan. No app code (apps/*, packages/*, orbit-api) is touched, so the cross-platform-parity, i18n, contract-drift, and security dimensions are all N/A by gate. The change is internally consistent on the whole — flags match the CI gate's hermetic stance, the schema/prompt/report-shape line up — with three concrete, low-blast-radius process gaps below. None of them touch app correctness or ship-blocking risk, so this clears the bar for merge.

Findings

Critical

None.

High

None.

Medium

[MEDIUM] Phase 2.5's react-doctor scan ignores the resolved {scope} token, unlike Phase 2
· dimension: Correctness (#1)
· location: orbit-ui-mobile/.claude/skills/audit-code-quality/SKILL.md (new "Phase 2.5 — React Doctor pass" section, inserted after line 96)
· issue: Phase 1 of this same skill resolves $ARGUMENTS into a {scope} token that explicitly supports "a path" ("Walk the whole repo (or a scoped path)"), and Phase 2 forwards it faithfully (args: { kind: 'code-quality', scope: '<resolved {scope}>' }). Phase 2.5's new bash command is hardcoded to --project apps/web,apps/mobile,packages/shared with no reference to {scope} at all — it always scans all three workspaces even when the user asked for a single file or folder.
· risk: Every future narrow-scoped /audit-code-quality <path> run gets its report polluted with the full standing react-doctor backlog (128 errors + 1094 warnings per this PR's own description) attributed to files outside the requested scope, defeating the purpose of scoping and misleading whoever reads the report about what was actually audited.
· fix: Either gate Phase 2.5 to only fire for blank/ui/both scope resolutions (skip it for a path scope, same as it already skips for api), or scope the react-doctor invocation itself to the resolved path when one was given.
· reference: audit-code-quality/SKILL.md Phase 1 scope table + Phase 2's own {scope} forwarding

[MEDIUM] prod-readiness SKILL.md doc and prod-readiness.mjs code disagree on when react-doctor runs
· dimension: Correctness (#1)
· location: orbit-ui-mobile/.claude/skills/prod-readiness/SKILL.md ("React Doctor (item 10) runs only for ui/both scope") vs orbit-ui-mobile/.claude/workflows/prod-readiness.mjs (if (scope !== 'api') { reactDoctor = ... })
· issue: The SKILL.md prose states react-doctor runs "only for ui/both scope" and frames api as the sole legitimate-skip case. The actual guard in the workflow is scope !== 'api', which also fires for an arbitrary path scope (Phase 1's scope table allows "A path" as a valid resolved {scope} for prod-readiness too), not just the literal ui/both values.
· risk: The Opus consolidator's own instructions only tell it how to treat reactDoctor.ran:false for a ui/both scope ("a Deferred coverage gap ... not clean"); they say nothing about a path-scoped run, so a future path-scoped prod-readiness sweep with ran:false has no documented handling — it could be silently misclassified as clean instead of Deferred.
· fix: Either tighten the code to if (scope === 'ui' || scope === 'both') to match the doc, or update the doc to say "any non-api scope" and extend the ran:false handling note to cover path scopes explicitly.
· reference: prod-readiness/SKILL.md Phase 0 binding inventory row 10 + prod-readiness.mjs React phase guard

[MEDIUM] Narrative // comment breaks this file family's established zero-comment convention
· dimension: Comment policy (#4)
· location: orbit-ui-mobile/.claude/workflows/prod-readiness.mjs, the 3-line block above const REACT_DOCTOR_SCHEMA
· issue: The added comment ("React Doctor — the deterministic React-correctness gate (also a REQUIRED CI check, .github/workflows/react-doctor.yml)...") is a // narration block with no JSDoc marker and no http(s):// URL — it references a file path, not a link — so it doesn't qualify under any of the four allowances in no-comments.cjs (JSDoc / /// / tooling directive / WHY-with-URL). Every sibling file in .claude/workflows/*.mjs (including this same file pre-diff) has zero // comments — a grep -n "^\s*//" .claude/workflows/*.mjs across the whole directory returns nothing outside this PR's addition, confirming it's a deliberate house convention, not an oversight.
· risk: Low on its own (this directory isn't wired into the ESLint local/no-comments glob), but it's the first crack in an otherwise-consistent file family and sets a precedent for narration creeping back into workflow scripts.
· fix: Delete the comment; the same rationale is already fully spelled out in prod-readiness/SKILL.md's "React Doctor (item 10)" paragraph, so nothing is lost. If a WHY anchor is wanted here, cite the actual CI workflow's own comment block via a URL-bearing reference instead of restating it.
· reference: eslint-rules/no-comments.cjs:17-24 (local/no-comments); CLAUDE.md rule 5

Low / Info

None posted (signal gate).

Subagents

Agent	Verdict
parity-checker	N/A — no apps/web/** or apps/mobile/** file changed
i18n-syncer	N/A — no user-facing strings or packages/shared/src/i18n/*.json changed
contract-aligner	N/A — no packages/shared/src/types/* / endpoints.ts, no orbit-api change
security-reviewer	N/A — no orbit-api code changed
design-reviewer	N/A — no apps/*/orbit-landing-page UI file changed

Validation

Check	Result
Lint	N/A — .claude/** is outside the npm workspaces glob (apps/*, packages/*); turbo run lint never touches these files
Type check	N/A — same reason; these are untyped .mjs/.md files outside any workspace's tsconfig
Tests	N/A — no test surface for skill/workflow prose + orchestration script; no existing test harness covers .claude/workflows/*.mjs
Build (api)	N/A — orbit-api untouched

Deferred — N/A dimensions & files not verdicted

• DESIGN.md / AI-slop (#8) — N/A, no apps/* UI file in the diff.
• Parity (#9) — N/A, no apps/web/apps/mobile file in the diff.
• i18n (#10) — N/A, no user-facing string or locale file in the diff.
• Contract drift + backward-compat (#11) — N/A, no packages/shared/src/types/* or orbit-api DTO in the diff.
• Security (#12) — reviewed at the tooling level (pinned react-doctor@0.7.6 version, -y auto-confirm scoped to a version-pinned package, mirrors the already-accepted CI pattern); no new attack surface introduced.
• Backend hard rules (#13) — N/A, orbit-api not touched.
• FEATURES.md parity (#14) — N/A, this changes internal audit tooling, not a user-facing feature surface.
• Whether react-doctor@0.7.6 actually supports the --project apps/web,apps/mobile,packages/shared flag as documented could not be independently verified in this sandbox (no network access to npx/the npm registry or react-doctor's docs site). Not flagged as a finding: the PR description cites concrete baseline numbers ("128 errors + 1094 warnings") that read as the author having actually run this exact invocation and gotten real output, which is reasonable evidence the flag works as documented.
• All 3 changed files (audit-code-quality/SKILL.md, prod-readiness/SKILL.md, prod-readiness.mjs) received a verdict — nothing changed was left unexamined.

What's good

• The --project apps/web,apps/mobile,packages/shared scoping to dodge design/handoff/**'s ~1054 false jsx-no-undef positives from vendored *.jsx mockups is a real root-cause fix (narrowing the tool's scope to actual app code), not a band-aid.
• The flags (--no-supply-chain --no-score --no-dead-code) are kept in lockstep with the already-shipped, required react-doctor.yml CI gate, so the two invocations stay hermetic and comparable.
• reactDoctor.errorCount/warningCount are derived from the returned findings array rather than trusted as an agent-reported aggregate — a good defense against an LLM agent mis-summing its own output.
• The REACT_DOCTOR_JSON_OUT temp path is built via node:os's tmpdir() with a \→/ normalization rather than a hardcoded OS-specific path, which is the right way to keep this portable across the Windows dev machine these workflows otherwise assume.
• The GO-verdict tightening (zero react-doctor errors required) is explicit and calibrated, with the current 128-error baseline named up front in the PR body rather than swept under the rug.

Recommendation

Merge as-is; the three Medium findings are worth a fast follow-up (either in a quick amend or a tracked issue) since they're each a one-line fix, but none of them touch app correctness, security, or ship-blocking behavior — they only affect the accuracy of future tooling reports.