[feat] Scaffold the mobile web app at /m (1/12) - #5679
Conversation
dosubot
Bot
added
size:XL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request adds a mobile Next.js workspace under ChangesMobile application foundation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/skills/mobile-app-structure/SKILL.md (1)
52-53: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRun filtered mobile commands from
web/.Both skill files omit the workspace directory for
pnpm --filtercommands. Add the directory in both instructions.
.agents/skills/mobile-app-structure/SKILL.md#L52-L53: Prefix both filtered commands withFrom web/, or use an explicit--dir webform..agents/skills/mobile-shadcn-conventions/SKILL.md#L35-L35: Prefix token generation withFrom web/; keepweb/mobile/for the shadcn CLI instructions.Based on learnings, run mobile commands from the documented
web/workspace. As per path instructions,web/mobile/AGENTS.mddocuments commands fromweb/.Source: Learnings
🧹 Nitpick comments (2)
.agents/skills/mobile-app-structure/SKILL.md (1)
39-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the data boundary.
This rule permits “thin fetchers in
lib/”.web/mobile/AGENTS.mdsays data and state come from@agenta/*packages only. Confirm that alib/fetcher may only wrap an@agenta/*client, then state that restriction here.Proposed clarification
- components get data via hooks from `@agenta/*` packages or thin fetchers in `lib/`. + components get data via hooks from `@agenta/*` packages. Thin fetchers in + `lib/` may only wrap those package clients.As per path instructions,
web/mobile/AGENTS.mdlimits data and state to@agenta/*packages.web/mobile/eslint.config.mjs (1)
22-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the plugin's recommended preset instead of two manually-picked rules.
eslint-plugin-react-hooks7.1.1 bundles compiler-aware rules (purity,refs,set-state-in-effect,immutability,static-components, and more) in itsrecommended/recommended-latestflat presets, enabled by default. Manually setting onlyrules-of-hooksandexhaustive-depsskips all of these.Spread the preset so the mobile app gets full React 19/compiler-aware Hooks linting:
♻️ Proposed refactor
const config = [ eslint.configs.recommended, ...tseslint.configs.recommended, ...tseslint.configs.stylistic, + reactHooks.configs.flat.recommended, { plugins: { import: importPlugin, "react-hooks": reactHooks, }, rules: { ... - "react-hooks/rules-of-hooks": "error", - "react-hooks/exhaustive-deps": "warn", "`@typescript-eslint/no-explicit-any`": "error",Ask on the web to confirm the exact rule set shipped in
reactHooks.configs.flat.recommendedfor the pinned^7.1.1version before applying, since presets evolve between minor releases.Also applies to: 60-61
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e7a7cb19-8be6-4367-a72a-4f487fd490c9
⛔ Files ignored due to path filters (2)
web/mobile/src/styles/theme.generated.cssis excluded by!**/*.generated.*web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.agents/skills/mobile-app-structure/SKILL.md.agents/skills/mobile-motion-patterns/SKILL.md.agents/skills/mobile-shadcn-conventions/SKILL.md.claude/skills/mobile-app-structure.claude/skills/mobile-motion-patterns.claude/skills/mobile-shadcn-conventionshosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.dev.ymlweb/ee/docker/Dockerfile.devweb/entrypoint.shweb/mobile/.gitignoreweb/mobile/AGENTS.mdweb/mobile/CLAUDE.mdweb/mobile/components.jsonweb/mobile/docker/Dockerfile.ghweb/mobile/eslint.config.mjsweb/mobile/next-env.d.tsweb/mobile/next.config.tsweb/mobile/package.jsonweb/mobile/postcss.config.mjsweb/mobile/public/.gitkeepweb/mobile/scripts/generate-shadcn-tokens.tsweb/mobile/src/components/ui/button.tsxweb/mobile/src/components/ui/skeleton.tsxweb/mobile/src/lib/motion/presets.tsweb/mobile/src/lib/utils.tsweb/mobile/src/pages/_app.tsxweb/mobile/src/pages/_document.tsxweb/mobile/src/pages/index.tsxweb/mobile/src/styles/globals.cssweb/mobile/tsconfig.jsonweb/oss/docker/Dockerfile.devweb/package.jsonweb/pnpm-workspace.yamlweb/turbo.json
| output: "standalone", | ||
| reactStrictMode: true, | ||
| pageExtensions: ["ts", "tsx"], | ||
| productionBrowserSourceMaps: true, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)web/mobile/next.config\.ts$|(^|/)package\.json$|(^|/)pnpm-lock\.yaml$' || true
echo "== next config =="
cat -n web/mobile/next.config.ts
echo "== package scripts/dependencies relevant =="
if [ -f web/mobile/package.json ]; then cat -n web/mobile/package.json; fi
if [ -f web/package.json ]; then cat -n web/package.json; fi
echo "== source map settings in repo =="
rg -n "productionBrowserSourceMaps|next-sourcemap|source maps|sourcemaps|Production" web mobile .github 2>/dev/null || trueRepository: Agenta-AI/agenta
Length of output: 14562
Disable production browser source maps unless authorized tracking requires them.
Setting productionBrowserSourceMaps: true embeds source comments in client-ready JavaScript and creates source map artifacts, which can expose implementation details. If you need source maps for error tracking, upload them only to an authorized error tracking service and keep this option disabled for client delivery.
Proposed fix
- productionBrowserSourceMaps: true,
+ productionBrowserSourceMaps: false,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| productionBrowserSourceMaps: true, | |
| productionBrowserSourceMaps: false, |
6534e92 to
39a672d
Compare
|
Verified all three against the code. Two were real and are fixed at the root; one I am not taking. 1. Greedy 2. Missing edition public dirs in the gh image — real, and it exposed a stack problem. The fix existed on the full branch but sat 28 commits later, which put it in PR 4 of this stack. So this PR really did ship an image whose container exits on start, and no reviewer of this PR could have known PR 4 repairs it. I fixed the cause rather than appending a patch: the repair is now folded into the commit that introduced the Dockerfile, so it never existed as a broken state. I audited the rest of the branch for the same shape (a later lane repairing an earlier lane) and this was the only case that shipped a genuinely broken state; the others are ordinary iteration on files an earlier lane introduced. 3. Also rebased onto current |
39a672d to
a8ad264
Compare
Railway Preview Environment
|
|
The four failing acceptance checks here are not from this PR. They are the repo-wide preview outage, and I traced the mechanism in case it helps whoever owns #5650. What fails: every acceptance job dies in Not specific to this stack. The Mechanism.
So the cloned environment runs with whatever the template environment carries, while Where a fix would go: either have the clone path apply the same secret set the legacy deploy does (the I have not changed any of it. It is not this stack's code, it is blocking other people's PRs too, and I cannot exercise Railway plumbing locally to verify a fix. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
a8ad264 to
885738f
Compare
|
Production browser source maps: not taking here, and it is not a mobile decision.
If the repo wants source maps off in production, that is one change to both configs plus a decision about how errors get symbolicated afterward. That belongs to whoever owns the OSS build config, not to the PR that scaffolds a second Next app. Worth noting the finding is about disclosure, not a vulnerability: the maps expose the same TypeScript already published in this open-source repo. |
Listing `inputs` for `@agenta/mobile#build` replaces Turbo's default file selection rather than adding to it, so the task hashed only `src`, `public` and four config files. `package.json` was not among them: a dependency bump changed nothing the cache could see, and CI would serve a build made against the old dependency tree. `$TURBO_DEFAULT$` restores every tracked file in the package as the base, with tests and markdown removed since neither reaches the bundle. Verified by hash: editing `web/mobile/package.json` moves `@agenta/mobile#build` from f7a97e16 to 9aa1f06c, where before it did not move at all.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a8594b8-41d7-4d50-8b96-5d72966c7c9d
⛔ Files ignored due to path filters (2)
web/mobile/src/styles/theme.generated.cssis excluded by!**/*.generated.*web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.agents/skills/mobile-app-structure/SKILL.md.agents/skills/mobile-motion-patterns/SKILL.md.agents/skills/mobile-shadcn-conventions/SKILL.md.claude/skills/mobile-app-structure.claude/skills/mobile-motion-patterns.claude/skills/mobile-shadcn-conventionshosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.dev.ymlweb/ee/docker/Dockerfile.devweb/entrypoint.shweb/mobile/.gitignoreweb/mobile/AGENTS.mdweb/mobile/CLAUDE.mdweb/mobile/components.jsonweb/mobile/docker/Dockerfile.ghweb/mobile/eslint.config.mjsweb/mobile/next-env.d.tsweb/mobile/next.config.tsweb/mobile/package.jsonweb/mobile/postcss.config.mjsweb/mobile/public/.gitkeepweb/mobile/scripts/generate-shadcn-tokens.tsweb/mobile/src/components/ui/button.tsxweb/mobile/src/components/ui/skeleton.tsxweb/mobile/src/lib/motion/presets.tsweb/mobile/src/lib/utils.tsweb/mobile/src/pages/_app.tsxweb/mobile/src/pages/_document.tsxweb/mobile/src/pages/index.tsxweb/mobile/src/styles/globals.cssweb/mobile/tsconfig.jsonweb/oss/docker/Dockerfile.devweb/package.jsonweb/pnpm-workspace.yamlweb/turbo.json
🚧 Files skipped from review as they are similar to previous changes (31)
- web/entrypoint.sh
- web/mobile/CLAUDE.md
- .claude/skills/mobile-app-structure
- web/pnpm-workspace.yaml
- web/oss/docker/Dockerfile.dev
- web/turbo.json
- web/mobile/src/lib/utils.ts
- hosting/docker-compose/oss/docker-compose.dev.yml
- web/mobile/.gitignore
- web/mobile/package.json
- .agents/skills/mobile-motion-patterns/SKILL.md
- web/mobile/src/pages/_app.tsx
- web/mobile/src/pages/index.tsx
- web/package.json
- web/mobile/next.config.ts
- web/mobile/scripts/generate-shadcn-tokens.ts
- hosting/docker-compose/ee/docker-compose.dev.yml
- .agents/skills/mobile-shadcn-conventions/SKILL.md
- web/ee/docker/Dockerfile.dev
- .agents/skills/mobile-app-structure/SKILL.md
- .claude/skills/mobile-motion-patterns
- web/mobile/src/components/ui/skeleton.tsx
- web/mobile/docker/Dockerfile.gh
- web/mobile/src/components/ui/button.tsx
- web/mobile/postcss.config.mjs
- web/mobile/tsconfig.json
- .claude/skills/mobile-shadcn-conventions
- web/mobile/next-env.d.ts
- web/mobile/components.json
- web/mobile/src/lib/motion/presets.ts
- web/mobile/AGENTS.md
| ...(includePrettierRule | ||
| ? { | ||
| "prettier/prettier": [ | ||
| "error", | ||
| { | ||
| printWidth: 100, | ||
| tabWidth: 4, | ||
| useTabs: false, | ||
| semi: false, | ||
| bracketSpacing: false, | ||
| }, | ||
| ], | ||
| } | ||
| : {}), | ||
| }, | ||
| }, | ||
| ...(includePrettierRule ? [eslintPluginPrettier] : []), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm that the recommended config currently follows the custom rule.
rg -n -C 3 '"prettier/prettier"|eslintPluginPrettier' web/mobile/eslint.config.mjsRepository: Agenta-AI/agenta
Length of output: 887
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file excerpt ---\n'
sed -n '1,150p' web/mobile/eslint.config.mjs
printf '\n--- lint/config package versions or lock entries ---\n'
if [ -f web/package.json ]; then
node -e "const p=require('./web/package.json'); console.log({eslint:p.dependencies?.eslint||p.devDependencies?.eslint, eslintPluginPrettier:p.dependencies?.['eslint-plugin-prettier']||p.devDependencies?.['eslint-plugin-prettier']})"
fi
if [ -f pnpm-lock.yaml ]; then
rg -n "eslint(@|/)|eslint-plugin-prettier" pnpm-lock.yaml | head -80
fiRepository: Agenta-AI/agenta
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- file excerpt ---'
sed -n '1,150p' web/mobile/eslint.config.mjs
printf '%s\n' ''
printf '%s\n' '--- lint/config package versions or lock entries ---'
if [ -f web/package.json ]; then
node -e "const p=require('./web/package.json'); console.log(JSON.stringify({eslint:p.dependencies?.eslint||p.devDependencies?.eslint, eslintPluginPrettier:p.dependencies?.['eslint-plugin-prettier']||p.devDependencies?.['eslint-plugin-prettier']}, null, 2))"
fi
if [ -f pnpm-lock.yaml ]; then
rg -n "eslint(@|/)|eslint-plugin-prettier" pnpm-lock.yaml | head -80
fi
printf '%s\n' ''
printf '%s\n' '--- static ordering check ---'
python3 - <<'PY'
from pathlib import Path
text = Path('web/mobile/eslint.config.mjs').read_text()
line = 0
events = []
for i, ch in enumerate(text, 1):
if ch == '\n':
line += 1
if '"prettier/prettier"' in text[max(0, text.rfind('\n',0,i-1)):i]:
events.append(('prettier/rule', i))
if 'eslintPluginPrettier' in text[i:i+100]:
events.append(('eslintPluginPrettier', i))
for event in events:
print(event)
# Find first line of each relevant token.
for token in ['eslintPluginPrettier', '"prettier/prettier"']:
idx = text.find(token)
print(token, idx, text[:idx].count('\n') + 1 if idx >= 0 else None)
PYRepository: Agenta-AI/agenta
Length of output: 10006
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- resolve recommended preset contents ---'
node - <<'JS'
try {
const preset = require('eslint-plugin-prettier/recommended')
console.log(JSON.stringify(preset, (key, value) => {
if (key === 'meta' && typeof value === 'object' && value !== null) return '[meta]'
if (key === 'configs') return '[configs]'
return value
}, 2))
} catch (err) {
console.log(err && err.message ? err.message : err)
}
JSRepository: Agenta-AI/agenta
Length of output: 298
🌐 Web query:
eslint-plugin-prettier recommended preset prettier/prettier rule options
💡 Result:
The eslint-plugin-prettier provides a recommended configuration, plugin:prettier/recommended, which simplifies setup by including both the plugin itself and eslint-config-prettier [1][2][3]. This preset performs three main actions: 1. Enables the prettier/prettier rule, which reports formatting issues as ESLint errors [1][3]. 2. Disables specific ESLint rules that are known to conflict with Prettier (e.g., arrow-body-style and prefer-arrow-callback) [1][3]. 3. Enables eslint-config-prettier to turn off other ESLint rules that may conflict with Prettier's formatting [1][4][3]. The prettier/prettier rule itself accepts two optional arguments [2][3]: First Option: An object representing Prettier configuration options (e.g., {"parser": "flow"}). These options override any settings defined in your.prettierrc file [2][3]. Second Option: An object containing specific plugin-level settings: - usePrettierrc (default: true): When set to false, it prevents the plugin from loading your Prettier configuration file [2][3]. - fileInfoOptions: Options passed to Prettier's getFileInfo function, which determines if a file should be formatted (useful, for example, to include node_modules) [2][3]. Example configuration: { "rules": { "prettier/prettier": [ "error", { "singleQuote": true }, { "usePrettierrc": false } ] } } While using a plugin to run Prettier as a linter rule was common practice in the past, the Prettier maintainers now generally recommend running Prettier directly via the CLI (e.g., prettier --check.) or via editor integration, as running it as an ESLint rule can be slower and cause unnecessary editor noise [5].
Citations:
- 1: https://github.com/prettier/eslint-plugin-prettier?tab=readme-ov-file
- 2: https://github.com/prettier/eslint-plugin-prettier
- 3: https://github.com/prettier/eslint-plugin-prettier/blob/master/README.md
- 4: https://github.com/prettier/eslint-config-prettier?tab=readme-ov-file
- 5: https://prettier.io/docs/integrating-with-linters
Do not include the shared mobile ESLint config in the review.
The mobile config must not import or mirror web/packages/eslint.config.mjs; this creates a circular/shared ESLint config instead of an isolated mobile config.
1 participant
Context
Agenta has no mobile experience. The desktop app shows a "works better on larger screens" overlay on a phone, and the agent product now has a reason to be reachable from one: a cloud agent can pause for approval while you are away from your desk.
This is the first of a 12-PR stack that adds a minimal mobile web app at
/m. It contains the scaffold only. No screens, no data, nothing user-reachable yet: the app renders a placeholder, and the container that serves it does not start unless you ask for it.What this adds
A new
web/mobileworkspace package: a Next.js Pages Router app withbasePath: "/m"and standalone output, served behind Traefik at/malongside the desktop app. It shares the desktop's SuperTokens cookie session because it is the same origin.Styling comes from shadcn/ui on Tailwind v4, and the colors are not a second palette.
scripts/generate-shadcn-tokens.tsreads the existing source of truth,web/oss/src/styles/theme/palette.ts, and emitssrc/styles/theme.generated.css(committed, checked in CI). Change a color inpalette.tsand mobile follows.Three rules are lint-enforced rather than documented, because both are easy to violate by habit:
@ant-design/*, no Lexical. The app is shadcn-only.@/oss/*,@agenta/ossand@agenta/eeare banned; data comes from the@agenta/*packages.The dev compose service is opt-in behind
--with-mobile. It originally rode the existingwith-webprofile and started automatically, but a live run showed the second Next dev server pushing an 8GB Docker VM into OOM-killing the main web app's first Turbopack compile (dmesg-confirmednext-serverkills at ~4.5GB RSS). Running both dev servers wants a 12GB+ VM, so it does not start unless you opt in.Tests / notes
pnpm --filter @agenta/mobile types:checkandlint(which includes the generated-token freshness check) pass./mserves through Traefik while the desktop app keeps working at/.ghDockerfile is added here but not wired into the release build. That happens in a later PR in the stack, so this one cannot affect production images.entrypoint.shmirrors__env.jsinto the mobile app's public dir when that directory exists.What to QA
Nothing user-visible ships here. If you want to see it run:
bash ./hosting/docker-compose/run.sh --ee --dev --with-mobile --build, then openhttp://localhost/m. You get a placeholder page.http://localhost/in the same browser. The desktop app loads and your session still works.--with-mobile. Theweb-mobilecontainer does not start and the desktop app is unaffected.