Skip to content

chore: clean harness, split CLAUDE.md, harden PR reviewer, multi-issue workflow - #106

Merged
thomasluizon merged 1 commit into
mainfrom
chore/clean-harness-105
May 22, 2026
Merged

thomasluizon merged 1 commit into
mainfrom
chore/clean-harness-105

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

Summary

Six-part harness overhaul per the issue. Pure infrastructure — no application behavior changes.

Closes #105.
Paired with: thomasluizon/orbit-api#170 (orbit-api side: scoped CLAUDE.md split + .claudeignore).

Part 1 — Cleanup (~57 MB removed)

  • .opencode/ deleted entirely (non-Claude tooling).
  • .agents/ deleted entirely (OpenCode artifact dump — PRDs/plans/reports/reviews/stories). .agents/WORKFLOW.md preserved at repo root.
  • AGENTS.md, opencode.json, .claude/rules/task-management.md deleted.
  • .claudeignore added.
  • temporary screenshots/ gitignored.
  • Dead ios script removed from apps/mobile/package.json (Android-only app).

Part 2 — claude-review.yml hardening

Keeps the default-to-approve + noise-filter structure. Adds three new high-signal categories the reviewer must check:

  1. Stale/dead code introduced by THIS PR — zero-ref exports, unused imports, orphaned i18n keys, unread env vars.
  2. Workarounds vs. root-cause fixes — TODO/FIXME/HACK/XXX, commented-out code, swallowed errors, as any, unjustified eslint-disable, hardcoded fallbacks masking config bugs, copy-pasted logic that belongs in packages/shared.
  3. Useless comments — restate-the-code, reference-the-PR/ticket, untracked TODOs.

Every finding must include file:line + the concrete root-cause fix. Self-check clause preserved (zero high-signal → MUST APPROVE).

Part 3 — Scoped CLAUDE.md split + Code Standards

File Owns
CLAUDE.md (root, ~110 lines) Cross-cutting only: parity rule, 10-rule Code Standards, subagent-delegation default, security boundaries, DESIGN.md pointer, API-repo access, LSP wiring, git, testing, WORKFLOW.md pointer.
apps/web/CLAUDE.md Next.js 15 App Router, Server Components, Zustand + TanStack patterns, BFF proxy, Server Actions, web-only conventions.
apps/mobile/CLAUDE.md Expo SDK 53 + Expo Router, NativeWind, direct API, SQLite offline queue, React 19 / Compiler rules, Android-only realities.
packages/shared/CLAUDE.md Zod-first types, endpoints.ts, query keys, i18n parity, theme tokens, test factories.

10-rule Code Standards in root: root cause over workarounds, delete unused, no any, no console.log, comments policy (WHY only + public-API docs), no premature abstraction, function size caps (~50 soft / ~100 hard), error handling at boundaries, naming discipline, DRY at the right level.

Part 4 — Harness gaps

Hooks (.claude/settings.json + .claude/hooks/)

Six PostToolUse scripts. Scoped to the changed file path (no full-repo scans). Exit 2 + stderr feedback to Claude when violations land.

Hook What it catches
forbid-ts-antipatterns.mjs : any, as any, as unknown as X, console.log, @ts-ignore/@ts-expect-error, unjustified eslint-disable in .ts/.tsx outside __tests__.
flag-new-todos.mjs New untracked TODO/FIXME/HACK/XXX/BUG markers. Allows TODO #123: ... references.
parity-nudge.mjs Session-scoped state: nudges after 3+ edits to one workspace with zero in its mirror.
csharp-authz.mjs orbit-api Controller without [Authorize] or [AllowAnonymous].
csharp-tz.mjs orbit-api Application code using raw DateTime.UtcNow outside Created/UpdatedAtUtc and cache keys.
csharp-fluentconfig.mjs EF migration creates a table without a matching IEntityTypeConfiguration<T>.

Meta files (.claude/hooks/, .claude/scripts/, .github/workflows/, .md) are skipped so the hooks dont self-trigger.

Skills (.claude/skills/)

5 new skills, each with frontmatter description: for auto-invocation:

Skill Trigger
add-api-endpoint "Add an API endpoint" — cross-repo scaffold (orbit-api Controller + CQRS + validator + test, plus packages/shared + Server Action + apiClient + query key).
add-shared-type "Add a shared type" — Zod schema + barrel export + optional consumer hook stubs.
add-feature-page "Add a new page/screen" — parallel apps/web/app/(app)/X/page.tsx + apps/mobile/app/X.tsx + i18n in both locales.
add-cqrs-handler "Add an internal CQRS handler" — orbit-api Command/Query + Validator + Handler + unit test (no client work).
add-migration "Add a database migration" — Confirm-first; entity update + FluentConfig + EF migration + DbContext + tests.

Subagents (.claude/agents/)

4 new subagents:

Subagent Policy Behavior
parity-checker Auto Reports PAIRED / PARTIAL / MISSING per mirror file after web/mobile edits.
i18n-syncer Auto Detects drift between en.json and pt-BR.json.
contract-aligner Auto on /review Cross-checks packages/shared/src/types/* and endpoints.ts against orbit-api DTOs and Controllers.
security-reviewer Explicit-only Reviews orbit-api Controllers + Infrastructure for missing [Authorize], webhook signature verification, JWT leaks, CORS gaps, input-validation gaps.

LSP

  • .claude/mcp.json.example documents the C# LSP MCP server entry. The active .mcp.json is gitignored — developers copy from the example and fill in env vars.
  • .claude/scripts/csharp-lsp.mjs is a fallback wrapper that bridges OmniSharp/Roslyn (or Microsoft.CodeAnalysis.LanguageServer via env override) to MCP. Used when the registered package cannot be installed.
  • Documented in both repos root CLAUDE.md so the wiring is discoverable from either side.

Part 5 — Auto-invocation descriptions

Every skill + subagent description matches the table in the issues Part 5 comment verbatim — trigger verbs/nouns, inputs, output. add-migration is Confirm-first; security-reviewer is Explicit-only.

Smoke test: this sessions reload showed the 5 new skills registered correctly in the harness. Live trigger-phrase routing will be verified post-merge in fresh sessions.

Part 6 — Multi-issue workflow + subagent-by-default

  • WORKFLOW.md moved to repo root with a new "Multi-issue path" section.
  • prime.md / plan.md / implement.md extended with multi-issue mode: ≥ 2 numeric args triggers the parallel path. Each subagent runs the single-issue flow inside its paired worktree (mobile + orbit-api). 3 concurrent max. Failures dont halt siblings — surfaced in the aggregate report.
  • Subagent-delegation-default section added to root CLAUDE.md (delegate by default for research, multi-issue, independent edits, long-running tasks; dont delegate for single-file edits or conversational work).

Misc

  • All .claude/commands/*.md updated: .agents/ paths → .claude/, AGENTS.md refs → CLAUDE.md.
  • .claude/plans/, .claude/PRDs/, .claude/stories/, .claude/reviews/, .claude/reports/ added to .gitignore (local scratch — never committed).
  • apps/web ESLint migration: next lint was removed in Next.js 16. Migrated apps/web to ESLint v9 flat config (eslint.config.mjs spreading eslint-config-next + core-web-vitals + typescript). Mobile auto-generated eslint.config.js with eslint-config-expo/flat during the install.

Known: lint now surfaces pre-existing code issues

Before this PR, npm run lint was broken (exited 1 because next lint no longer exists in Next.js 16). Now it works — and surfaces 91 errors + 74 warnings in apps/web that were always there:

Rule Count
@typescript-eslint/no-unused-vars 51
react-hooks/set-state-in-effect 47
@typescript-eslint/no-explicit-any 16
react-hooks/exhaustive-deps 16
react-hooks/refs 16
react-hooks/preserve-manual-memoization 5
@next/next/no-img-element 3
...misc (each ≤ 2) 11

These are pre-existing code-level issues in files this PR doesnt touch. Much of this area is under active rework in redesign/v2 (which already fixed many of these patterns there). Fixing them in this PR would duplicate work and create rebase conflicts when redesign/v2 lands.

CI (test.yml) does not run lint, so this PR is mergeable. The lint tool now works correctly — that is the win. A follow-up issue should be opened for the React 19 / unused-vars cleanup in main.

Validation

  • npm run type-check: PASS (3/3 workspaces clean).
  • npm test: PASS (1552 web tests, all mobile + shared).
  • npm run lint: TOOL WORKS — surfaces pre-existing findings above.

Paired PRs

🤖 Generated with Claude Code

…g, multi-issue workflow

Closes #105.

## Part 1 — Cleanup
- Delete .opencode/ (1.3MB of non-Claude tooling), .agents/ (OpenCode artifact dump), AGENTS.md, opencode.json, .claude/rules/task-management.md.
- Add .claudeignore (mirrors orbit-api's): node_modules/, build outputs, native artifacts, secrets, .claude/ scratch dirs.
- Add temporary screenshots/ to .gitignore.
- Drop dead `ios` script from apps/mobile/package.json (Orbit mobile is Android-only).

## Part 2 — claude-review.yml hardening
Adds three new high-signal categories to the PR reviewer prompt while preserving the existing default-to-approve + noise-filter structure:
1. Stale/dead code introduced by THIS PR (zero-ref exports, unused imports, orphaned i18n keys, etc.).
2. Workarounds vs. root-cause fixes (TODOs, swallowed errors, `as any`, unjustified eslint-disable, etc.).
3. Useless comments (restating code, referencing the PR/ticket, untracked TODOs).
Each finding must include file:line + concrete root-cause fix. Self-check clause preserved (zero high-signal → MUST APPROVE).

## Part 3 — Scoped CLAUDE.md split + Code Standards
- Root CLAUDE.md (~110 lines): cross-cutting only — parity rule, 10-rule Code Standards, subagent-delegation default, security boundaries, DESIGN.md pointer, API repo access, LSP wiring, git, testing, WORKFLOW.md pointer.
- apps/web/CLAUDE.md: Next.js 15 App Router specifics, Server Components, Zustand + TanStack, BFF proxy, Server Actions.
- apps/mobile/CLAUDE.md: Expo SDK 53 + Expo Router, NativeWind, direct API, SQLite offline queue, React 19 / Compiler rules.
- packages/shared/CLAUDE.md: Zod-first types, endpoints.ts, query keys, i18n parity, theme tokens.
Each rule lives in exactly one file; lower files cross-reference upward.

## Part 4 — Harness gaps
- .claude/settings.json + 6 PostToolUse hooks:
  - forbid-ts-antipatterns: blocks `: any`, `as any`, `as unknown as X`, `console.log`, `@ts-ignore/@ts-expect-error`, unjustified `eslint-disable` in .ts/.tsx outside __tests__.
  - flag-new-todos: blocks new untracked TODO/FIXME/HACK/XXX/BUG markers (allows `TODO #123: ...` references).
  - parity-nudge: session-scoped — warns after 3+ edits to one workspace with zero in its mirror.
  - csharp-authz: warns when an orbit-api Controller is edited without [Authorize] or [AllowAnonymous].
  - csharp-tz: warns on raw DateTime.UtcNow outside Created/UpdatedAtUtc and cache keys.
  - csharp-fluentconfig: warns when a migration adds a new table without a matching FluentConfig.
- 5 new skills (`add-shared-type`, `add-api-endpoint`, `add-feature-page`, `add-cqrs-handler`, `add-migration`).
- 4 new subagents (`parity-checker`, `i18n-syncer`, `contract-aligner`, `security-reviewer`).
- C# LSP wiring: .claude/mcp.json.example documents the MCP server entry; .claude/scripts/csharp-lsp.mjs is a fallback wrapper that bridges OmniSharp/Roslyn to MCP. Active when the session is launched from orbit-ui-mobile.

## Part 5 — Auto-invocation descriptions
Each new skill + subagent has a tight `description:` in its frontmatter matching the issue's table (trigger phrases, inputs, output). add-migration is Confirm-first; security-reviewer is Explicit-only; others auto-invoke.

## Part 6 — Multi-issue workflow + subagent default
- WORKFLOW.md moved from .agents/ → repo root. Adds "Multi-issue path" section: `/prime <A> <B> <C>` creates paired worktrees, `/plan` and `/implement` dispatch one subagent per worktree, 3 concurrent max, failures don't halt siblings.
- prime.md / plan.md / implement.md extended with multi-issue mode detection (≥ 2 numeric args triggers the parallel path).
- Subagent delegation default section added to root CLAUDE.md (delegate by default for research, multi-issue, independent edits, long-running tasks; don't delegate for single-file edits or conversational work).

## Misc
- All command files (.claude/commands/*) updated: .agents/ paths → .claude/, AGENTS.md refs → CLAUDE.md.
- .claude/plans, /reports, /PRDs, /stories, /reviews added to .gitignore (local scratch — never committed).
- apps/web ESLint setup: `next lint` was removed in Next.js 16; migrated apps/web to ESLint v9 flat config (eslint.config.mjs spreading eslint-config-next + core-web-vitals + typescript). `eslint-config-expo/flat` auto-wired for mobile.

## Known: lint surfaces pre-existing code issues
Now that the lint tool works, it surfaces 91 errors + 74 warnings in apps/web (mostly `react-hooks/set-state-in-effect`, `@typescript-eslint/no-unused-vars`, `react-hooks/refs`). These are pre-existing code-level issues NOT introduced by this PR — much of this area is under active rework in redesign/v2. Fixing them is a separate concern; CI's test.yml doesn't run lint, so this PR doesn't regress CI. Tracked as follow-up.

## Validation
- npm run type-check: PASS (3/3 workspaces).
- npm test: PASS (1552 web tests, all mobile + shared tests).
- npm run lint: PASS in shape (tool works); surfaces pre-existing findings as documented above.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel

vercel Bot commented May 22, 2026

Copy link
Copy Markdown

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 Ignored May 22, 2026 6:28pm

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 2ad121e into main May 22, 2026
7 of 8 checks passed
@thomasluizon
thomasluizon deleted the chore/clean-harness-105 branch May 22, 2026 19:00
thomasluizon added a commit that referenced this pull request May 22, 2026
Brings the harness overhaul (#106) into redesign/v2: scoped CLAUDE.md split, 6 PostToolUse hooks, 5 new skills, 4 new subagents, C# LSP wiring, multi-issue workflow.

Conflict resolutions:
- AGENTS.md: deleted (main wins; replaced by scoped CLAUDE.md files).
- CLAUDE.md: took main's lean cross-cutting version with 10-rule Code Standards.
- apps/web/eslint.config.mjs: combined main's three-config spread (next + core-web-vitals + typescript) with redesign/v2's broader ignores (e2e/, coverage/, public/, *.config.*).
- apps/web/package.json: added eslint v9 declared devDep from main.
- package-lock.json: regenerated from main with --legacy-peer-deps (pre-existing react-native-worklets peer mismatch in apps/mobile).

Validation:
- npm run type-check: PASS (3/3 workspaces).
- npm test: PASS (1476 web tests).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

Codebase cleanup + CLAUDE.md / PR-review hardening (clean-code enforcement)

1 participant