docs(tooling): align docs with repository simplification#477
Merged
docs(tooling): align docs with repository simplification#477
Conversation
Sync docs with current repo structure and moved mobile app path. Made-with: Cursor
|
Important Review skippedToo many files! This PR contains 173 files, which is 23 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (59)
📒 Files selected for processing (173)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughRepository documentation, CI, ownership, and workspace metadata were updated to remove deprecated contexts (e.g., apps/ai-orchestrator, packages/ai-config, packages/content-models), introduce and document Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Update docs and ownership paths to match moving Expo from mobile/expo to apps/mobile. Made-with: Cursor
![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
🧹 Nitpick comments (1)
apps/mobile/README.md (1)
85-85: Fix awkward phrasing about generated files.The phrase "generated files under
packages/graphql/src/graphql-env.d.ts" is misleading sincegraphql-env.d.tsis a file, not a directory.📝 Suggested rewording
-- **Codegen:** Types and operations come from the shared package `@forge/graphql` (schema: `apps/cms/schema.graphql`). When the Strapi schema changes, run from the **repo root**: `pnpm run codegen`. That regenerates `packages/graphql`; do not hand-edit generated files under `packages/graphql/src/graphql-env.d.ts`. +- **Codegen:** Types and operations come from the shared package `@forge/graphql` (schema: `apps/cms/schema.graphql`). When the Strapi schema changes, run from the **repo root**: `pnpm run codegen`. That regenerates `packages/graphql`; do not hand-edit the generated file `packages/graphql/src/graphql-env.d.ts`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/README.md` at line 85, The README line misleadingly refers to "generated files under `packages/graphql/src/graphql-env.d.ts`" even though `graphql-env.d.ts` is a single file; update the sentence to clearly state that `pnpm run codegen` regenerates the shared package `packages/graphql` and that the generated file `packages/graphql/src/graphql-env.d.ts` (not a directory) must not be hand-edited—keep the instruction tied to the command `pnpm run codegen` and the package `@forge/graphql`/schema path so maintainers know what to regenerate and which specific generated file to avoid editing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/AGENTS.md`:
- Line 16: The removal of the `apps/ai-orchestrator` entry from the
apps/AGENTS.md doc left ownership out of sync with the repo workspaces; restore
a clear, doc-safe ownership entry for `apps/ai-orchestrator` (either re-add the
original bullet or replace it with a short deprecation note like
"apps/ai-orchestrator: deprecated — see current owner or migration plan") so the
document accurately reflects that the workspace still exists and who owns it;
update the single line referencing `apps/ai-orchestrator` to include owner/team
and a brief status to avoid misrouting issues while the orchestrator is being
removed or migrated.
In `@CLAUDE.md`:
- Around line 65-70: The CLAUDE.md scope-to-folder table is incorrect for the
mobile scopes; update the table rows so `mobile-expo` maps to `mobile/expo/` and
add/restore the mobile platform rows `mobile-ios` -> `mobile/ios/` and
`mobile-android` -> `mobile/android/`, while keeping `web` -> `apps/web/`, `cms`
-> `apps/cms/`, and `graphql` -> `packages/graphql/`; edit the table in
CLAUDE.md (the block that currently lists `Scope | Allowed folders`) to replace
the `mobile-expo` entry and add the two mobile entries exactly as described so
the documented mappings match the actual codebase layout.
---
Nitpick comments:
In `@apps/mobile/README.md`:
- Line 85: The README line misleadingly refers to "generated files under
`packages/graphql/src/graphql-env.d.ts`" even though `graphql-env.d.ts` is a
single file; update the sentence to clearly state that `pnpm run codegen`
regenerates the shared package `packages/graphql` and that the generated file
`packages/graphql/src/graphql-env.d.ts` (not a directory) must not be
hand-edited—keep the instruction tied to the command `pnpm run codegen` and the
package `@forge/graphql`/schema path so maintainers know what to regenerate and
which specific generated file to avoid editing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfc4118a-9d97-4533-b732-65bf6558fe33
📒 Files selected for processing (12)
AGENTS.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdSECURITY.mdapps/AGENTS.mdapps/README.mdapps/mobile/README.mdapps/web/AGENTS.mdapps/web/README.mdpackages/AGENTS.mdpackages/README.md
💤 Files with no reviewable changes (1)
- packages/AGENTS.md
Collapse mobile linting to one Expo job and remove legacy iOS/Android lint paths. Made-with: Cursor
Drop remaining ai-orchestrator references from scripts, label scopes, and lockfile. Made-with: Cursor
Standardize script names and add root shortcuts for mobile lint/test/typecheck/build. Made-with: Cursor
Rename mobile package id and update scripts, CI filters, docs, and lockfile references. Made-with: Cursor
Detect mobile/web/cms/graphql in affected and gate generated-code checks to cms/graphql changes. Made-with: Cursor
Apply cms/graphql affected condition on generated-code job instead of step-level gating. Made-with: Cursor
Use affected matrix jobs so lint/test/build run per project in parallel. Made-with: Cursor
Run affected lint/test/build directly from affected outputs without waiting on generated-code. Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
1-42:⚠️ Potential issue | 🟠 MajorLockfile drift is currently blocking CI.
pnpm install --frozen-lockfilefails becausepnpm-lock.yamlis out of sync with manifests (express: ^5.2.1). Please regenerate and commit the lockfile state for this branch before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 1 - 42, The lockfile drift means pnpm-lock.yaml is out of date with the manifests; using the packageManager entry ("pnpm@9.12.3") run a fresh installation to regenerate the lockfile (e.g., pnpm install using the same pnpm version), verify the manifest dependency mismatch (express ^5.2.1) is resolved, commit the updated pnpm-lock.yaml, and re-run pnpm install --frozen-lockfile to confirm CI will pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/rules/bounded-context-folder-guard.mdc:
- Around line 24-25: Clarify precedence between Rule 2 and Rule 3 by updating
the policy text so that Rule 3 (shared packages) is explicitly allowed to
override Rule 2 for the packages/graphql/ path: state that Rule 2 forbids
cross-context changes by default but Rule 3 permits changes to packages/graphql/
when the change is an explicit contract/schema modification (or regenerated
code), and add a short sentence that Rule 3 takes precedence for
packages/graphql/ to make enforcement deterministic.
In @.cursor/rules/cross-platform-demo.mdc:
- Line 32: Replace the placeholder JAVA_HOME="..." in the runnable command so it
doesn't override a valid environment variable: either remove the inline
JAVA_HOME assignment so the command uses the existing JAVA_HOME set earlier
(i.e., keep "cd apps/mobile && npx expo run:android"), or replace "..." with a
clear instruction to set JAVA_HOME to a real JDK path before running the
command; update the line containing the command string `cd apps/mobile &&
JAVA_HOME="..." npx expo run:android` accordingly.
In @.github/workflows/ci.yml:
- Around line 59-67: The CI step currently masks failures by appending "|| true"
and "|| echo """, causing TURBO_OUT/AFFECTED to silently default and skip
downstream jobs; update the Detect affected packages step to fail fast by
removing the "|| true" after the pnpm turbo run and removing the fallback "||
echo """, enable strict shell behavior (e.g., set -o pipefail -e) so any failure
in computing TURBO_OUT or parsing with jq surfaces, and add explicit error
handling/logging that prints TURBO_OUT and jq errors before exiting non-zero;
refer to the variables/commands TURBO_OUT, AFFECTED, the pnpm turbo run
invocation, and the jq parsing pipeline when making these changes.
In @.github/workflows/issue-labels.yml:
- Line 27: Update the validScopes array to match the current scope taxonomy: in
every place where the constant validScopes is defined (the arrays that currently
include 'contracts', 'mobile-ios', 'mobile-android'), replace those entries with
the new scopes 'graphql' and 'mobile-expo' so titles using the current scopes
are recognized; ensure both occurrences of the validScopes definition are
changed to the same updated list.
In `@CODEOWNERS`:
- Around line 5-8: Add explicit CODEOWNERS entries for the native mobile
subtrees so reviews for platform-specific changes route to the mobile team;
update the file to add lines for /apps/mobile/ios/ and /apps/mobile/android/
(same owner as /apps/mobile/ — `@JesusFilm/engineering/forge`) so those
directories don't fall back to the global owner and get proper context-specific
review routing.
---
Outside diff comments:
In `@package.json`:
- Around line 1-42: The lockfile drift means pnpm-lock.yaml is out of date with
the manifests; using the packageManager entry ("pnpm@9.12.3") run a fresh
installation to regenerate the lockfile (e.g., pnpm install using the same pnpm
version), verify the manifest dependency mismatch (express ^5.2.1) is resolved,
commit the updated pnpm-lock.yaml, and re-run pnpm install --frozen-lockfile to
confirm CI will pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e082d188-abca-4394-90d3-4c4fc25158b9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.cursor/rules/bounded-context-folder-guard.mdc.cursor/rules/cross-platform-demo.mdc.github/workflows/ci.yml.github/workflows/issue-labels.ymlCODEOWNERSapps/mobile/.env.exampleapps/mobile/README.mdapps/mobile/package.jsonpackage.json
✅ Files skipped from review due to trivial changes (1)
- apps/mobile/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/mobile/README.md
Switch affected output to JSON services array and drive matrix jobs from fromJson(). Made-with: Cursor
Use turbo ls JSON output with full @forge/* services so new services work without CI edits. Made-with: Cursor
Print resolved affected services JSON in the affected job for easier CI debugging. Made-with: Cursor
Commit remaining repo simplification moves and deletions across apps, mobile, and packages. Made-with: Cursor
Commit post-hook workflow formatting changes. Made-with: Cursor
State Rule 3 precedence for packages/graphql to keep enforcement deterministic. Made-with: Cursor
Use existing JAVA_HOME setup and remove placeholder override from demo command. Made-with: Cursor
Update issue and PR label scope allowlist to current contexts. Made-with: Cursor
Update issue and PR label valid scopes to use mobile instead of mobile-expo. Made-with: Cursor
Align remaining scope references in Claude and Cursor guidance to mobile. Made-with: Cursor
Make graphql depend on cms via workspace protocol and ensure mobile/web both depend on graphql. Made-with: Cursor
Run generated-code job only when @forge/graphql is in affected services. Made-with: Cursor
Update CI and docs to use explicit turbo commands instead of removed root scripts. Made-with: Cursor
Commit pending CI and root script updates. Made-with: Cursor
Handle turbo ls JSON packages.items shape so affected services are detected reliably. Made-with: Cursor
Pass max-warnings directly to pnpm run to avoid eslint interpreting it as a file pattern. Made-with: Cursor
Exclude apps/cms/.strapi and apps/cms/types/generated from ESLint checks. Made-with: Cursor
Split lint config by project and inherit from top-level shared base. Made-with: Cursor
Drop no-error-on-unmatched-pattern from cms lint script. Made-with: Cursor
Add typecheck scripts for web, cms, and graphql plus root/turbo support. Made-with: Cursor
Move graphql->cms link to devDependencies and ignore tsconfig.tsbuildinfo artifacts. Made-with: Cursor
Remove irrelevant TS suppression comments. Fix remaining type errors in tests, section mapping, and link navigation. Made-with: Cursor
Limit mobile type discovery to its local @types packages. Avoid workspace-level React type collisions in CI typecheck. Made-with: Cursor
Map react type resolution to mobile-local @types/react files. Prevents pnpm virtual-store fallback to React 18 types in forced CI checks. Made-with: Cursor
Remove unnecessary mobile tsconfig typeRoots override. Retain only React path mappings needed for stable CI type resolution. Made-with: Cursor
Map react runtime modules in Jest so mobile can use the same tsconfig for both runtime tests and TypeScript typecheck. Made-with: Cursor
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
1 participant
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.
Summary
Align repository documentation with the current simplification work by removing references to deleted contexts (
apps/ai-orchestrator,packages/ai-config,packages/content-models) and updating mobile docs to the newapps/mobilelocation.Resolves #476
Contracts Changed
Regeneration Required
Validation
Made with Cursor
Summary by CodeRabbit