fix: forge app without main.js output#196
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds many new agent skill docs and CI-monitoring scripts, refactors e2e test utilities from project->workspace, updates agent/config files, changes the generated Webpack config to produce a single Changes
Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant PollDecide as ci-poll-decide.mjs
participant StateUpdate as ci-state-update.mjs
participant Subagent
participant MCP
participant CI as CI_System
Orchestrator->>Subagent: FETCH_STATUS (ci_information)
Subagent->>CI: request status
CI-->>Subagent: status payload
Subagent-->>Orchestrator: status summary
Orchestrator->>PollDecide: decide action (ci_information + state)
PollDecide-->>Orchestrator: action JSON (poll/wait/apply/done)
alt action == "apply"
Orchestrator->>MCP: UPDATE_FIX / APPLY
MCP-->>Orchestrator: apply result
Orchestrator->>StateUpdate: post-action (action, cipe-url/sha)
StateUpdate-->>Orchestrator: waitMode / expectedCommitSha
else action == "poll"
Orchestrator->>CI: wait/delay then repeat
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
tbinna
force-pushed
the
webpack-only-index-js-output
branch
from
1898a5b to
c1119a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/nx-forge-e2e/tests/application.generator.spec.ts (1)
28-28: Assert the bundled files here too.Line 28 only proves
src/main.tsis not scaffolded. Issue#195is about webpack output, so this test can still pass if the build emitsdist/apps/${appName}/src/main.jsor writesindex.jsto the wrong path.💡 Example follow-up
expect(() => checkFilesExist(`apps/${appName}/src/index.ts`)).not.toThrow(); expect(() => checkFilesExist(`apps/${appName}/src/main.ts`)).toThrow(); + + await runNxCommandAsync(`build ${appName}`); + expect(() => + checkFilesExist(`dist/apps/${appName}/src/index.js`) + ).not.toThrow(); + expect(() => + checkFilesExist(`dist/apps/${appName}/src/main.js`) + ).toThrow();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/tests/application.generator.spec.ts` at line 28, Current test only asserts src/main.ts is absent; add positive assertions to verify the webpack output is produced in the correct bundle location. After the existing expect(() => checkFilesExist(`apps/${appName}/src/main.ts`)).toThrow(), call expect(() => checkFilesExist(`dist/apps/${appName}/src/main.js`)).not.toThrow() (and optionally also check `dist/apps/${appName}/index.js` if your build may emit index.js) so the test ensures the build writes the bundled file into dist rather than the wrong path; use the existing checkFilesExist, expect and appName symbols to implement these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/nx-forge/src/generators/application/files/webpack.config.js__tmpl__`:
- Line 13: The outputPath currently appends an extra '/src' causing double
'.../src/src' because webpackPluginOptions.outputPath already contains
'dist/<app>/src'; update the webpack config line that sets outputPath (the one
referencing webpackPluginOptions.outputPath) to remove the hard-coded '/src'
suffix so it uses just '<%= offset %><%= webpackPluginOptions.outputPath %>';
this will restore the expected layout used by the tunnel flow (see run-tunnel
logic) and prevent the bundle being written to the wrong directory.
---
Nitpick comments:
In `@e2e/nx-forge-e2e/tests/application.generator.spec.ts`:
- Line 28: Current test only asserts src/main.ts is absent; add positive
assertions to verify the webpack output is produced in the correct bundle
location. After the existing expect(() =>
checkFilesExist(`apps/${appName}/src/main.ts`)).toThrow(), call expect(() =>
checkFilesExist(`dist/apps/${appName}/src/main.js`)).not.toThrow() (and
optionally also check `dist/apps/${appName}/index.js` if your build may emit
index.js) so the test ensures the build writes the bundled file into dist rather
than the wrong path; use the existing checkFilesExist, expect and appName
symbols to implement these assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e53b188-14f6-419f-8ef7-8804f09c73ec
📒 Files selected for processing (3)
e2e/nx-forge-e2e/tests/application.generator.spec.tspackages/nx-forge/src/generators/application/files/webpack.config.js__tmpl__packages/nx-forge/src/generators/application/generator.spec.ts
tbinna
force-pushed
the
webpack-only-index-js-output
branch
2 times, most recently
from
c233792 to
94d1586
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs (2)
132-135: Wait timeout assumes fixed 30s intervals.The
isWaitTimedOut()function assumes 30-second wait intervals (pollCount * 30), which matches thedelay: 30inbuildOutput()for wait mode. This is correct but implicit—consider adding a constant or comment linking these values.const WAIT_INTERVAL_SECONDS = 30; function isWaitTimedOut() { if (newCipeTimeoutSeconds <= 0) return false; return pollCount * WAIT_INTERVAL_SECONDS >= newCipeTimeoutSeconds; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs around lines 132 - 135, The timeout calculation in isWaitTimedOut() implicitly assumes the 30s poll interval used in buildOutput() (delay: 30); make this explicit by introducing a named constant (e.g., WAIT_INTERVAL_SECONDS) and use it in isWaitTimedOut() instead of the literal 30, and add a brief comment referencing buildOutput's delay to keep both values aligned (update references to isWaitTimedOut and buildOutput to use the new constant).
126-130: The timeout calculation may not accurately reflect elapsed time.The
isTimedOut()function estimates elapsed time aspollCount * avgDelay, but:
avgDelayis computed asbackoff(Math.floor(pollCount / 2)), which doesn't account for actual delays used in previous iterations- The backoff delays are [60, 90, 120] seconds, but actual elapsed time depends on which delay was used at each poll
- This doesn't account for time spent processing (subagent calls, script execution)
Consider tracking actual
startTimeand computing elapsed time directly, or document this as an intentional approximation.-function isTimedOut() { - if (timeoutSeconds <= 0) return false; - const avgDelay = pollCount === 0 ? 0 : backoff(Math.floor(pollCount / 2)); - return pollCount * avgDelay >= timeoutSeconds; -} +// Note: This is an approximation. The orchestrator tracks actual startTime +// and should pass remaining timeout or elapsed time if precise timing is needed. +function isTimedOut() { + if (timeoutSeconds <= 0) return false; + // Conservative estimate using minimum delay + const estimatedMinElapsed = pollCount * 60; + return estimatedMinElapsed >= timeoutSeconds; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs around lines 126 - 130, The isTimedOut() calculation is an approximation and can be inaccurate because it multiplies pollCount by a guessed avgDelay from backoff(Math.floor(pollCount/2)) and ignores actual processing time; modify the logic to record a startTime when polling begins and change isTimedOut() to compute elapsed = (now - startTime) in seconds and compare that to timeoutSeconds (using the existing timeoutSeconds, pollCount and backoff for logging if desired); update any callers of isTimedOut() or its scope so startTime is initialized (e.g., on poll loop start) and used inside isTimedOut() to return true when elapsed >= timeoutSeconds..agents/skills/nx-import/references/JEST.md (1)
62-64: Consider adding language specifiers to fenced code blocks.The shell/pnpm commands in these code blocks lack language specifiers. While functional, adding
bashorshellimproves syntax highlighting and consistency:-``` -pnpm add -wD jest ts-jest `@types/jest` `@nx/jest` -``` +```bash +pnpm add -wD jest ts-jest `@types/jest` `@nx/jest` +```This applies to lines 62-64, 73-75, 81-83, and 89-91.
Also applies to: 73-75, 81-83, 89-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nx-import/references/JEST.md around lines 62 - 64, Update the fenced code blocks in .agents/skills/nx-import/references/JEST.md that contain the pnpm/shell commands so they include a language specifier (use ```bash). Specifically change the three-backtick openers for the blocks containing the command "pnpm add -wD jest ts-jest `@types/jest` `@nx/jest`" and the other pnpm commands in the same file to "```bash" (applies to the four blocks mentioned in the review) to enable shell syntax highlighting and keep formatting consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/monitor-ci/scripts/ci-state-update.mjs:
- Around line 41-43: parseInt results used for count and max (from
getArg('--local-verify-count' / '--local-verify-attempts')) are not validated
and can become NaN; change the parsing to validate and fallback to a safe
integer (e.g., 0 for count, 3 for max) when Number.isFinite or Number.isNaN
indicates an invalid value, or create a small helper like
parseIntOrDefault(value, default) and use it for the occurrences that set count,
max and the other parseInt usages in the file (the other getArg parses around
the same logic). Ensure the code updates the variables count and max only with
validated integers before performing the comparison (count >= max).
- Around line 96-112: The handler currently allows actions that require tracking
IDs to proceed with null values; update the logic after computing trackByCipeUrl
and trackByCommitSha to validate presence of the required identifiers and return
an error via output(...) when missing: if trackByCipeUrl is true but cipeUrl is
falsy, call output({ error: 'Action <action> requires --cipe-url' }); similarly
if trackByCommitSha is true but commitSha is falsy, call output({ error: 'Action
<action> requires --commit-sha' }); keep using the existing symbols (action,
cipeUrl, commitSha, trackByCipeUrl, trackByCommitSha, output) so the rest of the
state-setting block only runs when required IDs are present.
In @.agents/skills/nx-import/references/VITE.md:
- Around line 1-397: The VITE.md content is unrelated to the PR objective about
the Forge/Webpack output (index.js vs main.js); either remove or move this
Vite-specific documentation out of this PR and update the PR title/description
to reflect only the Forge/Webpack fix, or add a brief note in VITE.md explaining
why it’s included in this PR (e.g., infrastructure changes affecting multiple
skills). Locate the "Vite" header or the "`@nx/vite/plugin` Typecheck Target"
section in VITE.md to remove/move or to append the explanatory note, and ensure
the PR description references the moved file or the reason for its inclusion so
reviewers see the correct scope.
In @.agents/skills/nx-run-tasks/SKILL.md:
- Around line 18-20: Add language tags to the fenced code blocks to satisfy
markdownlint MD040: update the three code fences that contain the commands `nx
run <project>:<task>`, `nx run-many -t build test lint typecheck`, and `nx
affected -t build test lint` (and the other occurrences you noted at lines 26-28
and 42-44) to use a language specifier such as ```bash so they read ```bash ...
```; ensure all similar fenced blocks in this document are updated consistently.
In `@e2e/nx-forge-e2e/src/application.generator.spec.ts`:
- Around line 95-98: The assertions are checking the wrong output path (they
expect files under outputDir/src) causing CI to fail; update the two existsSync
checks in this test to target the built artifacts at the app root instead of the
src subfolder by using join(outputDir, 'index.js') for the positive assertion
and join(outputDir, 'main.js') for the negative assertion (referencing outputDir
and appName in the existing test).
In `@e2e/nx-forge-e2e/src/utils/test-workspace.ts`:
- Around line 33-35: The test setup command currently uses
`create-nx-workspace@latest`, causing non-deterministic workspace generation;
update the command string (the one constructed with workspaceName and
TEST_WORKSPACES_ROOT) to pin the package to the repository's Nx version (e.g.,
`create-nx-workspace@22.6.1`) so test workspaces match package.json's Nx
version; locate the command construction that includes workspaceName and
TEST_WORKSPACES_ROOT and replace the `@latest` tag with the pinned version (or
read the version from package.json and interpolate it) to ensure deterministic
e2e setup.
---
Nitpick comments:
In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs:
- Around line 132-135: The timeout calculation in isWaitTimedOut() implicitly
assumes the 30s poll interval used in buildOutput() (delay: 30); make this
explicit by introducing a named constant (e.g., WAIT_INTERVAL_SECONDS) and use
it in isWaitTimedOut() instead of the literal 30, and add a brief comment
referencing buildOutput's delay to keep both values aligned (update references
to isWaitTimedOut and buildOutput to use the new constant).
- Around line 126-130: The isTimedOut() calculation is an approximation and can
be inaccurate because it multiplies pollCount by a guessed avgDelay from
backoff(Math.floor(pollCount/2)) and ignores actual processing time; modify the
logic to record a startTime when polling begins and change isTimedOut() to
compute elapsed = (now - startTime) in seconds and compare that to
timeoutSeconds (using the existing timeoutSeconds, pollCount and backoff for
logging if desired); update any callers of isTimedOut() or its scope so
startTime is initialized (e.g., on poll loop start) and used inside isTimedOut()
to return true when elapsed >= timeoutSeconds.
In @.agents/skills/nx-import/references/JEST.md:
- Around line 62-64: Update the fenced code blocks in
.agents/skills/nx-import/references/JEST.md that contain the pnpm/shell commands
so they include a language specifier (use ```bash). Specifically change the
three-backtick openers for the blocks containing the command "pnpm add -wD jest
ts-jest `@types/jest` `@nx/jest`" and the other pnpm commands in the same file to
"```bash" (applies to the four blocks mentioned in the review) to enable shell
syntax highlighting and keep formatting consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c480ab2b-3b78-4218-a920-fb3407a68843
📒 Files selected for processing (31)
.agents/skills/link-workspace-packages/SKILL.md.agents/skills/monitor-ci/SKILL.md.agents/skills/monitor-ci/references/fix-flows.md.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs.agents/skills/monitor-ci/scripts/ci-state-update.mjs.agents/skills/nx-generate/SKILL.md.agents/skills/nx-import/SKILL.md.agents/skills/nx-import/references/ESLINT.md.agents/skills/nx-import/references/GRADLE.md.agents/skills/nx-import/references/JEST.md.agents/skills/nx-import/references/NEXT.md.agents/skills/nx-import/references/TURBOREPO.md.agents/skills/nx-import/references/VITE.md.agents/skills/nx-plugins/SKILL.md.agents/skills/nx-run-tasks/SKILL.md.agents/skills/nx-workspace/SKILL.md.agents/skills/nx-workspace/references/AFFECTED.md.claude/settings.json.codex/agents/ci-monitor-subagent.toml.codex/config.toml.gitignoreAGENTS.mde2e/nx-forge-e2e/src/application.generator.spec.tse2e/nx-forge-e2e/src/basic-setup.spec.tse2e/nx-forge-e2e/src/utils/async-commands.tse2e/nx-forge-e2e/src/utils/atlassian-graphql-client.tse2e/nx-forge-e2e/src/utils/cleanup-registered-forge-app.tse2e/nx-forge-e2e/src/utils/test-project.tse2e/nx-forge-e2e/src/utils/test-workspace.tspackages/nx-forge/src/generators/application/files/webpack.config.js__tmpl__tools/scripts/stop-local-registry.ts
💤 Files with no reviewable changes (1)
- e2e/nx-forge-e2e/src/utils/test-project.ts
✅ Files skipped from review due to trivial changes (11)
- .agents/skills/nx-plugins/SKILL.md
- .gitignore
- .claude/settings.json
- .agents/skills/nx-import/references/GRADLE.md
- .agents/skills/link-workspace-packages/SKILL.md
- .agents/skills/nx-workspace/references/AFFECTED.md
- .agents/skills/nx-workspace/SKILL.md
- .agents/skills/nx-import/references/ESLINT.md
- .codex/agents/ci-monitor-subagent.toml
- .agents/skills/nx-import/references/NEXT.md
- .agents/skills/nx-import/references/TURBOREPO.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nx-forge/src/generators/application/files/webpack.config.js__tmpl__
Comment on lines
+41
to
+43
| const count = parseInt(getArg('--local-verify-count') || '0', 10); | ||
| const max = parseInt(getArg('--local-verify-attempts') || '3', 10); | ||
| if (count >= max) { |
There was a problem hiding this comment.
Validate numeric flags before arithmetic.
parseInt results are used without NaN checks. Invalid flag values can emit NaN counters and poison downstream state.
🛡️ Proposed fix
+function parseCounter(name, fallback) {
+ const raw = getArg(name);
+ const value = parseInt(raw ?? String(fallback), 10);
+ if (!Number.isFinite(value) || value < 0) {
+ output({ error: `Invalid numeric value for ${name}: ${raw}` });
+ return null;
+ }
+ return value;
+}
+
function gate() {
const gateType = getArg('--gate-type');
if (gateType === 'local-fix') {
- const count = parseInt(getArg('--local-verify-count') || '0', 10);
- const max = parseInt(getArg('--local-verify-attempts') || '3', 10);
+ const count = parseCounter('--local-verify-count', 0);
+ const max = parseCounter('--local-verify-attempts', 3);
+ if (count === null || max === null) return;
...
if (gateType === 'env-rerun') {
- const count = parseInt(getArg('--env-rerun-count') || '0', 10);
+ const count = parseCounter('--env-rerun-count', 0);
+ if (count === null) return;
...
function cycleCheck() {
...
- let cycleCount = parseInt(getArg('--cycle-count') || '0', 10);
- const maxCycles = parseInt(getArg('--max-cycles') || '10', 10);
- let envRerunCount = parseInt(getArg('--env-rerun-count') || '0', 10);
+ let cycleCount = parseCounter('--cycle-count', 0);
+ const maxCycles = parseCounter('--max-cycles', 10);
+ let envRerunCount = parseCounter('--env-rerun-count', 0);
+ if (cycleCount === null || maxCycles === null || envRerunCount === null) return;Also applies to: 58-59, 122-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/monitor-ci/scripts/ci-state-update.mjs around lines 41 - 43,
parseInt results used for count and max (from getArg('--local-verify-count' /
'--local-verify-attempts')) are not validated and can become NaN; change the
parsing to validate and fallback to a safe integer (e.g., 0 for count, 3 for
max) when Number.isFinite or Number.isNaN indicates an invalid value, or create
a small helper like parseIntOrDefault(value, default) and use it for the
occurrences that set count, max and the other parseInt usages in the file (the
other getArg parses around the same logic). Ensure the code updates the
variables count and max only with validated integers before performing the
comparison (count >= max).
Comment on lines
+96
to
+112
| const trackByCipeUrl = cipeUrlActions.includes(action); | ||
| const trackByCommitSha = commitShaActions.includes(action); | ||
|
|
||
| if (!trackByCipeUrl && !trackByCommitSha) { | ||
| return output({ error: `Unknown action: ${action}` }); | ||
| } | ||
|
|
||
| // fix-auto-applying: self-healing did it, NOT the monitor | ||
| const agentTriggered = action !== 'fix-auto-applying'; | ||
|
|
||
| output({ | ||
| waitMode: true, | ||
| pollCount: 0, | ||
| lastCipeUrl: trackByCipeUrl ? cipeUrl : null, | ||
| expectedCommitSha: trackByCommitSha ? commitSha : null, | ||
| agentTriggered, | ||
| }); |
There was a problem hiding this comment.
Enforce required tracking IDs per action type.
post-action accepts actions that require --cipe-url or --commit-sha but does not validate presence; this can emit null tracking state for an otherwise valid action.
🔧 Proposed fix
const trackByCipeUrl = cipeUrlActions.includes(action);
const trackByCommitSha = commitShaActions.includes(action);
if (!trackByCipeUrl && !trackByCommitSha) {
return output({ error: `Unknown action: ${action}` });
}
+ if (trackByCipeUrl && !cipeUrl) {
+ return output({ error: `--cipe-url is required for action: ${action}` });
+ }
+ if (trackByCommitSha && !commitSha) {
+ return output({ error: `--commit-sha is required for action: ${action}` });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/monitor-ci/scripts/ci-state-update.mjs around lines 96 - 112,
The handler currently allows actions that require tracking IDs to proceed with
null values; update the logic after computing trackByCipeUrl and
trackByCommitSha to validate presence of the required identifiers and return an
error via output(...) when missing: if trackByCipeUrl is true but cipeUrl is
falsy, call output({ error: 'Action <action> requires --cipe-url' }); similarly
if trackByCommitSha is true but commitSha is falsy, call output({ error: 'Action
<action> requires --commit-sha' }); keep using the existing symbols (action,
cipeUrl, commitSha, trackByCipeUrl, trackByCommitSha, output) so the rest of the
state-setting block only runs when required IDs are present.
Comment on lines
+1
to
+397
| ## Vite | ||
|
|
||
| Vite-specific guidance for `nx import`. For generic import issues (pnpm globs, root deps, project references, name collisions, ESLint, frontend tsconfig base settings, `@nx/react` typings, Jest preset, non-Nx source handling), see `SKILL.md`. | ||
|
|
||
| --- | ||
|
|
||
| ### `@nx/vite/plugin` Typecheck Target | ||
|
|
||
| `@nx/vite/plugin` defaults `typecheckTargetName` to `"vite:typecheck"`. If the workspace expects `"typecheck"`, set it explicitly in `nx.json`. If `@nx/js/typescript` is also registered, rename one target to avoid conflicts (e.g. `"tsc-typecheck"` for the JS plugin). | ||
|
|
||
| Keep both plugins only if the workspace has non-Vite pure TS libraries — `@nx/js/typescript` handles those while `@nx/vite/plugin` handles Vite projects. | ||
|
|
||
| ### @nx/vite Plugin Install Failure | ||
|
|
||
| Plugin init loads `vite.config.ts` before deps are available. **Fix**: `pnpm add -wD vite @vitejs/plugin-react` (or `@vitejs/plugin-vue`) first, then `pnpm exec nx add @nx/vite`. | ||
|
|
||
| ### Vite `resolve.alias` and `__dirname` (Non-Nx Sources) | ||
|
|
||
| **`__dirname` undefined** (CJS-only): Replace with `fileURLToPath(new URL('./src', import.meta.url))` from `'node:url'`. | ||
|
|
||
| **`@/` path alias**: Vite's `resolve.alias` works at runtime but TS needs matching `"paths"`. Set `"baseUrl": "."` in project tsconfig. | ||
|
|
||
| **PostCSS/Tailwind**: Verify `content` globs resolve correctly after import. | ||
|
|
||
| ### Missing TypeScript `types` (Non-Nx Sources) | ||
|
|
||
| Non-Nx tsconfigs may not declare all needed types. Ensure Vite projects include `"types": ["node", "vite/client"]` in their tsconfig. | ||
|
|
||
| ### `noEmit` Fix: Vite-Specific Notes | ||
|
|
||
| See SKILL.md for the generic noEmit→composite fix. Vite-specific additions: | ||
|
|
||
| - Non-Nx Vite projects often have **both** `tsconfig.app.json` and `tsconfig.node.json` with `noEmit` — fix both | ||
| - Solution-style tsconfigs (`"files": [], "references": [...]`) may lack `extends`. Add `extends` pointing to the dest root `tsconfig.base.json` so base settings (`moduleResolution`, `lib`) apply. | ||
| - This is safe — Vite/Vitest ignore TypeScript emit settings. | ||
|
|
||
| ### Dependency Version Conflicts | ||
|
|
||
| **Shared Vite deps (both frameworks):** `vite`, `vitest`, `jsdom`, `@types/node`, `typescript` (dev) | ||
|
|
||
| **Vite 6→7**: Typecheck fails (`Plugin<any>` type mismatch); build/serve still works. Fix: align versions. | ||
| **Vitest 3→4**: Usually works; type conflicts may surface in shared test utils. | ||
|
|
||
| --- | ||
|
|
||
| ## React Router 7 (Vite-Based) | ||
|
|
||
| React Router 7 (`@react-router/dev`) uses Vite under the hood with a `vite.config.ts` and a `react-router.config.ts`. The `@nx/vite/plugin` detects `vite.config.ts` and creates inferred targets. | ||
|
|
||
| ### Targets | ||
|
|
||
| `@nx/vite/plugin` creates `build`, `dev`, `serve` targets. The `build` target invokes the script defined in `package.json` (usually `react-router build`), not `vite build` directly. | ||
|
|
||
| **No separate typecheck target from `@nx/vite/plugin`** — React Router 7 typegen is run as part of `typecheck` (e.g. `react-router typegen && tsc`). The `typecheck` target is inferred from the tsconfig. Keep the `typecheck` script in `package.json` if present; it is not rewritten. | ||
|
|
||
| ### tsconfig Notes | ||
|
|
||
| React Router 7 uses a single `tsconfig.json` (no `tsconfig.app.json`/`tsconfig.node.json` split). It includes: | ||
|
|
||
| - `"rootDirs": [".", "./.react-router/types"]` — for generated type files; keep as-is | ||
| - `"paths": { "~/*": ["./app/*"] }` — self-referential alias; keep as-is | ||
| - `"noEmit": true` — replace with composite settings per SKILL.md | ||
|
|
||
| ### Build Output | ||
|
|
||
| React Router 7 outputs to `build/` (not `dist/`). Add `build` to the dest root `.gitignore`. | ||
|
|
||
| ### Generated Types Directory | ||
|
|
||
| React Router 7 generates `.react-router/` at the project root for route type generation. Add `.react-router` to the dest root `.gitignore`. | ||
|
|
||
| --- | ||
|
|
||
| ## TanStack Start (Vite-Based) | ||
|
|
||
| TanStack Start uses Vinxi under the hood, which wraps Vite. Projects have a standard `vite.config.ts` that `@nx/vite/plugin` detects normally. | ||
|
|
||
| ### Targets | ||
|
|
||
| `@nx/vite/plugin` creates `build`, `dev`, `preview`, `serve-static`, `typecheck` targets. The `build` target runs `vite build` which invokes the TanStack Start Vinxi pipeline (produces both client and SSR bundles). | ||
|
|
||
| ### tsconfig Notes | ||
|
|
||
| TanStack Start uses a single `tsconfig.json` with `"allowImportingTsExtensions": true` and `"noEmit": true`. Apply the standard noEmit → composite fix. `allowImportingTsExtensions` is compatible with `emitDeclarationOnly: true` — no change needed. | ||
|
|
||
| ### `paths` Aliases | ||
|
|
||
| TanStack Start commonly uses `"#/*": ["./src/*"]` and `"@/*": ["./src/*"]`. These are self-referential — keep as-is for a single-project app. | ||
|
|
||
| ### Uncommitted Source Repo | ||
|
|
||
| `create-tan-stack` initializes a git repo but does NOT make an initial commit. Before importing, commit first: | ||
|
|
||
| ```bash | ||
| git -C /path/to/source add . && git -C /path/to/source commit -m "Initial commit" | ||
| ``` | ||
|
|
||
| ### Generated and Build Directories | ||
|
|
||
| TanStack Start / Vinxi / Nitro generate several directories that must be added to the dest root `.gitignore`: | ||
|
|
||
| - `.vinxi` — Vinxi build cache | ||
| - `.tanstack` — TanStack generated files | ||
| - `.nitro` — Nitro build artifacts | ||
| - `.output` — server-side build output (SSR/edge) | ||
|
|
||
| These are not covered by `dist` or `build`. | ||
|
|
||
| --- | ||
|
|
||
| ## React-Specific | ||
|
|
||
| ### React Dependencies | ||
|
|
||
| **Production:** `react`, `react-dom` | ||
| **Dev:** `@types/react`, `@types/react-dom`, `@vitejs/plugin-react`, `@testing-library/react`, `@testing-library/jest-dom`, `jsdom` | ||
| **ESLint (Nx sources):** `eslint-plugin-import`, `eslint-plugin-jsx-a11y`, `eslint-plugin-react`, `eslint-plugin-react-hooks` | ||
| **ESLint (`create-vite`):** `eslint-plugin-react-refresh`, `eslint-plugin-react-hooks` — self-contained flat configs can be left as-is | ||
| **Nx plugins:** `@nx/react` (generators), `@nx/vite`, `@nx/vitest`, `@nx/eslint` | ||
|
|
||
| ### React TypeScript Configuration | ||
|
|
||
| Add `"jsx": "react-jsx"` — in `tsconfig.base.json` for single-framework workspaces, per-project for mixed (see Mixed section). | ||
|
|
||
| ### React ESLint Config | ||
|
|
||
| ```js | ||
| import nx from '@nx/eslint-plugin'; | ||
| import baseConfig from '../../eslint.config.mjs'; | ||
| export default [ | ||
| ...baseConfig, | ||
| ...nx.configs['flat/react'], | ||
| { files: ['**/*.ts', '**/*.tsx'], rules: {} }, | ||
| ]; | ||
| ``` | ||
|
|
||
| ### React Version Conflicts | ||
|
|
||
| React 18 (source) + React 19 (dest): pnpm may hoist mismatched `react-dom`, causing `TypeError: Cannot read properties of undefined (reading 'S')`. **Fix**: Align versions with `pnpm.overrides`. | ||
|
|
||
| ### `@testing-library/jest-dom` with Vitest | ||
|
|
||
| If source used Jest: change import to `@testing-library/jest-dom/vitest` in test-setup.ts, add to tsconfig `types`. | ||
|
|
||
| --- | ||
|
|
||
| ## Vue-Specific | ||
|
|
||
| ### Vue Dependencies | ||
|
|
||
| **Production:** `vue` (plus `vue-router`, `pinia` if used) | ||
| **Dev:** `@vitejs/plugin-vue`, `vue-tsc`, `@vue/test-utils`, `jsdom` | ||
| **ESLint:** `eslint-plugin-vue`, `vue-eslint-parser`, `@vue/eslint-config-typescript`, `@vue/eslint-config-prettier` | ||
| **Nx plugins:** `@nx/vue` (generators), `@nx/vite`, `@nx/vitest`, `@nx/eslint` (install AFTER deps — see below) | ||
|
|
||
| ### Vue TypeScript Configuration | ||
|
|
||
| Add to `tsconfig.base.json` (single-framework) or per-project (mixed): | ||
|
|
||
| ```json | ||
| { "jsx": "preserve", "jsxImportSource": "vue", "resolveJsonModule": true } | ||
| ``` | ||
|
|
||
| ### `vue-shims.d.ts` | ||
|
|
||
| Vue SFC files need a type declaration. Usually exists in each project's `src/` and imports cleanly. If missing: | ||
|
|
||
| ```ts | ||
| declare module '*.vue' { | ||
| import { defineComponent } from 'vue'; | ||
| const component: ReturnType<typeof defineComponent>; | ||
| export default component; | ||
| } | ||
| ``` | ||
|
|
||
| ### `vue-tsc` Auto-Detection | ||
|
|
||
| Both `@nx/js/typescript` and `@nx/vite/plugin` auto-detect `vue-tsc` when installed — no manual config needed. Remove source scripts like `"typecheck": "vue-tsc --noEmit"`. | ||
|
|
||
| ### ESLint Plugin Installation Order (Critical) | ||
|
|
||
| `@nx/eslint` init **crashes** if Vue ESLint deps aren't installed first (it loads all config files). | ||
|
|
||
| **Correct order:** | ||
|
|
||
| 1. `pnpm add -wD eslint@^9 eslint-plugin-vue vue-eslint-parser @vue/eslint-config-typescript @typescript-eslint/parser @nx/eslint-plugin typescript-eslint` | ||
| 2. Create root `eslint.config.mjs` | ||
| 3. Then `npx nx add @nx/eslint` | ||
|
|
||
| ### Vue ESLint Config Pattern | ||
|
|
||
| ```js | ||
| import vue from 'eslint-plugin-vue'; | ||
| import vueParser from 'vue-eslint-parser'; | ||
| import tsParser from '@typescript-eslint/parser'; | ||
| import baseConfig from '../../eslint.config.mjs'; | ||
| export default [ | ||
| ...baseConfig, | ||
| ...vue.configs['flat/recommended'], | ||
| { | ||
| files: ['**/*.vue'], | ||
| languageOptions: { parser: vueParser, parserOptions: { parser: tsParser } }, | ||
| }, | ||
| { | ||
| files: ['**/*.ts', '**/*.tsx', '**/*.js', '**/*.jsx', '**/*.vue'], | ||
| rules: { 'vue/multi-word-component-names': 'off' }, | ||
| }, | ||
| ]; | ||
| ``` | ||
|
|
||
| **Important**: `vue-eslint-parser` override must come **AFTER** base config — `flat/typescript` sets the TS parser globally without a `files` filter, breaking `.vue` parsing. | ||
|
|
||
| `vue-eslint-parser` must be an explicit pnpm dependency (strict resolution prevents transitive import). | ||
|
|
||
| **Known issue**: Some generated Vue ESLint configs omit `vue-eslint-parser`. Use the pattern above instead. | ||
|
|
||
| --- | ||
|
|
||
| ## Mixed React + Vue | ||
|
|
||
| When both frameworks coexist, several settings become per-project. | ||
|
|
||
| ### tsconfig `jsx` — Per-Project Only | ||
|
|
||
| - React: `"jsx": "react-jsx"` in project tsconfig | ||
| - Vue: `"jsx": "preserve"`, `"jsxImportSource": "vue"` in project tsconfig | ||
| - Root: **NO** `jsx` setting | ||
|
|
||
| ### Typecheck — Auto-Detects Framework | ||
|
|
||
| `@nx/vite/plugin` uses `vue-tsc` for Vue projects and `tsc` for React automatically. | ||
|
|
||
| ```json | ||
| { | ||
| "plugins": [ | ||
| { "plugin": "@nx/eslint/plugin", "options": { "targetName": "lint" } }, | ||
| { | ||
| "plugin": "@nx/vite/plugin", | ||
| "options": { | ||
| "buildTargetName": "build", | ||
| "typecheckTargetName": "typecheck", | ||
| "testTargetName": "test" | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ``` | ||
|
|
||
| Remove `@nx/js/typescript` if all projects use Vite. Keep it (renamed to `"tsc-typecheck"`) only for non-Vite pure TS libs. | ||
|
|
||
| ### ESLint — Three-Tier Config | ||
|
|
||
| 1. **Root**: Base rules only, no framework-specific rules | ||
| 2. **React projects**: Extend root + `nx.configs['flat/react']` | ||
| 3. **Vue projects**: Extend root + `vue.configs['flat/recommended']` + `vue-eslint-parser` | ||
|
|
||
| **Required packages**: Shared (`eslint@^9`, `@nx/eslint-plugin`, `typescript-eslint`, `@typescript-eslint/parser`), React (`eslint-plugin-import`, `eslint-plugin-jsx-a11y`, `eslint-plugin-react`, `eslint-plugin-react-hooks`), Vue (`eslint-plugin-vue`, `vue-eslint-parser`) | ||
|
|
||
| `@nx/react`/`@nx/vue` are for generators only — no target conflicts. | ||
|
|
||
| --- | ||
|
|
||
| ## Redundant npm Scripts After Import | ||
|
|
||
| `nx import` copies `package.json` verbatim, so npm scripts come along. For Vite-based projects `@nx/vite/plugin` already infers the same targets from `vite.config.ts` — the npm scripts just shadow the plugin with weaker `nx:run-script` wrappers (no first-class caching inputs/outputs). Remove them after import. | ||
|
|
||
| ### Standalone Vite App (`create-vite`) | ||
|
|
||
| Remove the following scripts — every one is redundant: | ||
|
|
||
| | Script | Plugin replacement | | ||
| | ----------------------------- | ---------------------------------------------------------------------------- | | ||
| | `dev: vite` | `@nx/vite/plugin` → `dev` | | ||
| | `build: tsc -b && vite build` | `@nx/vite/plugin` → `build`; `typecheck` via `@nx/js/typescript` handles tsc | | ||
| | `preview: vite preview` | `@nx/vite/plugin` → `preview` | | ||
| | `lint: eslint .` | `@nx/eslint/plugin` → `eslint:lint` | | ||
|
|
||
| ### TanStack Start | ||
|
|
||
| Remove `build`, `dev`, `preview`, and `test` scripts, but move any hardcoded `--port` flag to `vite.config.ts` first: | ||
|
|
||
| ```ts | ||
| // vite.config.ts | ||
| export default defineConfig({ | ||
| server: { port: 3000 }, // replaces `vite dev --port 3000` | ||
| ... | ||
| }) | ||
| ``` | ||
|
|
||
| ### React Router 7 — Keep ALL scripts | ||
|
|
||
| Do **not** remove React Router 7 scripts. They use the framework CLI (`react-router build`, `react-router dev`, `react-router-serve`) which is not interchangeable with plain `vite`: | ||
|
|
||
| - `typecheck` runs `react-router typegen && tsc` — typegen must precede `tsc` or it fails on missing route types | ||
| - `start` serves the SSR bundle — no plugin equivalent | ||
|
|
||
| --- | ||
|
|
||
| ## Fix Orders | ||
|
|
||
| ### Nx Source | ||
|
|
||
| 1. Generic fixes from SKILL.md (pnpm globs, root deps, executor paths, frontend tsconfig base settings, `@nx/react` typings) | ||
| 2. Configure `@nx/vite/plugin` typecheck target | ||
| 3. **React**: `jsx: "react-jsx"` (root or per-project) | ||
| 4. **Vue**: `jsx: "preserve"` + `jsxImportSource: "vue"`; verify `vue-shims.d.ts`; install ESLint deps before `@nx/eslint` | ||
| 5. **Mixed**: `jsx` per-project; remove/rename `@nx/js/typescript` | ||
| 6. `nx sync --yes && nx reset && nx run-many -t typecheck,build,test,lint` | ||
|
|
||
| ### Non-Nx Source (additional steps) | ||
|
|
||
| 0. Import into `apps/<name>` (see SKILL.md: "Application vs Library Detection") | ||
| 1. Generic fixes from SKILL.md (stale files cleanup, pnpm globs, rewritten scripts, target name prefixing, noEmit→composite, ESLint handling) | ||
| 2. Fix `noEmit` in **all** tsconfigs (app, node, etc. — non-Nx projects often have multiple) | ||
| 3. Add `extends` to solution-style tsconfigs so root settings apply | ||
| 4. Fix `resolve.alias` / `__dirname` / `baseUrl` | ||
| 5. Ensure `types` include `vite/client` and `node` | ||
| 6. Install `@nx/vite` manually if it failed during import | ||
| 7. Remove redundant npm scripts so `@nx/vite/plugin` infers them natively (see "Redundant npm Scripts" section) | ||
| 8. **Vue**: Add `outDir` + `**/*.vue.d.ts` to ESLint ignores | ||
| 9. Full verification | ||
|
|
||
| ### Multiple-Source Imports | ||
|
|
||
| See SKILL.md for generic multi-import (name collisions, dep refs). Vite-specific: fix tsconfig `references` paths for alternate directories (`../../libs/` → `../../libs-beta/`). | ||
|
|
||
| ### Non-Nx Source: React Router 7 | ||
|
|
||
| 1. Ensure source has at least one commit (see SKILL.md: "Source Repo Has No Commits") | ||
| 2. `nx import` whole-repo into `apps/<name>` (see SKILL.md: "Application vs Library Detection") → auto-installs `@nx/vite`, `@nx/react` | ||
| 3. Stale file cleanup: `node_modules/`, `package-lock.json`, `.gitignore` | ||
| 4. Fix `tsconfig.json`: `noEmit` → `composite + emitDeclarationOnly + outDir + tsBuildInfoFile` | ||
| 5. Add `build` and `.react-router` to dest root `.gitignore` | ||
| 6. **Keep all npm scripts** — React Router 7 uses framework CLI (`react-router build/dev`), not plain vite (see "Redundant npm Scripts" above) | ||
| 7. `npm install && nx reset && nx sync --yes` | ||
|
|
||
| ### Non-Nx Source: TanStack Start | ||
|
|
||
| 1. Ensure source has at least one commit — `create-tan-stack` does NOT auto-commit (see SKILL.md) | ||
| 2. `nx import` whole-repo into `apps/<name>` (see SKILL.md: "Application vs Library Detection") → auto-installs `@nx/vite`, `@nx/vitest` | ||
| 3. Stale file cleanup: `node_modules/`, `package-lock.json`, `.gitignore` | ||
| 4. Fix `tsconfig.json`: `noEmit` → `composite + emitDeclarationOnly + outDir + tsBuildInfoFile` | ||
| 5. Keep `allowImportingTsExtensions` — compatible with `emitDeclarationOnly: true` | ||
| 6. Add `.vinxi`, `.tanstack`, `.nitro`, `.output` to dest root `.gitignore` | ||
| 7. Move hardcoded `--port` from `dev` script into `vite.config.ts` (`server: { port: N }`) | ||
| 8. Remove redundant npm scripts — `@nx/vite/plugin` infers `build`, `dev`, `preview`, `test` (see "Redundant npm Scripts" above) | ||
| 9. `npm install && nx reset && nx sync --yes` | ||
|
|
||
| ### Quick Reference: React vs Vue | ||
|
|
||
| | Aspect | React | Vue | | ||
| | ------------- | ------------------------ | ----------------------------------------- | | ||
| | Vite plugin | `@vitejs/plugin-react` | `@vitejs/plugin-vue` | | ||
| | Type checker | `tsc` | `vue-tsc` (auto-detected) | | ||
| | SFC support | N/A | `vue-shims.d.ts` needed | | ||
| | tsconfig jsx | `"react-jsx"` | `"preserve"` + `"jsxImportSource": "vue"` | | ||
| | ESLint parser | Standard TS | `vue-eslint-parser` + TS sub-parser | | ||
| | ESLint setup | Straightforward | Must install deps before `@nx/eslint` | | ||
| | Test utils | `@testing-library/react` | `@vue/test-utils` | | ||
|
|
||
| ### Quick Reference: Vite-Based React Frameworks | ||
|
|
||
| | Aspect | Vite (standalone) | React Router 7 | TanStack Start | | ||
| | ------------------ | ----------------- | ----------------------- | ------------------------ | | ||
| | Build config | `vite.config.ts` | `vite.config.ts` | `vite.config.ts` | | ||
| | Build output | `dist/` | `build/` | `dist/` | | ||
| | SSR bundle | No | Yes (`build/server/`) | Yes (`dist/server/`) | | ||
| | tsconfig layout | app + node split | Single tsconfig | Single tsconfig | | ||
| | Auto-committed | Depends on tool | Usually yes | **No — commit first** | | ||
| | `nx import` plugin | `@nx/vite` | `@nx/vite`, `@nx/react` | `@nx/vite`, `@nx/vitest` | | ||
|
|
||
| --- | ||
|
|
||
| ## Iteration Log | ||
|
|
||
| ### Scenario 6: Multiple non-Nx React apps (CRA, Next.js, React Router 7, TanStack Start, Vite) → TS preset (PASS) | ||
|
|
||
| - Sources: 5 standalone non-Nx repos with different build tools | ||
| - Dest: CNW ts preset (Nx 22.5.1), npm workspaces, `packages/*` | ||
| - Import: whole-repo for each, sequential into `packages/<name>` | ||
| - Pre-import fixes: | ||
| 1. Removed `packages/.gitkeep` and committed | ||
| 2. `git init && git add . && git commit` in Vite app (no git at all) | ||
| 3. `git add . && git commit` in TanStack app (git init'd but no commits) | ||
| - Import: `npm exec nx -- import <source> packages/<name> --source=. --ref=main --no-interactive` | ||
| - Next.js import auto-installed `@nx/eslint`, `@nx/next` | ||
| - React Router 7 import auto-installed `@nx/vite`, `@nx/react`, `@nx/docker` (Dockerfile present) | ||
| - TanStack import auto-installed `@nx/vitest` | ||
| - Post-import fixes: | ||
| 1. Removed stale `node_modules/`, `package-lock.json`, `.gitignore` from each package | ||
| 2. Removed Nx-rewritten scripts from `board-games-nextjs/package.json` (had `"build": "nx next:build"`, etc.) | ||
| 3. Updated root `tsconfig.base.json`: `nodenext` → `bundler`, added `dom`/`dom.iterable` to lib, added `jsx: react-jsx` | ||
| 4. Added `build` to dest root `.gitignore` (CRA and React Router 7 output there) | ||
| 5. Fixed `noEmit` → `composite + emitDeclarationOnly` in: `board-games-vite/tsconfig.app.json`, `board-games-vite/tsconfig.node.json`, `board-games-react-router/tsconfig.json`, `board-games-tanstack/tsconfig.json` | ||
| 6. Fixed `tsBuildInfoFile` paths from `./node_modules/.tmp/...` to `./dist/...` | ||
| 7. Installed root `@types/react`, `@types/react-dom`, `@types/node` | ||
| - All targets green: `build` for all 5 projects; `typecheck` for Vite/React Router/TanStack; `next:build` for Next.js |
There was a problem hiding this comment.
Verify alignment with PR objectives.
This file provides comprehensive Vite-specific guidance for the nx import skill, but the PR title and objectives focus on fixing a Webpack configuration for a Forge app to output only index.js instead of both main.js and index.js. This documentation doesn't appear to address that objective.
While the AI summary indicates the PR includes broader agent infrastructure changes, confirming the relationship between this Vite documentation and the stated Forge/Webpack objective would clarify the scope.
🧰 Tools
🪛 LanguageTool
[grammar] ~61-61: Ensure spelling is correct/": ["./app/"] }
Context: ...— self-referential alias; keep as-is -"noEmit": true` — replace...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nx-import/references/VITE.md around lines 1 - 397, The
VITE.md content is unrelated to the PR objective about the Forge/Webpack output
(index.js vs main.js); either remove or move this Vite-specific documentation
out of this PR and update the PR title/description to reflect only the
Forge/Webpack fix, or add a brief note in VITE.md explaining why it’s included
in this PR (e.g., infrastructure changes affecting multiple skills). Locate the
"Vite" header or the "`@nx/vite/plugin` Typecheck Target" section in VITE.md to
remove/move or to append the explanatory note, and ensure the PR description
references the moved file or the reason for its inclusion so reviewers see the
correct scope.
Comment on lines
+18
to
+20
| ``` | ||
| nx run <project>:<task> | ||
| ``` |
There was a problem hiding this comment.
Add fenced code block languages to satisfy markdownlint.
These blocks should be tagged (e.g. bash) to resolve MD040 warnings.
📝 Proposed fix
-```
+```bash
nx run <project>:<task>...
- +bash
nx run-many -t build test lint typecheck
...
-```
+```bash
nx affected -t build test lint
</details>
Also applies to: 26-28, 42-44
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nx-run-tasks/SKILL.md around lines 18 - 20, Add language tags
to the fenced code blocks to satisfy markdownlint MD040: update the three code
fences that contain the commands nx run <project>:<task>, nx run-many -t build test lint typecheck, and nx affected -t build test lint (and the other
occurrences you noted at lines 26-28 and 42-44) to use a language specifier such
as bash so they read bash ... ```; ensure all similar fenced blocks in
this document are updated consistently.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+95
to
+98
| const outputDir = join(workspaceDirectory, 'dist', 'apps', appName); | ||
| expect(existsSync(join(outputDir, 'src', 'index.js'))).toBe(true); | ||
| expect(existsSync(join(outputDir, 'src', 'main.js'))).toBe(false); | ||
|
|
There was a problem hiding this comment.
Fix build artifact assertion path (currently failing CI).
The CI failure shows dist/apps/<appName>/src/index.js does not exist. This assertion is blocking validation of the PR objective and likely targets the wrong location.
✅ Proposed fix
- expect(existsSync(join(outputDir, 'src', 'index.js'))).toBe(true);
- expect(existsSync(join(outputDir, 'src', 'main.js'))).toBe(false);
+ expect(existsSync(join(outputDir, 'index.js'))).toBe(true);
+ expect(existsSync(join(outputDir, 'main.js'))).toBe(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.
Suggested change
| const outputDir = join(workspaceDirectory, 'dist', 'apps', appName); | |
| expect(existsSync(join(outputDir, 'src', 'index.js'))).toBe(true); | |
| expect(existsSync(join(outputDir, 'src', 'main.js'))).toBe(false); | |
| const outputDir = join(workspaceDirectory, 'dist', 'apps', appName); | |
| expect(existsSync(join(outputDir, 'index.js'))).toBe(true); | |
| expect(existsSync(join(outputDir, 'main.js'))).toBe(false); |
🧰 Tools
🪛 GitHub Actions: CI
[error] 96-96: Forge application generator test failed. Expected generated output file to exist: dist/apps//src/index.js (existsSync(...toBe(true)) returned false).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/nx-forge-e2e/src/application.generator.spec.ts` around lines 95 - 98, The
assertions are checking the wrong output path (they expect files under
outputDir/src) causing CI to fail; update the two existsSync checks in this test
to target the built artifacts at the app root instead of the src subfolder by
using join(outputDir, 'index.js') for the positive assertion and join(outputDir,
'main.js') for the negative assertion (referencing outputDir and appName in the
existing test).
Comment on lines
+33
to
+35
| `npx -y create-nx-workspace@latest ${workspaceName} --preset=apps --nxCloud=skip --packageManager=npm --no-interactive`, | ||
| TEST_WORKSPACES_ROOT | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Bootstrap command in test workspace utility:"
rg -n "create-nx-workspace@" e2e/nx-forge-e2e/src/utils/test-workspace.ts
echo
echo "Nx versions declared in package manifests:"
rg -n '"nx"\s*:\s*".*"' --glob '**/package.json'Repository: toolsplus/nx-forge
Length of output: 314
Pin create-nx-workspace version to match repository Nx version.
Line 33 uses create-nx-workspace@latest, which causes test setup to drift from the pinned Nx 22.6.1 version in package.json. This non-determinism can cause test failures when Nx releases new versions with changed workspace defaults or behavior.
Proposed fix
+const CREATE_NX_WORKSPACE_VERSION = '22.6.1';
+
runCommand(
- `npx -y create-nx-workspace@latest ${workspaceName} --preset=apps --nxCloud=skip --packageManager=npm --no-interactive`,
+ `npx -y create-nx-workspace@${CREATE_NX_WORKSPACE_VERSION} ${workspaceName} --preset=apps --nxCloud=skip --packageManager=npm --no-interactive`,
TEST_WORKSPACES_ROOT
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/nx-forge-e2e/src/utils/test-workspace.ts` around lines 33 - 35, The test
setup command currently uses `create-nx-workspace@latest`, causing
non-deterministic workspace generation; update the command string (the one
constructed with workspaceName and TEST_WORKSPACES_ROOT) to pin the package to
the repository's Nx version (e.g., `create-nx-workspace@22.6.1`) so test
workspaces match package.json's Nx version; locate the command construction that
includes workspaceName and TEST_WORKSPACES_ROOT and replace the `@latest` tag
with the pinned version (or read the version from package.json and interpolate
it) to ensure deterministic e2e setup.
tbinna
force-pushed
the
webpack-only-index-js-output
branch
from
94d1586 to
6887a0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
e2e/nx-forge-e2e/src/application.generator.spec.ts (1)
48-60:⚠️ Potential issue | 🟠 MajorBuild artifact assertions still target the wrong folder.
Line 48 and Line 60 still assert under
dist/apps/<appName>/src, which was already shown to fail CI for this PR path validation. This can keep the test red and mis-validate theindex.js-only objective.✅ Proposed fix
- const indexJsPath = join(outputDir, 'src', 'index.js'); + const indexJsPath = join(outputDir, 'index.js'); @@ - expect(existsSync(join(outputDir, 'src', 'main.js'))).toBe(false); + expect(existsSync(join(outputDir, 'main.js'))).toBe(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/src/application.generator.spec.ts` around lines 48 - 60, The test is still checking build artifacts under the wrong subfolder; update the path references that use indexJsPath and the main.js existence check to target the top-level outputDir instead of outputDir/src. Specifically, change the indexJsPath assignment from join(outputDir, 'src', 'index.js') to join(outputDir, 'index.js') and update the expect existsSync(join(outputDir, 'src', 'main.js')) to expect existsSync(join(outputDir, 'main.js')) to correctly assert the index.js-only output and absence of main.js..agents/skills/monitor-ci/scripts/ci-state-update.mjs (2)
96-110:⚠️ Potential issue | 🟠 MajorRequire the tracking ID for each
post-actionmode.
apply-mcp/env-rerundepend on--cipe-url, and the push-based actions depend on--commit-sha. Without that check, wait mode can be entered with anulltracker and new-CI detection becomes blind.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/monitor-ci/scripts/ci-state-update.mjs around lines 96 - 110, The current logic allows entering waitMode with a null tracker because it only checks action membership (trackByCipeUrl/trackByCommitSha) but doesn’t require the actual tracking ID; update the validation so that when trackByCipeUrl is true (e.g., actions like 'apply-mcp' or 'env-rerun') you ensure cipeUrl is present and return an error if not, and when trackByCommitSha is true (push-based actions) you ensure commitSha is present and return an error if not; adjust the branch around trackByCipeUrl/trackByCommitSha (the code that sets agentTriggered and the output waitMode object) to enforce these checks before emitting the waitMode response.
41-43:⚠️ Potential issue | 🟠 MajorValidate numeric flags before arithmetic.
These
parseIntresults still flow straight into comparisons and increments. One malformed CLI value is enough to emitNaNcounters and poison the monitor state carried to the next cycle.Also applies to: 58-59, 122-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/monitor-ci/scripts/ci-state-update.mjs around lines 41 - 43, The parsed CLI flags for counters are not validated, so malformed values become NaN and break comparisons; update the parsing logic used around getArg('--local-verify-count') and getArg('--local-verify-attempts') to validate and coerce to a safe integer before any arithmetic: parse with parseInt, then check Number.isFinite or Number.isInteger/!Number.isNaN and if invalid replace with a sensible default (0 for counts, 3 for attempts), optionally clamp to non-negative integers, and use that sanitized value for the comparisons and increments; apply the same validation fix to the other identical parse sites (the other getArg usages noted).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs:
- Around line 42-49: The parsed numeric CLI flags timeoutSeconds,
newCipeTimeoutSeconds, envRerunCount, and inputNoProgressCount can become NaN
for invalid inputs; validate each after parseInt (e.g., with Number.isFinite or
isNaN) and replace NaN or negative values with safe defaults (0 or another
sensible default) before they are used in counters/delays and included in JSON
output; update the code that sets timeoutSeconds, newCipeTimeoutSeconds,
envRerunCount, and inputNoProgressCount to perform this check so invalid CLI
values cannot disable guards or leak into the result.
- Around line 419-425: The calculation for noProgressCount is accidentally
treating the very first poll as “no progress” when previous-state inputs are
still unset; update the IIFE that computes noProgressCount to detect an initial
poll (when inputNoProgressCount is null/undefined or when previous-state inputs
are all unset) and return 0 in that case instead of incrementing; keep the
existing branches for waitMode, isNewCipe(), and hasStateChanged() but add an
early check like "if (inputNoProgressCount == null) return 0" (or equivalent
check for unset previous-state inputs) so the first steady-state poll does not
increment to 1.
In @.agents/skills/monitor-ci/SKILL.md:
- Around line 92-95: HEAVY_FIELDS is missing fields that the Step 3 branches
read: add taskFailureSummaries and suggestedFixSummary to the HEAVY_FIELDS list
so the branches fix_needs_review, fix_failed and no_fix have the data they
require; update every HEAVY_FIELDS occurrence in the document (the earlier
HEAVY_FIELDS block and the later repeated block) to include these two
identifiers alongside taskOutputSummary, suggestedFix, suggestedFixReasoning and
suggestedFixDescription.
- Around line 24-35: The timeout flags (--timeout and --new-cipe-timeout) are
documented in minutes but are passed to ci-poll-decide.mjs as seconds
(<timeout_seconds> / <new_cipe_timeout_seconds>) without conversion; update the
argument parsing/dispatch logic (where you build the ci-poll-decide.mjs
invocation) to explicitly convert the minute defaults/overrides to seconds by
multiplying by 60 before passing them, and apply the same change in the second
invocation/site (the other ci-poll-decide.mjs call referenced in the file);
ensure the variables used in the call match the converted names (e.g.,
timeout_seconds, new_cipe_timeout_seconds) so the companion script gets seconds.
In @.agents/skills/nx-import/references/JEST.md:
- Around line 62-64: The markdown fenced code blocks in
.agents/skills/nx-import/references/JEST.md containing shell commands (for
example the blocks with "pnpm add -wD jest ts-jest `@types/jest` `@nx/jest`", "pnpm
add -wD `@testing-library/react` `@testing-library/jest-dom`", "pnpm add -wD
babel-jest `@babel/core` `@babel/preset-env` `@babel/preset-react`
`@babel/preset-typescript`", and "pnpm add -wD `@vue/test-utils`") are unlabeled;
add the language identifier "bash" (i.e., change ``` to ```bash) for each of
these fenced blocks so markdown linters and syntax highlighting treat them as
shell snippets. Ensure every similar fenced block in the file is updated (the
ones containing the above commands) to maintain consistency.
In @.agents/skills/nx-import/references/VITE.md:
- Around line 327-330: The cross-reference in the "Non-Nx Source: React Router
7" section currently points to a non-existent SKILL.md section "Source Repo Has
No Commits"; update that reference to point to the local "Uncommitted Source
Repo" note (or add a matching "Source Repo Has No Commits" section to the
runbook) so readers can find guidance; edit the text in the VITE.md block under
"Non-Nx Source: React Router 7" to replace the SKILL.md cross-reference with the
correct local note title and ensure links/anchor text match the existing
"Uncommitted Source Repo" heading.
---
Duplicate comments:
In @.agents/skills/monitor-ci/scripts/ci-state-update.mjs:
- Around line 96-110: The current logic allows entering waitMode with a null
tracker because it only checks action membership
(trackByCipeUrl/trackByCommitSha) but doesn’t require the actual tracking ID;
update the validation so that when trackByCipeUrl is true (e.g., actions like
'apply-mcp' or 'env-rerun') you ensure cipeUrl is present and return an error if
not, and when trackByCommitSha is true (push-based actions) you ensure commitSha
is present and return an error if not; adjust the branch around
trackByCipeUrl/trackByCommitSha (the code that sets agentTriggered and the
output waitMode object) to enforce these checks before emitting the waitMode
response.
- Around line 41-43: The parsed CLI flags for counters are not validated, so
malformed values become NaN and break comparisons; update the parsing logic used
around getArg('--local-verify-count') and getArg('--local-verify-attempts') to
validate and coerce to a safe integer before any arithmetic: parse with
parseInt, then check Number.isFinite or Number.isInteger/!Number.isNaN and if
invalid replace with a sensible default (0 for counts, 3 for attempts),
optionally clamp to non-negative integers, and use that sanitized value for the
comparisons and increments; apply the same validation fix to the other identical
parse sites (the other getArg usages noted).
In `@e2e/nx-forge-e2e/src/application.generator.spec.ts`:
- Around line 48-60: The test is still checking build artifacts under the wrong
subfolder; update the path references that use indexJsPath and the main.js
existence check to target the top-level outputDir instead of outputDir/src.
Specifically, change the indexJsPath assignment from join(outputDir, 'src',
'index.js') to join(outputDir, 'index.js') and update the expect
existsSync(join(outputDir, 'src', 'main.js')) to expect
existsSync(join(outputDir, 'main.js')) to correctly assert the index.js-only
output and absence of main.js.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9924c0ab-8386-4d28-b5fc-5dbf6ceb5884
📒 Files selected for processing (31)
.agents/skills/link-workspace-packages/SKILL.md.agents/skills/monitor-ci/SKILL.md.agents/skills/monitor-ci/references/fix-flows.md.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs.agents/skills/monitor-ci/scripts/ci-state-update.mjs.agents/skills/nx-generate/SKILL.md.agents/skills/nx-import/SKILL.md.agents/skills/nx-import/references/ESLINT.md.agents/skills/nx-import/references/GRADLE.md.agents/skills/nx-import/references/JEST.md.agents/skills/nx-import/references/NEXT.md.agents/skills/nx-import/references/TURBOREPO.md.agents/skills/nx-import/references/VITE.md.agents/skills/nx-plugins/SKILL.md.agents/skills/nx-run-tasks/SKILL.md.agents/skills/nx-workspace/SKILL.md.agents/skills/nx-workspace/references/AFFECTED.md.claude/settings.json.codex/agents/ci-monitor-subagent.toml.codex/config.toml.gitignoreAGENTS.mde2e/nx-forge-e2e/src/application.generator.spec.tse2e/nx-forge-e2e/src/basic-setup.spec.tse2e/nx-forge-e2e/src/utils/async-commands.tse2e/nx-forge-e2e/src/utils/atlassian-graphql-client.tse2e/nx-forge-e2e/src/utils/cleanup-registered-forge-app.tse2e/nx-forge-e2e/src/utils/test-project.tse2e/nx-forge-e2e/src/utils/test-workspace.tspackages/nx-forge/src/generators/application/files/webpack.config.js__tmpl__tools/scripts/stop-local-registry.ts
💤 Files with no reviewable changes (1)
- e2e/nx-forge-e2e/src/utils/test-project.ts
✅ Files skipped from review due to trivial changes (17)
- .gitignore
- .agents/skills/nx-plugins/SKILL.md
- .agents/skills/nx-workspace/references/AFFECTED.md
- .claude/settings.json
- e2e/nx-forge-e2e/src/utils/atlassian-graphql-client.ts
- e2e/nx-forge-e2e/src/basic-setup.spec.ts
- .agents/skills/nx-import/references/GRADLE.md
- .agents/skills/nx-run-tasks/SKILL.md
- .agents/skills/link-workspace-packages/SKILL.md
- .agents/skills/nx-workspace/SKILL.md
- .agents/skills/nx-generate/SKILL.md
- .codex/agents/ci-monitor-subagent.toml
- .agents/skills/nx-import/references/TURBOREPO.md
- .agents/skills/nx-import/references/ESLINT.md
- e2e/nx-forge-e2e/src/utils/test-workspace.ts
- .agents/skills/monitor-ci/references/fix-flows.md
- .agents/skills/nx-import/references/NEXT.md
🚧 Files skipped from review as they are similar to previous changes (5)
- tools/scripts/stop-local-registry.ts
- e2e/nx-forge-e2e/src/utils/async-commands.ts
- .codex/config.toml
- e2e/nx-forge-e2e/src/utils/cleanup-registered-forge-app.ts
- packages/nx-forge/src/generators/application/files/webpack.config.js__tmpl__
Comment on lines
+42
to
+49
| const timeoutSeconds = parseInt(getArg('--timeout') || '0', 10); | ||
| const newCipeTimeoutSeconds = parseInt(getArg('--new-cipe-timeout') || '0', 10); | ||
| const envRerunCount = parseInt(getArg('--env-rerun-count') || '0', 10); | ||
| const inputNoProgressCount = parseInt(getArg('--no-progress-count') || '0', 10); | ||
| const prevCipeStatus = getArg('--prev-cipe-status'); | ||
| const prevShStatus = getArg('--prev-sh-status'); | ||
| const prevVerificationStatus = getArg('--prev-verification-status'); | ||
| const prevFailureClassification = getArg('--prev-failure-classification'); |
There was a problem hiding this comment.
Validate numeric CLI flags before using them in counters and delays.
A bad --timeout, --new-cipe-timeout, --env-rerun-count, or --no-progress-count currently becomes NaN, which disables the relevant guard and can leak invalid state into the JSON result.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs around lines 42 - 49,
The parsed numeric CLI flags timeoutSeconds, newCipeTimeoutSeconds,
envRerunCount, and inputNoProgressCount can become NaN for invalid inputs;
validate each after parseInt (e.g., with Number.isFinite or isNaN) and replace
NaN or negative values with safe defaults (0 or another sensible default) before
they are used in counters/delays and included in JSON output; update the code
that sets timeoutSeconds, newCipeTimeoutSeconds, envRerunCount, and
inputNoProgressCount to perform this check so invalid CLI values cannot disable
guards or leak into the result.
Comment on lines
+419
to
+425
| // Compute noProgressCount from input. Single assignment, no mutation. | ||
| // Wait mode: reset on new cipe, otherwise unchanged (wait doesn't count as no-progress). | ||
| // Normal mode: reset on any state change, otherwise increment. | ||
| const noProgressCount = (() => { | ||
| if (waitMode) return isNewCipe() ? 0 : inputNoProgressCount; | ||
| if (isNewCipe() || hasStateChanged()) return 0; | ||
| return inputNoProgressCount + 1; |
There was a problem hiding this comment.
Don’t treat the first poll as “no progress.”
When all previous-state inputs are still unset, this starts noProgressCount at 1. That bumps the first steady-state delay to 90 seconds and makes the 5-poll circuit breaker fire one poll early on long-running CI attempts.
🔧 Proposed fix
+const hasPreviousObservation =
+ prevCipeStatus !== null ||
+ prevShStatus !== null ||
+ prevVerificationStatus !== null ||
+ prevFailureClassification !== null;
+
// Compute noProgressCount from input. Single assignment, no mutation.
// Wait mode: reset on new cipe, otherwise unchanged (wait doesn't count as no-progress).
// Normal mode: reset on any state change, otherwise increment.
const noProgressCount = (() => {
if (waitMode) return isNewCipe() ? 0 : inputNoProgressCount;
+ if (!hasPreviousObservation) return 0;
if (isNewCipe() || hasStateChanged()) return 0;
return inputNoProgressCount + 1;
})();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs around lines 419 - 425,
The calculation for noProgressCount is accidentally treating the very first poll
as “no progress” when previous-state inputs are still unset; update the IIFE
that computes noProgressCount to detect an initial poll (when
inputNoProgressCount is null/undefined or when previous-state inputs are all
unset) and return 0 in that case instead of incrementing; keep the existing
branches for waitMode, isNewCipe(), and hasStateChanged() but add an early check
like "if (inputNoProgressCount == null) return 0" (or equivalent check for unset
previous-state inputs) so the first steady-state poll does not increment to 1.
Comment on lines
+24
to
+35
| | Setting | Default | Description | | ||
| | ------------------------- | ------------- | ------------------------------------------------------------------------- | | ||
| | `--max-cycles` | 10 | Maximum **agent-initiated** CI Attempt cycles before timeout | | ||
| | `--timeout` | 120 | Maximum duration in minutes | | ||
| | `--verbosity` | medium | Output level: minimal, medium, verbose | | ||
| | `--branch` | (auto-detect) | Branch to monitor | | ||
| | `--fresh` | false | Ignore previous context, start fresh | | ||
| | `--auto-fix-workflow` | false | Attempt common fixes for pre-CI-Attempt failures (e.g., lockfile updates) | | ||
| | `--new-cipe-timeout` | 10 | Minutes to wait for new CI Attempt after action | | ||
| | `--local-verify-attempts` | 3 | Max local verification + enhance cycles before pushing to CI | | ||
|
|
||
| Parse any overrides from `$ARGUMENTS` and merge with defaults. |
There was a problem hiding this comment.
Make the timeout conversion explicit before invoking ci-poll-decide.mjs.
The defaults are documented in minutes, but the companion script compares these args against 30/60/90-second delays. Step 2b passes <timeout_seconds> / <new_cipe_timeout_seconds> without ever stating the conversion, so a literal default value would expire about 60× too early.
📝 Suggested spec tweak
Parse any overrides from `$ARGUMENTS` and merge with defaults.
+Convert `--timeout` and `--new-cipe-timeout` from minutes to seconds before passing them to `ci-poll-decide.mjs`.Also applies to: 177-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/monitor-ci/SKILL.md around lines 24 - 35, The timeout flags
(--timeout and --new-cipe-timeout) are documented in minutes but are passed to
ci-poll-decide.mjs as seconds (<timeout_seconds> / <new_cipe_timeout_seconds>)
without conversion; update the argument parsing/dispatch logic (where you build
the ci-poll-decide.mjs invocation) to explicitly convert the minute
defaults/overrides to seconds by multiplying by 60 before passing them, and
apply the same change in the second invocation/site (the other
ci-poll-decide.mjs call referenced in the file); ensure the variables used in
the call match the converted names (e.g., timeout_seconds,
new_cipe_timeout_seconds) so the companion script gets seconds.
Comment on lines
+92
to
+95
| ```yaml | ||
| WAIT_FIELDS: 'cipeUrl,commitSha,cipeStatus' | ||
| LIGHT_FIELDS: 'cipeStatus,cipeUrl,branch,commitSha,selfHealingStatus,verificationStatus,userAction,failedTaskIds,verifiedTaskIds,selfHealingEnabled,failureClassification,couldAutoApplyTasks,autoApplySkipped,autoApplySkipReason,shortLink,confidence,confidenceReasoning,hints,selfHealingSkippedReason,selfHealingSkipMessage' | ||
| HEAVY_FIELDS: 'taskOutputSummary,suggestedFix,suggestedFixReasoning,suggestedFixDescription' |
There was a problem hiding this comment.
Request the heavy fields that Step 3 actually reads.
HEAVY_FIELDS does not include taskFailureSummaries or suggestedFixSummary, but the fix_needs_review / fix_failed / no_fix branches later depend on them. As written, the documented fetch can succeed while still leaving those flows without the data they ask for.
Also applies to: 231-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/monitor-ci/SKILL.md around lines 92 - 95, HEAVY_FIELDS is
missing fields that the Step 3 branches read: add taskFailureSummaries and
suggestedFixSummary to the HEAVY_FIELDS list so the branches fix_needs_review,
fix_failed and no_fix have the data they require; update every HEAVY_FIELDS
occurrence in the document (the earlier HEAVY_FIELDS block and the later
repeated block) to include these two identifiers alongside taskOutputSummary,
suggestedFix, suggestedFixReasoning and suggestedFixDescription.
Comment on lines
+62
to
+64
| ``` | ||
| pnpm add -wD jest ts-jest @types/jest @nx/jest | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to the shell snippets.
These fenced blocks are currently unlabeled, so markdown linting and syntax highlighting will keep treating them as generic text.
📝 Suggested cleanup
-```
+```bash
pnpm add -wD jest ts-jest `@types/jest` `@nx/jest`- +bash
pnpm add -wD @testing-library/react @testing-library/jest-dom
-```
+```bash
pnpm add -wD babel-jest `@babel/core` `@babel/preset-env` `@babel/preset-react` `@babel/preset-typescript`
- +bash
pnpm add -wD @vue/test-utils
Also applies to: 73-75, 81-83, 89-91
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nx-import/references/JEST.md around lines 62 - 64, The
markdown fenced code blocks in .agents/skills/nx-import/references/JEST.md
containing shell commands (for example the blocks with "pnpm add -wD jest
ts-jest `@types/jest` `@nx/jest`", "pnpm add -wD `@testing-library/react`
`@testing-library/jest-dom`", "pnpm add -wD babel-jest `@babel/core`
`@babel/preset-env` `@babel/preset-react` `@babel/preset-typescript`", and "pnpm add
-wD `@vue/test-utils`") are unlabeled; add the language identifier "bash" (i.e.,
change ``` to ```bash) for each of these fenced blocks so markdown linters and
syntax highlighting treat them as shell snippets. Ensure every similar fenced
block in the file is updated (the ones containing the above commands) to
maintain consistency.
Comment on lines
+327
to
+330
| ### Non-Nx Source: React Router 7 | ||
|
|
||
| 1. Ensure source has at least one commit (see SKILL.md: "Source Repo Has No Commits") | ||
| 2. `nx import` whole-repo into `apps/<name>` (see SKILL.md: "Application vs Library Detection") → auto-installs `@nx/vite`, `@nx/react` |
There was a problem hiding this comment.
Fix the stale SKILL.md cross-reference.
This section points readers to SKILL.md: "Source Repo Has No Commits", but that section does not exist in the companion runbook. Point this at the local "Uncommitted Source Repo" note or add the missing shared section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nx-import/references/VITE.md around lines 327 - 330, The
cross-reference in the "Non-Nx Source: React Router 7" section currently points
to a non-existent SKILL.md section "Source Repo Has No Commits"; update that
reference to point to the local "Uncommitted Source Repo" note (or add a
matching "Source Repo Has No Commits" section to the runbook) so readers can
find guidance; edit the text in the VITE.md block under "Non-Nx Source: React
Router 7" to replace the SKILL.md cross-reference with the correct local note
title and ensure links/anchor text match the existing "Uncommitted Source Repo"
heading.
Contributor
|
🎉 This PR is included in version 7.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Closes #195
Summary by CodeRabbit
Documentation
New Features
Tests
Chores